Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI: Optimize docker build #1669

Merged
merged 7 commits into from
Mar 1, 2023

Conversation

mion00
Copy link
Contributor

@mion00 mion00 commented Mar 1, 2023

This PR is built on top of #1655, augmenting it with the new project setup (pyproject.toml instead of requirements.txt) and CI pipeline.

Description

  • Always build the Docker image and Python package (pushing only with tag or dev branch) as part of the Github pipeline.
    This way we can easily catch any problem in the build process, without having to first merge the changes inside dev.
  • Use Python package built during the build_package job for building the Docker image. This way we avoid having to copy the entire project source inside the image, reducing the image size.
    At the same time this process "mimics" the step an end-user of the package does (a pip install), so we can verify that the package can indeed install correctly.
  • Optimize the docker build cache by using RUN --mount=type=cache to speed up the process between subsequent builds.
  • Small fix to the pyproject.toml: exclude the static assets inside docs. folder, otherwise they get included as part of the Python package, unnecessarily bloating it

@mion00 mion00 mentioned this pull request Mar 1, 2023
@acockburn
Copy link
Member

The build failed with the new docker file:

https://github.com/AppDaemon/appdaemon/actions/runs/4303953648/jobs/7504338740

@mion00
Copy link
Contributor Author

mion00 commented Mar 1, 2023

Yeah the changes from #1655 where not exactly up-to-date with the state of the projects, for instance the use of requirements.txt breaks the build.
I am working on a fix in this PR.

@acockburn acockburn merged commit 928a2e2 into AppDaemon:dev Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants