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

Fix support for dependency version clauses for certain Python dependencies in Docker images #420

Merged
merged 5 commits into from
Sep 12, 2023

Conversation

robertbartel
Copy link
Contributor

@robertbartel robertbartel commented Sep 7, 2023

We have to be specific regarding certain Python dependencies - pandas for example. Additionally, a subset of such packages also each take a long time to install within our Docker images, leading us to have set up the images/stages to build wheels for these packages in advance and in isolated build stages.

There were Dockerfile ARG elements set up to control the version clause of these packages if desired, but nothing in place to actually supply values to those. These changes address that by updating the stack compose config file to supply those ARGs from the .env config.

Also added are two other .env-controlled flags. One regards whether the aforementioned version clause ARGs should override an explicit version clause within the project-level requirements.txt file, which is also used by the builds. The other regards whether a mismatch between .env-provided and requirements.txt-provided version clauses (when both are explicitly provided) should result in an error and stop the build.

Passing version clauses for certain dependency packages into build from
.env configuration file via compose config.
Fixing version clause overrides for numpy, pandas, etc.
@robertbartel robertbartel added bug Something isn't working maas MaaS Workstream labels Sep 7, 2023
@robertbartel robertbartel marked this pull request as draft September 7, 2023 15:41
Updating with additional steps to (optionally) ensure that an explicit
version clause, if provided on both sides, matches between .env and
requirements.txt.
Copy link
Member

@aaraney aaraney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! The PR documentation made this really easy to review. Thanks as always, @robertbartel. Merge at will!

@robertbartel robertbartel merged commit b7dbfc4 into NOAA-OWP:master Sep 12, 2023
1 check passed
@robertbartel robertbartel deleted the b/docker_py_pack_vers/main branch September 12, 2023 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working maas MaaS Workstream
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants