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

Container improvements #1496

Merged
merged 4 commits into from
Feb 26, 2023
Merged

Conversation

NigelRook
Copy link
Contributor

Provides two major benefits:

  • Reduced image size to ~285Mb
  • Now compatible with alpine's py3-* packages

Has the downside that this is potentially a breaking change for some people - the final image no longer contains rust/gcc, so if anybody is running any additional requirements that need those, they'd need to add them back through system_requirements.txt. This is mitigated somewhat by the fact that they might just be able to add an alpine package for it (I did this work for scikit-learn, which takes ~20 minutes to compile on my low-power i3 server, vs 20 seconds to install the alpine package + deps). That still requires a config change for people though, so I understand if you won't want to merge this. I'd be willing to add those packages back in to this PR if you'd prefer.

@acockburn
Copy link
Member

I am worried about introducing breaking changes, especially since I don't use docker myself.

@acockburn
Copy link
Member

If you can make the change to include alpine vs compile it I think that is a good compromise

@acockburn acockburn added the question Waiting for a response on an issue or PR label Feb 19, 2023
NigelRook and others added 4 commits February 25, 2023 15:18
- Reduced image size to ~285Mb
- Now compatible with alpine's py3-* packages
Preserves backwards compatibility
@NigelRook
Copy link
Contributor Author

OK updated

For clarity, I've:

  • rebased onto latest dev
  • added all alpine packages which were present in the current dev image
  • bumped alpine to the latest stable release

Image isn't much smaller anymore, but alpine py3-* packages are still working.

@acockburn
Copy link
Member

Thanks!

@acockburn acockburn merged commit e04a41d into AppDaemon:dev Feb 26, 2023
@acockburn acockburn removed the question Waiting for a response on an issue or PR label Feb 26, 2023
@acockburn
Copy link
Member

I had to revert these commits because my docker pipeline started failing - not sure if this is the cause or not

@acockburn
Copy link
Member

If/when I fix the build, I'd like to try again with this PR but it's merged and reverted and I am not sure there is a way I can re-merge it - could I ask you to create a fresh PR for this please?

Also, it seems your theory about memory and the size of the image was correct - I managed to get a build with one image, so now I am trying for 3. If that works I will call it good and drop support for linux/arm/v6. I have no idea if that is the right one to drop as I don't know who is even using AD ;) But I guess that the oldest non-intel image is probably the least used.

@NigelRook
Copy link
Contributor Author

Here you go: #1655

Rebased and tested on my end

@acockburn
Copy link
Member

Thanks!

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