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

Improve the “update constraints” mechanism #1129

Merged
merged 4 commits into from
Nov 5, 2024

Conversation

gouttegd
Copy link
Contributor

@gouttegd gouttegd commented Nov 4, 2024

This PR builds upon #1126 to make the updating of Python constraints more robust.

The key change is that, when computing the Python constraints, we install all the required Python packages in a minimal image that is just ubuntu:24.04 (same image as the base used to build the ODK) in which we install nothing else than python3-dev and python3-pip.

Previously, we were installing the Python packages in an image that was the ODK itself (obolibrary/odkfull:latest), which was problematic for at least two reasons:

  • The final ODK image already contains (obviously) all the Python packages we want. In theory this should not cause any problem because when computing the Python constraints we are working in a virtual environment, but for some packages (at least setuptools and wheels, and possibly others) this is not enough. Using a minimal image ensures that we are always computing the Python constraints from a clean state.
  • This cannot work when we need to reset the Python constraints after updating the base image itself (e.g., when we will upgrade to Ubuntu 26.04), because of a chicken-and-egg problem: we need the new obolibrary/odkfull:latest (based on the updated Ubuntu image) to compute the new Python constraints, but we need the new Python constraints to build the new obolibrary/odkfull:latest. Previously, we were working around that issue by having a separate, undocumented process specifically to update the Python constraints after a base image upgrade. Now we will be able to use exactly the same process (same update-constraints.sh script) both for the regular updates of the Python constraints, and for the “big updates” after a migration to a new base image.

In addition, when building the ODK itself (more precisely, the odkbuilder image), we check that the list of system-managed Python packages matches what we are expecting (what was determined by the update-constraints.sh script, and stored in pip-constraints.txt), and we log a warning if there is a discrepancy (which may happen if a Ubuntu-provided Python package has been updated, e.g. for security reasons, since the last refresh of the constraints -- something that should be unlikely to happen often, but just in case).

To compute the set of Python constraints, we install all the Python
packages we need in a bare Ubuntu image (same version as the one used to
build the ODK itself) in which we only install Python3 and PIP.

This ensure that `pip-constraints.txt`, the list of Python packages that
are installed by the system and that PIP should not touch no matter
what, always reflect the actual list of system-managed Python packages
found in the ODK.

This has the side-effect of allowing to use the `update-constraints.sh`
script for the specific case of bootstrapping the constraints when the
base image is upgraded to a newer version of Ubuntu, something that
previously necessitated an undocumented manual operation.
When building the ODK builder image, before attempting to install the
Python packages, we obtain the list of packages that have already been
installed by the system (i.e. by APT) and compare it with the
pre-computed list in `pip-constraints.txt`.

The two lists should normally always be the same. They MAY differ if,
between the time the constraints were last updated and the current
build, Ubuntu updated some of its Python packages (e.g. if they bump
`python3-setuptools` to a more recent minor version because of a
security vulnerability). If that happens, we allow the build to continue
(it may not necessarily cause a fatal error), but we log a warning so
that, should the build fail, we will know that we likely need to refresh
the constraints to reflect the updated Ubuntu packages.
@gouttegd gouttegd self-assigned this Nov 4, 2024
@gouttegd gouttegd mentioned this pull request Nov 4, 2024
@gouttegd
Copy link
Contributor Author

gouttegd commented Nov 4, 2024

And another run of the test suite, yeah!

Copy link
Contributor

@matentzn matentzn left a comment

Choose a reason for hiding this comment

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

I know you have not asked yet, but I have spent 20 min or so reviewing this (I admit, update-constraints.sh with the help of chatgpt to explain some things), and it all looks good to me!

@gouttegd
Copy link
Contributor Author

gouttegd commented Nov 4, 2024

I know you have not asked yet

As a general rule, I wait for the test suite to have passed before asking for a review. :)

(Officially to avoid wasting people’s time, by not asking them to review something that may need to be modified because the tests failed; in reality, to preserve the chance that, if I make a stupid mistake – the kind of mistake that the test suite will catch immediately –, I can fix it before anyone else can see it. :D )

@gouttegd
Copy link
Contributor Author

gouttegd commented Nov 4, 2024

update-constraints.sh with the help of chatgpt to explain some things

I am mildly curious of what in the update-constraints.sh script requires ChatGPT to be understood. O_o

@matentzn
Copy link
Contributor

matentzn commented Nov 4, 2024

I am mildly curious of what in the update-constraints.sh script requires ChatGPT to be understood. O_o

  1. I have never heard of PIP_CONSTRAINT as an environment variable
  2. I was too lazy to read https://github.com/INCATools/ontology-development-kit/pull/1129/files#diff-151c918f71ba2bcb208cea5edafa9abecc62d8b466ad400717ede5a0ee7e112dR18 until I realised you had already explained it to me.

@gouttegd gouttegd merged commit 43dce16 into master Nov 5, 2024
1 check passed
@gouttegd gouttegd deleted the improve-update-constraints branch November 5, 2024 00:16
@gouttegd
Copy link
Contributor Author

gouttegd commented Nov 5, 2024

I have never heard of PIP_CONSTRAINT as an environment variable

To be fair I think that Pip’s documentation is quite lacking here.

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