Repository: AppDaemon/appdaemon (language: python)

Issue:
Suggestion: Improve CI pipeline
## Introduction
Thanks for this amazing project!
I have been using it for quite some time to easily integrate an external device (a TP-link router) in Home Assistant.
After seeing the reported issues in the Discord chat about the docker builds failing, I would like to chime in and give an helping hand, having had some previous experience with CI systems (Gitlab CI and Github Actions) and Docker containers.
After a quick glance at the project, I have some questions regarding its development cycle:

## Continuous integration platform
It appears that multiple CI platforms are used:
- [semaphore ci](https://github.com/AppDaemon/appdaemon/blob/8de3a24ce447f954c8c7fe58d80eb6257c318508/.semaphore/semaphore.yml)
- [azure pipelines](https://github.com/AppDaemon/appdaemon/blob/8de3a24ce447f954c8c7fe58d80eb6257c318508/azure-pipelines.yml)
-  __Github Actions__: there has been a previous attempt in #1022 to introduce it, but at the moment in the `dev` branch the configuration has been removed, as seen from this commit: 363f1651fe5bbcb43814b712fa52be211946326f

### Question
Is there a reason why different CI platforms are needed? Wouldn't it be better to use a single one to consolidate the efforts and provide a single interface to see the latest build status?
Since the project is already using Github as its hosting platform, I am inclined to proposed __Github Actions__ as the CI provider, since it intuitively integrates with the rest of the ecosystem (github issues, pull request, releases, docker registry, etc..).

## Proposed changes
- [x] Open a discussion to decide if multiple CI platforms are indeed needed and preferable. In case a single one is preferred:
  - [x] Consolidate the workflow on a single platform to do automated testing and deployment
- [x] Define the automated steps that should happen in the CI pipeline (e.g.: should we run tests for every git push? create docker images on git tags?)
- [ ] Integrate the changes proposed in #1646 in a CI configuration (automatically generate release notes and release artifacts as part of the Github project)

## Implementation plan
General pipeline configuration:
- [x] Build dev as commits are pushed
- [x] Build releases whenever a TAG is created

The following is an outline of the ideal pipeline, summarized from the comments below:
- [x] Build and deploy docker multi arch images
- [x] Documentation (built using readthedocs)
- [x] Update dependencies (currently `dependabot` is used)
- [ ] Automatic release notes


Discussion / hints:
Hi Carlo - and thanks for reaching out!

I think consolidating all the CI pipeline would be a great idea - we got where we are today because the various pieces were added piecemeal over the last few years. A well meaning person would contribute some config for their favorite CI pipeline to solve a specific problem, then usually move on leaving me to maintain something I had no understanding of ;)

Keeping it all in one place sounds like a great idea, but I have limited experience in the CI side of things, and as you have seen, when things break, it is a scramble to fix them.

So, with the following points in mind, if it is a good fit, I would be happy to build a single pipeline, and I am comfortable with the choice of Git if it makes the most sense. The ideal pipeline would:

- Handle Pre commit hooks (black & flake) (Azure used for this now)
- Build and deploy docker multi arch images (semaphore currently)
- Documentation (Readthe docs currently in use, I would probably expect to keep that as I am not aware of any alternative)
- Update dependencies (currently dependably, not sure if there is an alternative)
- Some automation of release notes etc. (we have none at the moment)

Even consolidating the semaphore and azure pipelines would be a win in my opinion. However, there are some important non-technical considerations too:

- AppDaemon has zero funding of any sort so would need to use whatever free tiers of tools are available
- Whatever the chosen pipeline is, it would be very helpful if the original designer was available to help out if things went wrong, ideally interactively over discord

In terms of CI process, I think what we have at the moment works well:

- Build dev as commits are pushed
- Build releases whenever a TAG is created

So, given the above do you think Git CI would be a fit, and would yo use willing to help?  If so, what would you suggest for next steps? I know little about this subject so would really want you to help build the pipeline from scratch to agreed specs, then ideally be around afterwards to help out if anything goes wrong, if not, I will have just exchanged one set of arcane incomprehensible systems for another :) One advantage of your proposal is, as you said that it is all under one roof so hopefully it will be simpler to understand and maintain which would be a win in its own right.

Let me know what you think - I do like this idea a lot!
I can understand your frustration, being an open source project maintainer and having a changing sets of hands with the coming and going of each volunteer can be problematic.
For this reason I think a shared design and (a bit of) documentation on the chosen CI solution is fundamental, since in that case anyone willing to work on the implemented solution can understand what has been built and give its contribution, especially when things go wrong.

I know this sounds a bit cliche, but a well-managed CI pipeline reduces the need for human intervention, since each step is documented as code in the pipeline definition, thereby reducing the need for manual steps in the development of an application. This could be helpful in the maintenance aspects of this project, for instance reducing the need for manually publishing release artifacts and documentation.
I think the Docker container errors encountered the other day could be avoided by simply building the Docker image automatically as part of the pipeline, thereby easily catching any change that may introduce errors in the build process!

## Ideal candidate
I think Github Actions might be ideal since less third-party solutions there are, less things can break at the wrong time, plus at the same time it is all consolidated under the same roof, as you said.
It ticks the following boxes:
- free for open-source projects like this
- tightly integrated with github issues and pull requests, so the results of the pipeline are easily integrated as part of the development process
- can be easily customized without the need of an external service (for instance now semaphore and azure pipelines are configured via their external dashboard, with their own user access and secret management controls).
 With Github Actions every options is here inside the Github repository.
 
## Next steps
I can help out in the implementation. I cannot guarantee my 100% availability (aren't we all volunteers after all? :wink: ), but I will do my best.
To start, I can work on automating the setup of the dependencies when checking out the project, as part of a basic CI step.
Starting from there the sky is the limit! :)

P.S.: Does this project have any unit testing? (`pytest` or the like of it?)

OK, sounds good - I am looking forward to seeing how things progress. And of course I don't expect 100% availability - I just hope that you might respond to an email or on discord within a day or two if I find an issue I can't figure out :) Although as you say, if its 1 consistent pipeline I stand a better chance of getting familiar with it and needing less help.

As far as unit testing goes - no we don't have any. It's another thing on my long list to become an expert at ... I did at one point make a start writing my own framework for this but I got bogged down. If you can recommend a framework, and at least show me how to get started I can add this into the mix as well, as it is a good idea, and something I have been meaning to get to at some point.
> As far as unit testing goes - no we don't have any. It's another thing on my long list to become an expert at ... I did at one point make a start writing my own framework for this but I got bogged down. If you can recommend a framework, and at least show me how to get started I can add this into the mix as well, as it is a good idea, and something I have been meaning to get to at some point.

I have no familiarity at all with the AppDaemon code, so proposing one framework vs another might be hazardous, since I don't know the complexity and intricacies of the code we are dealing with.
In some other projects I used with great success `pytest`. It seems to have lots of popularity in the Python world, with plugins and extensions behind it for various use cases.

Ok, presuming that pytest can easily be integrated with the new pipeline I’ll take a look – thanks.

Looks like the build succeeded but the push to Docker failed, I did set up the secrets in GutHub, but maybe not correctly. Here is the end of the log:

