Skip to content

Standardize lock generation on Python 3.12 - #62

Merged
openshift-merge-bot[bot] merged 1 commit into
openstack-k8s-operators:split/pr14-docsfrom
SeanMooney:split/pr14-python312-locks
Aug 14, 2026
Merged

Standardize lock generation on Python 3.12#62
openshift-merge-bot[bot] merged 1 commit into
openstack-k8s-operators:split/pr14-docsfrom
SeanMooney:split/pr14-python312-locks

Conversation

@SeanMooney

Copy link
Copy Markdown
Contributor

Summary

  • standardize dependency generation on Python 3.12
  • pin the lock-generation tools and isolate their cache
  • remove generator headers, annotations, and package-index directives
  • verify pinned-source regeneration in GitHub Actions
  • regenerate the tracked lock files with the canonical environment

Validation

  • full tox -e update-lockfiles twice with identical checksums
  • focused reproducibility checks (4 passed)
  • existing source-update tests (22 passed)
  • Python 3.13 refusal check
  • SKIP=update-lockfiles uvx pre-commit run -a
  • bash -n build.sh
  • local Markdown link validation
  • git diff --check
  • verified legacy-cgi is absent from containers/

Dependency

Depends on #61 for the documentation layout. This PR is temporarily based on split/pr14-docs; retarget it to main after #61 merges.

Comment thread tests/test_reproducibility_architecture.py Outdated
@rebtoor

rebtoor commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

/hold

let's wait for #61 to land :)

BTW it's a +1 for me, I'll give my approval + lgtm once the patch will be rebased. (I've added a nit comment, but it's a really small thing :))

Dependency markers are evaluated by the Python interpreter running the
resolver. Allowing different Python minor versions and unpinned generator
tools therefore produces host-dependent lock files and noisy generator
metadata.

Require Python 3.12 for source and lock generation, pin the resolver tools,
and normalize annotations, headers, and index configuration from generated
locks. Add a pinned-source reproducibility workflow and architecture checks,
then regenerate the tracked lock files with the canonical environment.

Assisted-By: Pi gpt-5.6-sol
Signed-off-by: Sean Mooney <work@seanmooney.info>
@SeanMooney
SeanMooney force-pushed the split/pr14-python312-locks branch from eed4a3c to d28af26 Compare August 13, 2026 11:34
@rebtoor

rebtoor commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

/unhold

@rebtoor

rebtoor commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

@amoralej i guess this PR kinda "conflicts" with #71

@amoralej amoralej left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

#71 is superset of this one as adds clone-free reimplementation of update-lockfiles.

I can rebase 71 on this one and adapt.

@@ -1,13 +1,13 @@
name: Setup Python and tox
description: Install Python and tox (repository must already be checked out)
description: Install tox with the repository's canonical Python 3.12

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We have base_python = 3.12 in tox.ini . I assumed that would be enough to force using 3.12 but I guess forcing it also in this reusable action is also good.


- name: Verify update-sources runs successfully
run: tox -e update-sources
- name: Regenerate from committed source pins

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was proposing a slighly differen approach in #71 (https://github.com/openstack-k8s-operators/s2i-openstack-containers/pull/71/changes#diff-7971d8b59d1741371c2005851444df0d33024141e386601f9f88fdefdde1bb17) :

  • Run update-lockfiles only on the changed services (that was discussed in previous conversations when adding it to pre-commit).
  • Note git diff --exit-code -- containers/ will not monitor added files, only modifed ones, so if someone totally forgets adding the lockfiles when adding new services, this won't catch it.

Comment thread docs/developer-guide.md

```bash
STREAM=master SKIP_HASH_UPDATE=1 ./build.sh update-sources <project-or-all>
STREAM=master uvx --python 3.12 tox -e update-lockfiles -- <project-or-all>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good to add uvx usage. Won't tox force to use 3.12 as we are setting it as base_python?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not if you run it on a sytem that does not have 3.12 aviable by default.
i do not have tox installed system wide and tend to run it via uvx which is defualting ot 3.14 (which happens to be my ststems default python as well)
but i also tell my llm to use 3.13 by default when they run things so i ended up with the legacy_cgi this is partly just for me or rather future me to be explictly told use 3.12 (becuae this nomally runs on c10s) :)

Comment thread build.sh
Comment on lines +627 to +636
awk '
!seen_package && /^--(index-url|extra-index-url|trusted-host)[[:space:]]/ {
next
}
!seen_package && (/^#/ || /^$/) { next }
!/^#/ { seen_package = 1 }
{ print }
' "${lockfile}" > "${tmp}"
install -m 644 "${tmp}" "${lockfile}"
rm "${tmp}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I implemented similar in #71 by removing any empty or commented line. I'm not sure if that leaves anything out.

Comment thread build.sh
echo "--- Generating ${project_dir}/${lock_file} ---"
(cd "${project_dir}" && \
pip-compile --allow-unsafe --strip-extras \
pip-compile --allow-unsafe --no-annotate --strip-extras \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

removing annotations also in #71

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i was debating this one they are potically useful but just make everything so verbose that i tought better remove them now and if we need them we can generate them as a one of to check why a depency was pulled in

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I had the exact same debate and reach the same exact conclussion :)

Comment thread tox.ini
pre-commit run --all-files --show-diff-on-failure {posargs}

[testenv:update-sources]
description = Refresh source pins and generated lockfiles

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

@rebtoor

rebtoor commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

/approve
/lgtm

@openshift-ci

openshift-ci Bot commented Aug 14, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rebtoor

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot
openshift-merge-bot Bot merged commit 4265d58 into openstack-k8s-operators:split/pr14-docs Aug 14, 2026
5 checks passed
@SeanMooney

Copy link
Copy Markdown
Contributor Author

so this was merged into the wront brnach becase the target brnach was not deleted before and this is a stacked pr so it never updated to main
so ill have to recate this against main and check all the other ones that were mergd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants