Repository navigation
fix(deps): admit Python 3.13 for opal-common and opal-client - #960
Merged
Merged
Conversation
✅ Deploy Preview for opal-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
The `<3.13` ceiling came from PR #924 (f6c3d83, 2026-08-18), not #959, and has already shipped in 0.9.9. Its stated basis was the CPython `concurrent.futures.thread` internals that opal-server's `_DaemonThreadPoolExecutor` mirrors, and "opal-client capped to match opal-common". Both parts were re-checked on 3.13.14: - the internals mirror in opal_server/git_fetcher.py takes its guarded path on 3.13.14 (signature and attribute shape identical to 3.11), so the original rationale no longer holds on its own; - the only thing that actually fails to install on 3.13 is opal-server's `pygit2>=1.14.1,<1.15` - no cp313 wheel exists before pygit2 1.16.0, and the 1.14.x sdist needs libgit2 1.7 headers. That dependency is opal-server's alone; neither opal-common nor opal-client uses it. A full resolve of the shared + opal-common + opal-client requirements on 3.13 finds no other blocker (every pin has a 3.13-usable wheel), and all three packages pass their suites on 3.13.14 - 770 passed - when opal-server is given a cp313 pygit2. So the ceiling is metadata for the two packages that don't pull pygit2, and real for the one that does. This relaxes opal-common and opal-client to `<3.14` and adds the 3.13 classifier. opal-server stays at `<3.13` until pygit2 is bumped, which is a behaviour change (the PR3 timeout comments reason about the pinned libgit2) and belongs in its own PR. The two must move together: opal-client pins opal-common by exact version. 3.14 is untested, hence the cap. Why now: Python 3.13 has been the current stable release since October 2024, and downstream projects have moved to it - often for CPython security fixes with no 3.12 backport. pip refuses both packages on it outright ("requires a different Python: 3.13.14 not in '<3.13,>=3.10'") for a cap whose stated reason no longer applies and whose real cause lives in opal-server alone. A project that embeds opal-client on 3.13 has no path to any 0.10.x release until this moves. CI gets a 3.13 leg that installs only the two relaxed packages, WITH PIP - deliberately. uv does not enforce Requires-Python for local path sources and would have passed this leg even with the old `<3.13` metadata; pip is what the Dockerfiles and this workflow use, and it does enforce it. Three test modules that need an importable opal-server are ignored on that leg with the reason stated inline. `enum_inventory_test.py` needed to cope with an absent package rather than fail collection: a swept package whose distribution is not installed is recorded in NOT_INSTALLED, distinct from IMPORT_FAILURES (installed but broken - still a failure). A new guard test asserts nothing importable can ever land in NOT_INSTALLED and that every swept package is either walked or recorded absent, so the absent-package path cannot become a quiet way to shrink the sweep. On a full install NOT_INSTALLED is empty; on the 3.13 leg it is exactly `opal_server.scopes`. Verified: - 3.13.14, common+client only, pip-installed, as CI will run it: 366 passed - 3.11 full install, whole suite: 770 passed, 1 skipped - flake8 gate clean, pre-commit clean on all files - opal-server/setup.py untouched Also surfaced, not fixed here: `opal_common/sources/api_policy_source.py:22` imports `opal_server.config` at module level, so opal-common installed alone cannot import that module. Pre-existing, unrelated to this change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Zivxx
force-pushed
the
ziv/per-15885-relax-py313-ceiling
branch
from
September 22, 2026 13:15
8848f53 to
99b47d1
Compare
zeevmoney
approved these changes
Sep 22, 2026
zeevmoney
left a comment
zeevmoney
left a comment
Member
There was a problem hiding this comment.
Approved — no CRITICAL or HIGH issues found.
The core argument was verified rather than taken on trust: the <3.13 cap traces to #924 (f6c3d831, 2026-08-18, shipped in 0.9.9); _DaemonThreadPoolExecutor exists only in opal-server; pygit2>=1.14.1,<1.15 appears only in packages/opal-server/requires.txt and 1.16.0 is indeed the first release with cp313 wheels; uv pip compile for 3.13 over the shared + common + client requirements resolves to exactly 72 pins; and the new leg ran green in CI (366 passed on 3.13.15).
Non-blocking:
- MEDIUM
packages/opal-common/opal_common/tests/wire_compat/enum_inventory_test.py:204— the docstring states an invariant (NOT_INSTALLEDempty on a full install) that nothing asserts; the skip is unconditional on every leg, so any environment missing opal-server shrinks the sweep silently and still reports green - MEDIUM
packages/opal-common/setup.py:78— "that guard is verified to pass on 3.13" rests on a one-off manual check that no CI can maintain, since opal-server cannot install on 3.13 and its guard tests never run there - MEDIUM
.github/workflows/tests.yml:58— dead./tests/requirements.txtconditional carried forward into the rewritten install block; that file does not exist - LOW
packages/opal-client/setup.py:77— the "must move together" rationale is contradicted bypackages/opal-server/setup.py:48, which carries the identical pin and stays at<3.13; the distinguishing reason is pygit2, not the pin
Details are in the inline comments on each line.
Outside this diff, not blocking:
pip install opal-serverfrom PyPI on 3.13 silently resolves to 0.9.6 rather than erroring, because 0.9.9 declares<3.13. Pre-existing and unchanged here, but it is the user-facing shape of the split this PR formalizes.enum_inventory_test.py:256and:287-289— two furtherimport_modulecall sites were not taught about absent packages. Latent today (noopal_server.*entry in the corpus orNOT_ON_THE_WIRE), but they would turn into a whole-file collection error, or a misreported "models that no longer exist", the moment one is added.documentation/docs/tutorials/install_as_python_packages.mdx:10anddocumentation/docs/getting-started/running-opal/as-python-package/overview.mdx:9still state the ceiling as "below 3.13" and do not describe the new split.zizmorreports 14unpinned-usesfindings in this workflow, against the repo convention of SHA-pinning actions with a version comment. All on lines this PR did not touch.
Description accuracy: the claims checked out, with three exceptions worth a pass — "this can't become a quiet way for the sweep to shrink" overstates what the new guard enforces (see the first comment above); "Python 3.13 has been the current stable release since October 2024" is stale, since 3.14 shipped in October 2025; and the leg is reported as run on 3.13.14 while CI resolved 3.13.15 (the 366 count matches either way).
… gap
`test_sweep_skips_only_genuinely_absent_packages` checked that a package
recorded in NOT_INSTALLED was truly absent, but never that it was ALLOWED
to be absent. So an environment missing opal-client would drop three swept
packages and still report 14 passed - exactly the silent shrink the
NOT_INSTALLED mechanism was added to prevent, and which the docstring
claimed was closed.
Add ALLOWED_ABSENT = {"opal_server.scopes"} - the one package the CI 3.13
leg is designed to omit - and assert every absent entry is in it. Rewrite
the docstring to describe what is now enforced. Verified by uninstalling
opal-client from a 3.13 environment: the guard now fails naming
opal_client.engine, opal_client.policy_store and opal_client.policy, where
before it passed.
Also teach the two remaining import_module sites about absent
distributions: `_covered_by_corpus` would have turned a future
opal_server.* corpus case into a whole-file collection error on the 3.13
leg, and `test_exclusions_still_exist` would have reported an
opal_server.* exclusion as "no longer exists". Both now skip a known-absent
distribution; the full-install legs still check it.
Addresses review comments:
- #960 (comment) (@zeevmoney)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`./tests/requirements.txt` has never existed in this repo, so the line was a no-op that suggested a test-requirements file exists. It was carried into the rewritten install block instead of being removed. `requirements.txt` does exist, so its guard goes too. Addresses review comments: - #960 (comment) (@zeevmoney) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…move Two problems with the python_requires comments. opal-common's said the executor guard "is verified to pass on 3.13". That rests on a one-off manual check: the guard and its tests live in opal-server, which cannot install on 3.13, so no CI leg can maintain the claim. Drop it as justification and lean on what is durable - this package depends on neither the executor nor pygit2, and the CI 3.13 leg installs it with pip, which enforces the field. The manual check is attributed to #960 as not CI-covered. Also note that on 3.14 the mirrored internals have already changed shape (`_worker` takes 3 parameters; `_initializer` and `_initargs` are gone), which is part of why <3.14 is the right cap. opal-client's said the two "must move together" because it pins opal-common by exact version - but opal-server carries the identical pin and stays at <3.13, so read literally the rule forbids what this PR does. Say what actually distinguishes them: no pygit2 dependency. Addresses review comments: - #960 (comment) (@zeevmoney) - #960 (comment) (@zeevmoney) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Both python-package guides still said "3.10 or greater (and below 3.13)", which is now wrong for opal-client and opal-common and does not describe the split. State it per package, and warn about the footgun the split creates: on 3.13 `pip install opal-server` does not fail, it silently resolves to 0.9.6 - the newest release whose metadata admits 3.13, since 0.9.7 and 0.9.8 were never published to PyPI and 0.9.9 declares <3.13. Addresses review comments: - #960 (review) (@zeevmoney) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Author
|
@zeevmoney thanks for the pass — all four inline findings are addressed (4d0ee2a, 9ecec06, a3fd405); the threads have the details. On the outside-diff items:
|
dshoen619
added a commit
to permitio/PDP
that referenced
this pull request
Sep 23, 2026
…xit plan (PER-16234) Review follow-ups on the post-resolve aiofiles upgrade: - Tripwire: fail the build unless opal-client is 0.9.6. --no-deps also silences pip's conflict report, so a bumped opal-client would otherwise silently keep getting aiofiles 24.1.0, whatever it declares. - Guard comment: it smoke-tests the named-temp-file wrapper that broke, not "the exact call backup_store() makes" (that also passes delete=False/dir=/suffix=, then writes and os.replace()s). Keep the guard when the override goes: the next opal-client bump is when the resolver may start picking a newer aiofiles. - Exit plan: permitio/opal#960 shipped in opal-client 0.10.0rc1, which still caps aiofiles<1 and requires pydantic>=2.9 and starlette>=1.3.1, ruled out by our pydantic<2 and starlette==0.50.0 pins. Removal is gated on horizon moving to pydantic v2 (plus the aiofiles widening landing in opal) or a pydantic-v1 0.9.x backport, not on a pin bump. Verified: the edited pip layer builds on python:3.13-alpine3.23; the tripwire exits 1 with its message on a simulated pin drift; in the final layer (pip removed) the real OpalClient.backup_store() writes the backup with no *.json.tmp left over. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dshoen619
added a commit
to permitio/PDP
that referenced
this pull request
Sep 27, 2026
…de backups work on Python 3.13 (PER-16234) (#343) * fix(docker): upgrade aiofiles past opal-client's <1 cap so offline-mode backups work on Python 3.13 (PER-16234) opal-client 0.9.6 declares aiofiles>=0.8.0,<1, which under PEP 440 resolves only to aiofiles 0.8.0 (Nov 2021). Its tempfile shim raises AttributeError on CPython >= 3.12, so since the python:3.13 base bump in 0.9.14 (#329) OPAL's backup_store() fails on every STORE_BACKUP_INTERVAL tick when PDP_ENABLE_OFFLINE_MODE=true: the exception is swallowed, the PDP keeps serving, but no policy-store backup is ever written and a 0-byte *.json.tmp leaks into /app/backup per tick. pip cannot loosen an installed package's cap from requirements.txt (ResolutionImpossible), so upgrade aiofiles to 24.1.0 after the resolve, in the same layer, before pip is removed. Add a build-time guard that makes the same aiofiles call on the shipped interpreter, so a regression fails the image build in tests.yml and release.yml instead of surfacing as a log line after boot. Document next to the opal-client pin why an aiofiles line must not be added to requirements.txt. Verified: the guard fails a local build of this layer on aiofiles 0.8.0 with the exact traceback and passes on 24.1.0; overlaying 24.1.0 onto the published v0.9.15 image makes the real backup_store() write the backup. Fixes #340 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(docker): add opal-client tripwire to aiofiles override, correct exit plan (PER-16234) Review follow-ups on the post-resolve aiofiles upgrade: - Tripwire: fail the build unless opal-client is 0.9.6. --no-deps also silences pip's conflict report, so a bumped opal-client would otherwise silently keep getting aiofiles 24.1.0, whatever it declares. - Guard comment: it smoke-tests the named-temp-file wrapper that broke, not "the exact call backup_store() makes" (that also passes delete=False/dir=/suffix=, then writes and os.replace()s). Keep the guard when the override goes: the next opal-client bump is when the resolver may start picking a newer aiofiles. - Exit plan: permitio/opal#960 shipped in opal-client 0.10.0rc1, which still caps aiofiles<1 and requires pydantic>=2.9 and starlette>=1.3.1, ruled out by our pydantic<2 and starlette==0.50.0 pins. Removal is gated on horizon moving to pydantic v2 (plus the aiofiles widening landing in opal) or a pydantic-v1 0.9.x backport, not on a pin bump. Verified: the edited pip layer builds on python:3.13-alpine3.23; the tripwire exits 1 with its message on a simulated pin drift; in the final layer (pip removed) the real OpalClient.backup_store() writes the backup with no *.json.tmp left over. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(docker): share the aiofiles override with CI and test OPAL's backup_store (PER-16234) Addresses review on #343: - requirements-override.txt: aiofiles==24.1.0 with its PyPI sha256, installed post-resolve with `pip install --no-deps --require-hashes` by the Dockerfile pip layer, the pytests job and the README dev setup, so CI tests the aiofiles that ships. It is now the one place holding the rationale and exit note (previously spread across the Dockerfile, requirements.txt and the PR description). - horizon/tests/test_offline_backup.py: runs OPAL's real backup_store() with a stub policy store and asserts on disk state: backup written, stale backup replaced, missing backup dir created, no *.json.tmp left. - check_aiofiles_override.py: bind-mounted into the pip layer (never shipped) and run last, replacing the two python -c one-liners. Fails the build if opal-client leaves 0.9.6 or the real backup_store() round trip fails in the image. - Drop --upgrade (no-op next to ==). Correct two comments: the aiofiles context-manager change landed in 24.1.0, not 25.x; the backup is stale (a pre-0.9.14 one stays and is loaded offline), not missing. No new Dockerfile line exceeds 100 chars. - test_offline_mode: README uses `make prepare` and `docker compose up --build` and lists the expected results; .env.example sets OPAL_STORE_BACKUP_INTERVAL=5 so the first backup exists before the offline PDP starts. Verified on CPython 3.13, set up as the pytests job is: new tests 3 failed on aiofiles 0.8.0, 3 passed with the override; full horizon suite 258 passed. Exact pip layer on python:3.13-alpine3.23: builds; fails at the check without the override; pin mismatch and a wrong hash are rejected; the script is not in the image. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
7 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Relaxes
python_requiresfrom<3.13to<3.14on opal-common and opal-client only, adds the 3.13 classifier, adds a 3.13 leg to CI that actually exercises it, and states the resulting per-package ceiling in the install guides. opal-server stays at<3.13.Follows #959, which merged as
904b5fdb; this is a small stack on top of it.Why the ceiling can move — verified, not assumed
The
<3.13cap is from #924 (f6c3d83, 2026-08-18) and already shipped in 0.9.9; #959 left it alone. #924 gave two reasons, and both were re-checked:_DaemonThreadPoolExecutormirrors CPythonconcurrent.futures.threadinternals that might change_workersignature and every mirrored attribute are identical to 3.11. That is a one-off manual check, not CI-covered (opal-server cannot install on 3.13, so its guard tests never run there); it shows the stated rationale no longer binds, but the durable reason is the next row. On 3.14 those internals have already moved (_workeris 3-parameter;_initializer/_initargsare gone), so the guard would fall back to stdlib non-daemon threads there — part of why the cap is<3.14.opal-common=={version}and has no pygit2 dependency, so both can move. opal-server carries the identical pin and stays — the pin is not what distinguishes them; pygit2 is.The only real 3.13 blocker is opal-server's
pygit2>=1.14.1,<1.15. No cp313 wheel exists before pygit2 1.16.0, and the 1.14.x sdist needs libgit2 1.7 headers (brew's 1.9.4 fails:call to undeclared function 'git_error_set'). A fulluv pip compileof the shared + opal-common + opal-client requirements on 3.13 resolves with zero blockers — every one of the 72 pins has a 3.13-usable wheel. With opal-server given a cp313 pygit2, all three packages pass on 3.13.14: 770 passed.So for the two packages that don't pull pygit2, the ceiling was metadata. This PR removes it for exactly those two.
Why now
Python 3.13 shipped in October 2024 (3.14 followed in October 2025), and a growing share of downstream projects have moved to it — often specifically for CPython security fixes that were not backported to 3.12. Anyone on it gets a hard refusal from pip for both packages today:
— for a cap whose stated reason no longer applies, and whose real cause lives in a package (
opal-server) they may not even install. A project that embedsopal-clientand runs on 3.13 has no path to any 0.10.x release until this moves.The CI leg — and why it uses pip
The new 3.13 matrix entry installs only opal-common + opal-client (opal-server can't install there — see pygit2 above) and ignores the three test modules that need an importable server, with the reason inline.
It installs with pip, deliberately:
uv pip installdoes not enforceRequires-Pythonfor local path sources and would have passed this leg even with the old<3.13metadata. pip is what the Dockerfiles and this workflow use, and pip does enforce it — so this leg is a real check on the metadata, not a false pass.Local run of that leg: 366 passed on 3.13.14. CI's leg resolved 3.13.15 and reports the same 366.
enum_inventory_test.py— handling an absent package honestlyThat sweep walks
opal_server.scopes, which isn't installed on the 3.13 leg. Rather than ignore the whole test file (it covers the two packages this PR relaxes), an absent distribution is recorded inNOT_INSTALLED— distinct fromIMPORT_FAILURES, which stays a hard failure for a package that is installed but broken.A guard test asserts three things: nothing importable can land in
NOT_INSTALLED; every absent entry is inALLOWED_ABSENT = {"opal_server.scopes"}— the one package the leg is designed to omit; and every swept package is either walked or recorded absent. So on a full install the list is empty, on the 3.13 leg it is exactlyopal_server.scopes, and an environment missing anything else fails instead of passing over a smaller sweep — verified by uninstalling opal-client from a 3.13 env, which fails the guard naming the threeopal_client.*packages. The file's two otherimport_modulesites skip a known-absent distribution too, instead of turning into a collection error or a false "model no longer exists".Docs
The two python-package install guides now state the ceiling per package, and warn that on 3.13
pip install opal-serverdoes not error — it silently resolves to 0.9.6, the newest release whose metadata admits 3.13 (0.9.7 and 0.9.8 were never published to PyPI; 0.9.9 declares<3.13).Not in this PR
pygit2>=1.16, which also means libgit2 ≥1.8 semantics — the PR3 fetch-timeout comments reason about the pinned libgit2, so that's a behaviour change for its own PR. That PR should also revisit_DaemonThreadPoolExecutorfor 3.14, where the mirrored internals have already changed shape (see the table above).<3.14and not higher.opal_common/sources/api_policy_source.py:22importsopal_server.configat module level, sopip install opal-commonalone can't import that module. Pre-existing layering issue, surfaced while building the leg; unrelated to this change.tests.yml,pre-commit.ymlandon_release.ymlall pin actions by tag; onlydocs-build.ymlpins by SHA. A sweep for its own PR.Verification
enum_inventory_test.pynegative check: opal-client uninstalled from the 3.13 env → guard fails; reinstalled → 14 passedpre-commit run --all-filescleanopal-server/setup.pyuntouched🤖 Generated with Claude Code