```
621
#22 exporting to image
[622](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:626)
#22 exporting layers
[623](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:627)
#22 exporting layers 58.4s done
[624](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:628)
#22 exporting manifest sha256:dac1fc9cce3a670f6c42fba94c14d17dbe5d2e8c6e750d83ed12590ec563c47b 0.0s done
[625](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:629)
#22 exporting config sha256:d7a3ff999ba4adeabb683e1f5a9aab33861c61414b8736a7016e8ee882da1ef0 done
[626](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:630)
#22 exporting attestation manifest sha256:08cfe0dcabe58fdcb74572564f723eb89ed78e7076a2301c7d20a7f49f846b24 done
[627](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:631)
#22 exporting manifest sha256:9d6495fdd15b604e385b2f06bda08d13789a8fcfc9cfeadc956931c3f390724e done
[628](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:632)
#22 exporting config sha256:eb838c1fa96d919b608664bcf941574fa9711e0945b8e5957b781d386c7a4000 done
[629](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:633)
#22 exporting attestation manifest sha256:246d03399e2ea4d7ad4c65d26f08366771ffc72771ecde826732cd0fda33d3f2 done
[630](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:634)
#22 exporting manifest list sha256:9af23b690cf5b584a4090e63f92a356f604b393453afafa3b8685aef9b64b8a2 done
[631](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:635)
#22 pushing layers
[632](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:636)
#22 ...
[633](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:637)

[634](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:638)
#23 [auth] appdaemon/appdaemon:pull,push token for registry-1.docker.io
[635](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:639)
#23 DONE 0.0s
[636](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:640)

[637](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:641)
#22 exporting to image
[638](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:642)
#22 ...
[639](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:643)

[640](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:644)
#24 [auth] appdaemon/appdaemon:pull,push token for registry-1.docker.io
[641](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:645)
#24 DONE 0.0s
[642](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:646)

[643](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:647)
#25 [auth] appdaemon/appdaemon:pull,push token for registry-1.docker.io
[644](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:648)
#25 DONE 0.0s
[645](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:649)

[646](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:650)
#22 exporting to image
[647](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:651)
#22 ...
[648](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:652)

[649](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:653)
#26 [auth] appdaemon/appdaemon:pull,push token for registry-1.docker.io
[650](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:654)
#26 DONE 0.0s
[651](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:655)

[652](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:656)
#22 exporting to image
[653](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:657)
#22 pushing layers 1.0s done
[654](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:658)
#22 ERROR: failed to push appdaemon/appdaemon:dev: server message: insufficient_scope: authorization failed
[655](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:659)

[656](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:660)
#27 [auth] appdaemon/appdaemon:pull,push token for registry-1.docker.io
[657](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:661)
#27 DONE 0.0s
[658](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:662)

[659](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:663)
#28 [auth] appdaemon/appdaemon:pull,push token for registry-1.docker.io
[660](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:664)
#28 DONE 0.0s
[661](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:665)
------
[662](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:666)
 > exporting to image:
[663](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:667)
------
[664](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:668)
ERROR: failed to solve: failed to push appdaemon/appdaemon:dev: server message: insufficient_scope: authorization failed
[665](https://github.com/AppDaemon/appdaemon/actions/runs/4298011420/jobs/7491897073#step:6:669)
Error: buildx failed with: ERROR: failed to solve: failed to push appdaemon/appdaemon:dev: server message: insufficient_scope: authorization failed
```
Looks like we are close though :)
I fixed it!

The action was using $github.repository as the tag name, which resolves to "appdaemon/appdaemon", however it needed to be "acockburn/appdaemon" for docker hub. I hard coded that value in the ENV var, as I couldn't see an easy way to get it out of GitHub variables.

Now for an encore I am going to try and add the arm images to the build ...
The 4 platform build fails identically to how it did on Semaphore, so it doesn't seem like a CI issue, but a problem with buildx. Would be nice to find a solution at some point but that is outside the scope of this exercise.

