ci: test the declared dependency lower bounds and the install surface - #1319
Open
ogenstad wants to merge 2 commits into
Open
ci: test the declared dependency lower bounds and the install surface#1319ogenstad wants to merge 2 commits into
ogenstad wants to merge 2 commits into
Conversation
The unit-test matrix installs exactly what uv.lock pins, so the version ranges in pyproject.toml have never been exercised. Two jobs now cover the parts a pull request can be held to. dependency-lower-bounds re-resolves at the declared floors and runs the unit tests. It runs on 3.10 alone: the resolver picks the same floors on every supported version, but pyarrow 14, pyyaml 6.0 and ujson 5.0 ship no wheels past cp312, so a newer interpreter would compile them from source and fail for reasons unrelated to this code. A floor states the oldest version validated, not one installable everywhere. install-matrix builds the wheel and confirms it installs, imports every shipped module and runs the CLI on all five supported interpreters, resolving fresh rather than from the lock. That is the half the existing matrix cannot see, since it proves the pinned set works rather than what a user actually gets. Writing the first job found two floors that were wrong. typer 0.15 crashes on any `--help` with the click 8.3 this package already required, because click made `Parameter.make_metavar()` take a ctx and typer only followed in 0.16, so the old floor advertised a combination that cannot work. Jinja2 before 3.1.5 attributes a nested undefined variable to the wrong template and drops the source path from a missing-import error. Five development requirements had no lower bound at all, which made the floor resolution fail outright: yamllint reached back to 0.1.0, whose sdist does not build. Their bounds carry no promise to users, they only need to keep the resolution sane, so they are set near what the lock already holds. Upper bounds are deliberately absent. Any upstream release could turn such a job red without a line here changing, and blocking an unrelated pull request on that is worse than not knowing.
ogenstad
added
group/ci
Deploying infrahub-sdk-python with
|
| Latest commit: |
bdfca3d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a96fb884.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pog-dependency-bounds-ci-ihs.infrahub-sdk-python.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests.
@@ Coverage Diff @@
## infrahub-develop #1319 +/- ##
====================================================
- Coverage 85.33% 79.69% -5.65%
====================================================
Files 148 148
Lines 14085 14085
Branches 1936 1936
====================================================
- Hits 12020 11225 -795
- Misses 1499 2306 +807
+ Partials 566 554 -12
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/ci.yml">
<violation number="1" location=".github/workflows/ci.yml:316">
P2: According to linked Jira issue IHS-224, this support matrix also requires a scheduled upper-bound check. Add the non-gating Python 3.14 re-resolution job and failure tracking instead of omitting upper-bound coverage entirely.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| # ------------------------------------------ Dependency Bounds ------------------------------------------ | ||
| # `unit-tests` above installs exactly what uv.lock pins, so it never exercises the version ranges | ||
| # declared in pyproject.toml. The two jobs below cover the parts of that gap that a pull request | ||
| # can be held to. Upper bounds are deliberately absent: any upstream release could turn such a job |
Contributor
There was a problem hiding this comment.
P2: According to linked Jira issue IHS-224, this support matrix also requires a scheduled upper-bound check. Add the non-gating Python 3.14 re-resolution job and failure tracking instead of omitting upper-bound coverage entirely.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yml, line 316:
<comment>According to linked Jira issue IHS-224, this support matrix also requires a scheduled upper-bound check. Add the non-gating Python 3.14 re-resolution job and failure tracking instead of omitting upper-bound coverage entirely.</comment>
<file context>
@@ -310,6 +310,127 @@ jobs:
+ # ------------------------------------------ Dependency Bounds ------------------------------------------
+ # `unit-tests` above installs exactly what uv.lock pins, so it never exercises the version ranges
+ # declared in pyproject.toml. The two jobs below cover the parts of that gap that a pull request
+ # can be held to. Upper bounds are deliberately absent: any upstream release could turn such a job
+ # red without a line of our code changing, which is not something a PR should be blocked on.
+
</file context>
gmazoyer
approved these changes
Sep 3, 2026
Comment on lines
+340
to
+348
| # Pinned to the oldest interpreter we support, because that is where floor-era wheels exist. | ||
| # The resolver selects the same floors on every supported version, but installing them does | ||
| # not work everywhere: pyarrow 14, pyyaml 6.0 and ujson 5.0 ship no wheels past cp312, so on | ||
| # 3.13 or 3.14 this job would compile them from source and fail for reasons unrelated to our | ||
| # code. A matrix would therefore add nothing. A floor states the oldest version we have | ||
| # validated, not one installable on every interpreter; a newer interpreter simply resolves | ||
| # higher, which `install-matrix` below confirms still works. | ||
| # Bump this when the oldest supported version changes. If it goes stale the job fails loudly, | ||
| # because `requires-python` will reject the interpreter. |
Contributor
There was a problem hiding this comment.
bot's probably overdone that comment
…d floors The import check ran from the checkout root, where `python -` puts the working directory on sys.path, so it imported the source tree and never touched the wheel it was meant to be testing. It now runs from the runner temp directory. Confirmed: from the checkout it resolves infrahub_sdk to the repository, from elsewhere to site-packages. Adds dependency-lower-bounds-no-harness, the only place the floors masked by the all-groups job are reachable. With no dependency groups installed, pytest resolves to 7.0.0 rather than 9.0.0, pydantic to 2.0.3 rather than 2.10.6, anyio to 3.3.0 rather than 4.4.0, and typing-extensions to 4.6.1 rather than 4.12.2. That validates the pydantic floor corrected two releases ago, which nothing has exercised until now. httpx and graphql-core stay out of reach even here, because ariadne-codegen requires httpx>=0.28 and graphql-core>=3.2. It resolves from pyproject.toml rather than from the built wheel because `--resolution lowest-direct` only lowers what is named on the command line; asking for the wheel makes its dependencies transitive and leaves them newest. That was measured, not assumed. Without the harness it cannot run the unit suite, so it exercises the surface a user touches: every module imports, the CLI runs, and the bundled plugin collects a test from a generated repository config. The plugin check invokes pytest rather than importing the plugin, since pytest loads it through its entry point.
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/ci.yml">
<violation number="1" location=".github/workflows/ci.yml:542">
P3: GitHub Actions runs `run:` with `bash -e` but not `-o pipefail`, so in `python -m pytest ... | tee collected.txt` the pipeline's exit status is `tee`'s (0). A pytest collection failure therefore doesn't abort the step; it is only caught indirectly when the following `grep -q` finds no collected nodeid and fails, hiding the real pytest traceback behind an unhelpful grep error. Add a `PIPESTATUS` check (or `set -o pipefail`) so genuine collection failures surface directly.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| kind: "jinja2-transform-smoke" | ||
| YAML | ||
| printf 'router bgp {{ asn }}\n' > templates/bgp_config.j2 | ||
| ${{ github.workspace }}/.venv-floors/bin/python -m pytest \ |
Contributor
There was a problem hiding this comment.
P3: GitHub Actions runs run: with bash -e but not -o pipefail, so in python -m pytest ... | tee collected.txt the pipeline's exit status is tee's (0). A pytest collection failure therefore doesn't abort the step; it is only caught indirectly when the following grep -q finds no collected nodeid and fails, hiding the real pytest traceback behind an unhelpful grep error. Add a PIPESTATUS check (or set -o pipefail) so genuine collection failures surface directly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yml, line 542:
<comment>GitHub Actions runs `run:` with `bash -e` but not `-o pipefail`, so in `python -m pytest ... | tee collected.txt` the pipeline's exit status is `tee`'s (0). A pytest collection failure therefore doesn't abort the step; it is only caught indirectly when the following `grep -q` finds no collected nodeid and fails, hiding the real pytest traceback behind an unhelpful grep error. Add a `PIPESTATUS` check (or `set -o pipefail`) so genuine collection failures surface directly.</comment>
<file context>
@@ -431,6 +436,114 @@ jobs:
+ kind: "jinja2-transform-smoke"
+ YAML
+ printf 'router bgp {{ asn }}\n' > templates/bgp_config.j2
+ ${{ github.workspace }}/.venv-floors/bin/python -m pytest \
+ --infrahub-repo-config=infrahub_config.yml --strict-markers --collect-only -q \
+ | tee collected.txt
</file context>
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
2 participants
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.
Why
The
unit-testsmatrix runs five Python versions but every job installs exactly whatuv.lockpins, so the version ranges declared inpyproject.tomlhave never been exercised by anything. This adds the jobs that hold a pull request to them.Goal: the declared lower bounds are correct, and the published package installs on every supported interpreter.
Non-goals: upper bounds. Any upstream release could turn such a job red without a line of this code changing, and blocking an unrelated pull request on that is worse than not knowing. Tracked separately.
Ref IHS-224.
What changed
Two jobs:
dependency-lower-boundsre-resolves at the declared floors and runs the unit tests. Gates pull requests, because a floor only moves whenpyproject.tomldoes.install-matrixbuilds the wheel and confirms it installs, imports every shipped module and runs the CLI on all five supported interpreters, resolving fresh rather than from the lock. This is the half the existing matrix cannot see: it proves the pinned set works, not what a user actually gets from a plain install.Writing the first job immediately found two wrong floors:
typer>=0.15.0was impossible. Combined with theclick>=8.3this package already required, typer 0.15 crashes on anyinfrahubctl --helpwithTypeError: Parameter.make_metavar() missing 1 required positional argument: 'ctx'. click 8.3 made that argument mandatory and typer only followed in 0.16. Verified by holding typer at 0.15.0 and moving click:--helpSo
>=0.16.0is not a narrowing, it states a constraint that already existed implicitly. Note that widening typer support means lowering theclickfloor, not this one; typer 0.15.4 additionally capsclick<8.2, so the compatible set is not a simple range.Jinja2>=3was wrong. Before 3.1.5, template error reporting attributes a nested undefined variable to the wrong template and drops the source path from a missing-import error. Bisected: 3.1.4 fails, 3.1.5 passes.Five development requirements had no lower bound at all, which made the floor resolution fail outright rather than merely resolve oddly:
yamllintreached back to 0.1.0, whose sdist does not build. Bounds on development dependencies carry no promise to users; they only need to keep the resolution sane, so they are set near what the lock already holds.How to review
Start with the two jobs in
.github/workflows/ci.yml, then the floor changes inpyproject.toml. The reasoning that most deserves scrutiny is in the comments ondependency-lower-bounds, covering why it is a single job on 3.10 rather than a matrix, and what it cannot reach.Two limits stated there and worth repeating:
How to test
Verified locally before and after the floor corrections:
The failures common to the floor and control runs are artifacts of a scratch virtualenv, not of the floors. After the corrections, the floor run's failure set is a strict subset of the control's, so no floor-specific failure remains. The 11 that were floor-specific split cleanly into the two clusters above, 7 typer and 4 Jinja2.
Also verified: a fresh default-resolution install of
[all]succeeds and imports every shipped module on 3.10, 3.12 and 3.14; ruff, ty and mypy clean;yamllint -s .clean; unit suite 1859 passed at lock versions with the 2 known macOS temp-path failures; lock consistent.Impact & rollout
dependency-lower-boundsis intended as a required check. It is deterministic, so it will not flap.Checklist
Summary by cubic
Adds three CI jobs that exercise the declared dependency ranges in
pyproject.toml, which the unit-test matrix never covered because it installs exactly whatuv.lockpins. The lower-bound jobs immediately exposed two wrong floors, now corrected:typer>=0.16.0(0.15 crashes on anyinfrahubctl --helpwith theclick>=8.3the SDK already required) andJinja2>=3.1.5(earlier versions misattribute template errors). Addresses IHS-224.CI jobs
dependency-lower-boundsre-resolves at the declared floors and runs the unit suite on Python 3.10, the only supported interpreter with floor-era wheels for pyarrow, pyyaml and ujson.dependency-lower-bounds-no-harnessreaches the floors the first job cannot, since the test harness pulls pytest, pydantic, anyio and typing-extensions above their declared floors. It imports every shipped module, runs the CLI, and collects a test through the bundled pytest plugin.install-matrixbuilds the wheel and installs it with a fresh resolution on all five supported Python versions, importing every shipped module and running the CLI.Dependency metadata
Written for commit bdfca3d. Summary will update on new commits.