fix(packaging): correct declared dependencies and publish the tests extra - #1277
Open
ogenstad wants to merge 3 commits into
Open
fix(packaging): correct declared dependencies and publish the tests extra#1277ogenstad wants to merge 3 commits into
ogenstad wants to merge 3 commits into
Conversation
…xtra The declared dependency surface had drifted from what the SDK actually needs. pydantic>=2.0.0 admitted 2.0 and 2.0.2, on which `import infrahub_sdk` raises SchemaError because their regex engine rejects the `\_` escape in the generated schema model patterns. anyio, typing-extensions and packaging were imported by the shipped package but only ever arrived as transitive dependencies of httpx, pydantic and the test tooling, so a constrained resolution could install the SDK unusable. The `tests` extra is described in the installation guide but was never published, so `pip install 'infrahub-sdk[tests]'` warned and installed nothing beyond the base package. It now exists and carries what `infrahub_sdk.testing` and the bundled pytest plugin import. `all` becomes self-referential so it cannot drift from the extras it aggregates, which is how it had already lost mdxify. numpy and mdxify were declared but never imported. pyarrow declares numpy itself on the releases that need it, and mdxify only builds the docs, so it moves to a docs dependency group. Each floor was established by installing the candidate version and exercising the import rather than read off release notes: anyio.Path appears in 3.3.0 rather than 3.0, and typing-extensions needs 4.4.0 for PEP 696 TypeVar defaults. New unit tests over the packaging metadata keep the declared and imported dependency sets in agreement.
The extra is newly installable, so readers will reach it for the first time. It pulls in the container tooling behind `infrahub_sdk.testing`, which is a lot more than the `ctl` extra beside it.
ogenstad
added
the
type/tech-debt
github-actions
Bot
added
the
type/documentation
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## infrahub-develop #1277 +/- ##
=================================================
Coverage 84.16% 84.16%
=================================================
Files 147 147
Lines 13047 13047
Branches 1930 1930
=================================================
Hits 10981 10981
Misses 1503 1503
Partials 563 563
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.
4 issues found across 9 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="pyproject.toml">
<violation number="1" location="pyproject.toml:66">
P2: Custom agent: **Detect conflicting package versions across dependency files**
The new `tests` extra declares `pytest>=7.0`, but the same file's `tests` dependency group declares `pytest>=9.0,<9.1`. Align these pytest specifiers or otherwise avoid declaring conflicting versions in the same dependency metadata.</violation>
</file>
<file name="tests/unit/test_packaging_metadata.py">
<violation number="1" location="tests/unit/test_packaging_metadata.py:50">
P3: The docstring says this normalises per PEP 503, but consecutive separators are not collapsed (each '_'/'.'/ '-' becomes its own '-'), so names that PEP 503 treats as equal compare unequal here. Currently harmless because both comparison sides use the same helper, but the claim is misleading and a future dist name with doubled separators would silently fail to match. Collapse runs with a regex to be faithful to PEP 503.</violation>
<violation number="2" location="tests/unit/test_packaging_metadata.py:140">
P2: `test_imports_in_shipped_package_are_declared` aggregates `declared` across all extras, so it can't detect base-wheel modules that import extra-only packages. For example, `infrahub_sdk/transfer/importer/json.py` top-level imports `rich` (ctl-only) and `infrahub_sdk/template/__init__.py` imports `jinja2` (ctl-only), yet a plain `pip install infrahub-sdk` installs neither, and this test still passes. The docstring's guarantee ('anything the wheel imports must be installable') isn't actually enforced for base installs. Consider checking the set of distributions a module is importable with (base deps only) for modules that ship with no extra, or verifying base-install imports against base dependencies rather than the extra union.</violation>
</file>
<file name="changelog/+dependency-lower-bounds.changed.md">
<violation number="1" location="changelog/+dependency-lower-bounds.changed.md:5">
P3: The changelog groups `packaging` with `anyio`/`typing-extensions` as base dependency fixes that keep a minimal install usable, but `packaging` is declared only in the `tests` extra and imported only by `infrahub_sdk/testing/docker.py`, not by the base SDK. A plain `pip install infrahub-sdk` still does not install `packaging`. Clarify that `packaging` is a direct requirement of the `tests` extra (for the testing helpers), separate from the base-level `anyio`/`typing-extensions` additions.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| tests = [ | ||
| "infrahub-testcontainers>=1.7.3", | ||
| "packaging>=21.0", | ||
| "pytest>=7.0", |
Contributor
There was a problem hiding this comment.
P2: Custom agent: Detect conflicting package versions across dependency files
The new tests extra declares pytest>=7.0, but the same file's tests dependency group declares pytest>=9.0,<9.1. Align these pytest specifiers or otherwise avoid declaring conflicting versions in the same dependency metadata.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pyproject.toml, line 66:
<comment>The new `tests` extra declares `pytest>=7.0`, but the same file's `tests` dependency group declares `pytest>=9.0,<9.1`. Align these pytest specifiers or otherwise avoid declaring conflicting versions in the same dependency metadata.</comment>
<file context>
@@ -43,30 +47,28 @@ infrahubctl = "infrahub_sdk.ctl.cli:app"
+tests = [
+ "infrahub-testcontainers>=1.7.3",
+ "packaging>=21.0",
+ "pytest>=7.0",
+]
+
</file context>
|
|
||
| def test_imports_in_shipped_package_are_declared() -> None: | ||
| """Anything the wheel imports must be installable, not left to arrive as a transitive dependency.""" | ||
| declared = _declared_distributions() |
Contributor
There was a problem hiding this comment.
P2: test_imports_in_shipped_package_are_declared aggregates declared across all extras, so it can't detect base-wheel modules that import extra-only packages. For example, infrahub_sdk/transfer/importer/json.py top-level imports rich (ctl-only) and infrahub_sdk/template/__init__.py imports jinja2 (ctl-only), yet a plain pip install infrahub-sdk installs neither, and this test still passes. The docstring's guarantee ('anything the wheel imports must be installable') isn't actually enforced for base installs. Consider checking the set of distributions a module is importable with (base deps only) for modules that ship with no extra, or verifying base-install imports against base dependencies rather than the extra union.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/unit/test_packaging_metadata.py, line 140:
<comment>`test_imports_in_shipped_package_are_declared` aggregates `declared` across all extras, so it can't detect base-wheel modules that import extra-only packages. For example, `infrahub_sdk/transfer/importer/json.py` top-level imports `rich` (ctl-only) and `infrahub_sdk/template/__init__.py` imports `jinja2` (ctl-only), yet a plain `pip install infrahub-sdk` installs neither, and this test still passes. The docstring's guarantee ('anything the wheel imports must be installable') isn't actually enforced for base installs. Consider checking the set of distributions a module is importable with (base deps only) for modules that ship with no extra, or verifying base-install imports against base dependencies rather than the extra union.</comment>
<file context>
@@ -0,0 +1,178 @@
+
+def test_imports_in_shipped_package_are_declared() -> None:
+ """Anything the wheel imports must be installable, not left to arrive as a transitive dependency."""
+ declared = _declared_distributions()
+ undeclared = {
+ distribution: source
</file context>
| } | ||
|
|
||
|
|
||
| def _normalize(name: str) -> str: |
Contributor
There was a problem hiding this comment.
P3: The docstring says this normalises per PEP 503, but consecutive separators are not collapsed (each '_'/'.'/ '-' becomes its own '-'), so names that PEP 503 treats as equal compare unequal here. Currently harmless because both comparison sides use the same helper, but the claim is misleading and a future dist name with doubled separators would silently fail to match. Collapse runs with a regex to be faithful to PEP 503.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/unit/test_packaging_metadata.py, line 50:
<comment>The docstring says this normalises per PEP 503, but consecutive separators are not collapsed (each '_'/'.'/ '-' becomes its own '-'), so names that PEP 503 treats as equal compare unequal here. Currently harmless because both comparison sides use the same helper, but the claim is misleading and a future dist name with doubled separators would silently fail to match. Collapse runs with a regex to be faithful to PEP 503.</comment>
<file context>
@@ -0,0 +1,178 @@
+}
+
+
+def _normalize(name: str) -> str:
+ """Normalise a distribution name per PEP 503 so `ruamel.yaml` and `Jinja2` compare cleanly."""
+ return "".join("-" if character in "-_." else character for character in name.lower())
</file context>
|
|
||
| If you pin `pydantic` to 2.0, 2.0.1 or 2.0.2, installing the SDK now fails while resolving dependencies. Those versions never actually worked: `import infrahub_sdk` raised a `SchemaError` on them, because their regex engine rejects a pattern used by the schema models. Pin `pydantic>=2.0.3` to resolve it. | ||
|
|
||
| `anyio`, `typing-extensions` and `packaging` are also now installed as direct requirements. The SDK has always imported them but relied on other packages to pull them in, so a minimal or heavily constrained environment could end up with the SDK installed and unusable. No action is needed, installs simply become reliable. |
Contributor
There was a problem hiding this comment.
P3: The changelog groups packaging with anyio/typing-extensions as base dependency fixes that keep a minimal install usable, but packaging is declared only in the tests extra and imported only by infrahub_sdk/testing/docker.py, not by the base SDK. A plain pip install infrahub-sdk still does not install packaging. Clarify that packaging is a direct requirement of the tests extra (for the testing helpers), separate from the base-level anyio/typing-extensions additions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At changelog/+dependency-lower-bounds.changed.md, line 5:
<comment>The changelog groups `packaging` with `anyio`/`typing-extensions` as base dependency fixes that keep a minimal install usable, but `packaging` is declared only in the `tests` extra and imported only by `infrahub_sdk/testing/docker.py`, not by the base SDK. A plain `pip install infrahub-sdk` still does not install `packaging`. Clarify that `packaging` is a direct requirement of the `tests` extra (for the testing helpers), separate from the base-level `anyio`/`typing-extensions` additions.</comment>
<file context>
@@ -0,0 +1,5 @@
+
+If you pin `pydantic` to 2.0, 2.0.1 or 2.0.2, installing the SDK now fails while resolving dependencies. Those versions never actually worked: `import infrahub_sdk` raised a `SchemaError` on them, because their regex engine rejects a pattern used by the schema models. Pin `pydantic>=2.0.3` to resolve it.
+
+`anyio`, `typing-extensions` and `packaging` are also now installed as direct requirements. The SDK has always imported them but relied on other packages to pull them in, so a minimal or heavily constrained environment could end up with the SDK installed and unusable. No action is needed, installs simply become reliable.
</file context>
The import scan classified modules with the running interpreter's `sys.stdlib_module_names`. `tomllib` only joined the standard library in 3.11, so on 3.10 the guarded `import tomllib` in ctl/config.py looked like an undeclared third-party package and failed the check. Exempt it by name rather than skipping imports nested under a `sys.version_info` guard: the `tomli` backport sits in the same else branch and is a real declared dependency that must stay verified.
Deploying infrahub-sdk-python with
|
| Latest commit: |
1700088
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0170e75e.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pog-packaging-metadata-tests.infrahub-sdk-python.pages.dev |
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
1 participant
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-test matrix runs five Python versions but every job installs exactly what
uv.lockpins, so the version ranges declared inpyproject.tomlare never exercised. Checking them by hand turned up several that were wrong, two of which break users outright.Goal: make the declared dependency surface match what the SDK actually needs, and add tests so it stays that way.
Non-goals: no CI jobs that resolve at the declared lower and upper bounds. That is the follow-up work, and it is what will genuinely test the ranges end to end. This PR only covers what can be checked without extra CI time.
Ref IHS-224 (part 1 of 3).
What changed
Behavioral changes:
pip install 'infrahub-sdk[tests]'works. The extra is documented in the installation guide but was never published, so the command warned that no such extra existed and installed nothing beyond the base package.SchemaErroronimport infrahub_sdk. Those versions reject the\_escape in the generated schema model patterns, so they never worked.anyio,typing-extensionsandpackagingare declared directly. All three were imported by the shipped package but only arrived as transitive dependencies, so a constrained resolution could install the SDK unusable.infrahub-sdk[ctl]no longer installsnumpyormdxify, neither of which the SDK imports.infrahub-sdk[all]now coversctlandteststogether, so it pulls in considerably more than before.Implementation notes:
anyio.Patharrives in 3.3.0 rather than 3.0, andtyping-extensionsneeds 4.4.0 for PEP 696TypeVardefaults.allis now["infrahub-sdk[ctl,tests]"]. Hand-duplicating the lists is how it had already lostmdxify, and a self-reference cannot drift.mdxifymoves to a newdocsdependency group, included indev, souv sync --all-groupsstill provides it forinvoke docs-generate.What stayed the same: no runtime code changed. This is packaging metadata, a regenerated lock, one docs note, and a new test module.
How to review
Start with the
pyproject.tomldiff, which is the whole substance of the change. Thentests/unit/test_packaging_metadata.py. Theuv.lockdiff is mechanical, and mostly shrinkage from droppingnumpyandmdxify.Two things worth extra scrutiny:
testsextra shares its name with thetestsdependency group while holding different contents. The name was chosen to match what the docs and README already tell people to type. uv handles the two namespaces without complaint, but the follow-up PR that splits the test groups is the natural place to rename the group.testsextra is heavy, adding roughly 66 packages including Docker, FastAPI, uvicorn and Prefect client libraries, all viainfrahub-testcontainers. Someone who only wants the pytest plugin gets all of it. Splitting the container-based helpers into their own extra is worth considering in the follow-up.How to test
The new tests are non-vacuous: pointing them at the previous
pyproject.tomlfails four of the five checks, each naming a real defect. Therequires-pythoncheck passes on the old metadata too, so it is a regression guard rather than a bug finder.Verified locally:
ruff,tyandmypyclean;docs-validateexits 0 with no committed docs changed;lint-docsbyte-identical to the base branch. The unit suite is 1840 passed with 2 failures, both of which reproduce identically in a pristine worktree of the base commit (macOS-only Rich wrapping of long/private/var/folders/...temp paths, in files this PR does not touch).Impact & rollout
numpy/mdxifyfrom the extras are resolver-visible narrowings. Nothing that worked before stops working, since the removed versions could not import the SDK and the removed packages were never used, but the install-time behavior changes. This targetsinfrahub-developdeliberately so it ships with the next Infrahub version rather than as a patch to the current SDK line.Checklist
Summary by cubic
Aligns packaging metadata with actual imports and publishes the
testsextra so installs are reliable. Old: installs could miss needed deps or acceptpydantic2.0/2.0.2 and crash, andinfrahub-sdk[tests]did nothing; New: direct deps are declared, those versions are rejected during resolution, thetestsextra installs, and the metadata tests were fixed to treattomllibas stdlib on Python 3.10 to avoid false positives. Ref IHS-224.Dependencies
pydantic>=2.0.3(2.0 and 2.0.2 now fail during resolution).anyio>=3.3.0,typing-extensions>=4.4.0, andpackaging>=21.0as direct requirements.testsextra withinfrahub-testcontainers>=1.7.3,pytest>=7.0, andpackaging>=21.0.allaggregate other extras viainfrahub-sdk[ctl,tests]to prevent drift.numpyandmdxifyfromctl; movemdxifyto adocsdependency group included indev.Migration
pydanticto 2.0.x, raise it to>=2.0.3.infrahub-sdk[ctl]to installnumpyormdxify, declare them in your project.infrahub-sdk[all]to install more; useinfrahub-sdk[ctl]if you don’t need the testing tools.Written for commit 1700088. Summary will update on new commits.