So, we have docker builds, pre-commit runs, PyPi automation (still to be tested). I think you have covered most things apart from a few details to be discussed on the PyPi piece - once again thanks for diving in and helping with this I am really happy with the result, and more importantly I think I understand it to the point where I can do a better job of maintaining it moving forwards. 
Once we have finalized the PyPi piece I'll do a mini-re;ease to test it all - I have a minor addition to the Admin UI I'd like to get out there to gather info on proposed changes to the HASS plugin.
Glad that everything seems to be improving nicely!
I saw the Docker file has these install step
https://github.com/AppDaemon/appdaemon/blob/7f834a1005489453756073ad7b0eda7adf57987f/Dockerfile#L26
Are all those packages needed? Excpecially cargo seems to cause issues when building the images, from my understanding it is a package mangaer for rust, but we are working with Python here! 😉 
They were added at some point presumably to help with build problems ... I'll do some work on the docker build over the next few days. I have the new PR to merge and then I can look at your suggestions as well. I set up my IDE to build the docker image which is something I hadn't;t been setup to do for a while after I moved platforms.
Locally I tested by removing all these `apk add` and I was able to build successfully a Docker image. 
Including the apk dependencies produces a Docker image of a whooping 1.22GB in size, versus 118MB:
```bash
$ docker image ls
appdaemon             without-apk       9ed8d3b55d59   2 hours ago   118MB
appdaemon             with-apk          4cdacc10d194   2 hours ago   1.22GB


```
That may well be the cause of the build problems then – what even is APK?

It's the `apt-get` equivalent in the Alpine linux distribution
Ok, cool, which dod you settle on as being necessary and we’ll start with that as a base.



OK, looking at the build pipeline, seems like it will build any branch:

```
on:
  push:
    branches: ["**"]
    tags: ["*"]
  pull_request:
    branches: ["dev"]
```

Can I change it to:

```
on:
  push:
    branches: ["dev"]
    tags: ["*"]
  pull_request:
    branches: ["dev"]
```

To only build the dev branch when it changes and no others? That better suits the way we work - most branches outside of dev and master are experimental or temporary
My reasoning behind that change is that this way every time there is a change it triggers a Python build and a Docker image build, without actually publishing anything to PyPI or Docker Hub.
This allow to always test that the project can build correctly, and any change introduced surfaces immediately the problem, so it can be quickly fixed.
Otherwise we may end up like the other day, when you were releasing the new version and you scrambled to fix the Docker builds. Now if something breaks we are immediately notified, without having to wait the day of the release. :wink: 
ok, got it - it's built but not published, sounds good :)
OK, docs now b building cleanly as well
OK, I removed support for python 3.7 and upgraded flake8 to 6.0.0 via dependabot. I also need to keep the flake8 and black versions at the same level for the pre-commit hooks - is there a way to pull this info from project.tml instead of manually keeping them in sync?
What are your thoughts on including a step in the GitHub lint action to install and run pre-commit, as an alternative to explicitly running just flake8? That would pull black in too which I think is desirable. In addition, I am looking at ruff as an alternative to flake8 and black, and if I make that change the CI pipeline should just follow along. Can you think of a reason to avoid doing that? 
> OK, docs now b building cleanly as well

Wow, you have lots more patience than me! :wink: 

> OK, I removed support for python 3.7 and upgraded flake8 to 6.0.0 via dependabot. I also need to keep the flake8 and black versions at the same level for the pre-commit hooks - is there a way to pull this info from project.tml instead of manually keeping them in sync?

The `pre-commit` hooks run in their own separate environment from the project itself, as you can see from its CLI output the first time you initialize it. I added `flake` and `black` to the `pyproject.toml` yaml just for convenience, to be be readily available in the local dev environment by directly invoking them without the  `pre-commit` middle-man.
Since however both tools are invoked primarily via `pre-commit` and this has its own configuration file, we could remove them from `pyproject.toml`, to avoid this confusion and have a single source of truth for versioning them.

> What are your thoughts on including a step in the GitHub lint action to install and run pre-commit, as an alternative to explicitly running just flake8?

At the moment the pre-commit hooks are meant to run locally on a dev machine, before a commit is made, to reformat/lint the code.
Since the CI pipeline uses its own copy of the Git repo to do its things (via the `actions/checkout` action), if we run `pre-commit` as part of the Github action the changes done to the local file are lost, since they are not pushed back to the main repo on Github.
Well well, hear me out. I just found out this: https://pre-commit.ci/lite.html, a Github Action created by the author of `pre-commit`, that automatically runs `pre-commit` in the pipeline, suggesting fixes directly inside the PR! Maybe we should give it a try..

> In addition, I am looking at ruff as an alternative to flake8 and black, and if I make that change the CI pipeline should just follow along. Can you think of a reason to avoid doing that?

For this I think we should check if `ruff` nicely integrates with `pre-commit`, as `black` and `flake8` do.

Great - I think we are definitely on the same page. My concern with the pre-commit is that someone who didn't install pre-commit could still create a PR so I would want it checked when the PR is created. 

I like the idea of keeping pre-commit tools separate and having a single source of truth. I have found out that pre-commit has an update function you can run to grab the latest tool versions. This is manual but not really onerous to do once in a while, plus you can run pre-commit manually against the entire repo (I did it last night and found a ton of stuff that needed fixing despite having this in place for a while - also could have been than the new flake8 is more strict). Then the cool part is the the pipeline would just follow along if that pre-commit.ci/lite works how we think it should.

At that point, I think we will have done everything I can think of and I am confident I can maintain this new pipeline since you have documented it extremely well, and I have been involved in the decision making. With everything setup this way, if I do decide to look at ruff the pipeline will update automatically - but I'll leave ruff for another day after some more research. I think, it is interesting because it is super fast and also has plugins for multiple tools, some of which I am interested in like pyupdate and a few others. I got the tip from Frank - homeassistant is looking at using it.

I would like to compliment you on your thorough and considered approach, your clear communication, and commenting/documenting of the code you put together - you are a true professional and I am deeply grateful for all the help!

So, if you can help me figure out how to replace the current CI listing steps with the pre-commit stuff, I think we will be pretty much done :)

Fix patch:
diff --git a/pyproject.toml b/pyproject.toml
index a1fbd2bfb..0f0ba72b9 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -52,8 +52,6 @@ changelog = "https://github.com/AppDaemon/appdaemon/blob/master/docs/HISTORY.rst
 [project.optional-dependencies]
 dev = [
     "build==0.10.0",
-    "flake8==6.0.0",
-    "black==23.1.0",
     'pre-commit==3.1.1; python_version>"3.7"',  # pre-commit does not support Python < 3.8
     "pytest==7.2.1",
     "sphinx-autobuild==2021.3.14",


Test patch:
diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml
index e073a2753..6fb098e66 100644
--- a/.github/workflows/python-tests.yml
+++ b/.github/workflows/python-tests.yml
@@ -9,8 +9,25 @@ on:
     branches: ["dev"]
 
 jobs:
-  build:
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v3
+    # https://github.com/actions/setup-python
+    - name: Set up Python
+      uses: actions/setup-python@v4
+      with:
+        python-version: '3.10'
+        cache: 'pip' # caching pip dependencies
+        # Manually set pyproject.toml as the file to use for dependencies
+        # Workaround while waiting for https://github.com/actions/setup-python/issues/529
+        cache-dependency-path: 'pyproject.toml'
+    - name: Run pre-commit
+      uses: pre-commit/action@v3.0.0
+    - uses: pre-commit-ci/lite-action@v1.0.1
+      if: always()
 
+  test:
     runs-on: ubuntu-latest
     strategy:
       fail-fast: false
@@ -32,12 +49,6 @@ jobs:
       run: |
         python -m pip install --upgrade pip
         pip install -e .[dev]
-    - name: Lint with flake8
-      run: |
-        # stop the build if there are Python syntax errors or undefined names
-        flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
-        # exit-zero treats all errors as warnings.
-        flake8 . --count --exit-zero --statistics
     - name: Test with pytest
       run: |
         python -m pytest