Conversation
Render infrahub_sdk/exceptions/catalogue.py into the python_sdk submodule as part of `invoke backend.generate`, alongside the schema models and the protocols already produced there. The artefact carries a pydantic payload model per catalogue code, an exception class per code the SDK does not already own, and a dispatch that resolves a code and its payload to a concrete exception without widening the payload type on the way. Names and types derive from schema/error-catalogue.json alone, with no per-code table on either side. Classes the SDK already ships are adopted by parsing their CODE declarations out of exceptions/base.py, so adopting a further code needs no generator change. Generation aborts rather than emitting a guess when the catalogue would shadow a name the module or the SDK already binds, produce an unusable identifier, declare a payload field colliding with the exception's own members, use an unsupported schema construct, or omit a status, title or catalogue version. backend.validate-generated now fails when the committed artefact is stale or was never committed, and backend-validate-generated also triggers on a change to the catalogue JSON alone. The derivation logic lives in infrahub/errors/sdk_bindings.py, beside the exporter that produces its input, so tasks/backend.py stays the thin invoke wrapper this pipeline already uses and the tests import it directly. CI's regenerate-and-diff only proves the committed file matches what the generator produces today - a wrong rule produces the same wrong file on both sides of it - so the derivations and every refusal are covered by unit tests.
…line The pipeline diagram, the regeneration table and the validate-generated list each enumerate what Infrahub generates into the submodule, so all three went stale when the error bindings joined them.
ogenstad
added
type/feature
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 6 files
Confidence score: 2/5
tasks/backend.pywill failbackend.validate-generatedbecause the generated SDK binding is missing from thepython_sdksubmodule, blocking validation; commitinfrahub_sdk/exceptions/catalogue.pyin the SDK revision.backend/templates/generate_sdk_errors.j2invokesfrom_payloadfor adopted codes such asNODE_NOT_FOUND, but the imported exception class does not define it, causing generated bindings to fail; add or correctly reference the supported conversion method.backend/infrahub/errors/sdk_bindings.pycannot generate against the available SDK becausescan_sdk_exceptionsonly readsexceptions/base.pywhileGraphQLErrorremains in the monolithic module; update scanning for the current SDK layout or align the SDK revision.
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="tasks/backend.py">
<violation number="1" location="tasks/backend.py:432">
P1: `backend.validate-generated` will fail on this PR because the new SDK binding is generated but not committed in the `python_sdk` submodule. Add the generated `infrahub_sdk/exceptions/catalogue.py` to the SDK commit and bump the submodule pointer here.</violation>
</file>
<file name="backend/templates/generate_sdk_errors.j2">
<violation number="1" location="backend/templates/generate_sdk_errors.j2:97">
P1: For adopted codes such as `NODE_NOT_FOUND`, this builder calls a method the template never defines on the imported exception class. The current adopted class has no `from_payload` at opsmill/infrahub-sdk-python/infrahub_sdk/exceptions.py:99-121, so make the SDK base contract provide and validate an adapter, or generate wrapper classes for adopted exceptions.</violation>
</file>
<file name="backend/infrahub/errors/sdk_bindings.py">
<violation number="1" location="backend/infrahub/errors/sdk_bindings.py:22">
P1: `backend.generate` cannot run against the SDK revision currently available: `scan_sdk_exceptions` always reads `infrahub_sdk/exceptions/base.py`, while the SDK defines `GraphQLError` in its existing monolithic module at opsmill/infrahub-sdk-python/infrahub_sdk/exceptions.py:60-67. Add and select the SDK layout expected by this generator, or point the generator at the actual exception module before relying on this path.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| context.run(exec_cmd) | ||
|
|
||
| _generate_sdk_error_bindings(context=context) | ||
| _check_sdk_error_bindings_committed(context=context) |
Contributor
There was a problem hiding this comment.
P1: backend.validate-generated will fail on this PR because the new SDK binding is generated but not committed in the python_sdk submodule. Add the generated infrahub_sdk/exceptions/catalogue.py to the SDK commit and bump the submodule pointer here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tasks/backend.py, line 432:
<comment>`backend.validate-generated` will fail on this PR because the new SDK binding is generated but not committed in the `python_sdk` submodule. Add the generated `infrahub_sdk/exceptions/catalogue.py` to the SDK commit and bump the submodule pointer here.</comment>
<file context>
@@ -426,6 +428,90 @@ def validate_generated(context: Context, docker: bool = False) -> None: # noqa:
context.run(exec_cmd)
+ _generate_sdk_error_bindings(context=context)
+ _check_sdk_error_bindings_committed(context=context)
+
+
</file context>
| {% for builder in builders %} | ||
|
|
||
| def {{ builder.name }}(data: Mapping[str, Any]) -> GraphQLError: | ||
| return {{ builder.exception }}.from_payload({{ builder.model }}.model_validate(data)) |
Contributor
There was a problem hiding this comment.
P1: For adopted codes such as NODE_NOT_FOUND, this builder calls a method the template never defines on the imported exception class. The current adopted class has no from_payload at opsmill/infrahub-sdk-python/infrahub_sdk/exceptions.py:99-121, so make the SDK base contract provide and validate an adapter, or generate wrapper classes for adopted exceptions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/templates/generate_sdk_errors.j2, line 97:
<comment>For adopted codes such as `NODE_NOT_FOUND`, this builder calls a method the template never defines on the imported exception class. The current adopted class has no `from_payload` at opsmill/infrahub-sdk-python/infrahub_sdk/exceptions.py:99-121, so make the SDK base contract provide and validate an adapter, or generate wrapper classes for adopted exceptions.</comment>
<file context>
@@ -0,0 +1,117 @@
+{% for builder in builders %}
+
+def {{ builder.name }}(data: Mapping[str, Any]) -> GraphQLError:
+ return {{ builder.exception }}.from_payload({{ builder.model }}.model_validate(data))
+{% endfor %}
+
</file context>
| from pathlib import Path | ||
|
|
||
| CATALOGUE_SOURCE = "schema/error-catalogue.json" | ||
| SDK_EXCEPTIONS_BASE = "infrahub_sdk/exceptions/base.py" |
Contributor
There was a problem hiding this comment.
P1: backend.generate cannot run against the SDK revision currently available: scan_sdk_exceptions always reads infrahub_sdk/exceptions/base.py, while the SDK defines GraphQLError in its existing monolithic module at opsmill/infrahub-sdk-python/infrahub_sdk/exceptions.py:60-67. Add and select the SDK layout expected by this generator, or point the generator at the actual exception module before relying on this path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/errors/sdk_bindings.py, line 22:
<comment>`backend.generate` cannot run against the SDK revision currently available: `scan_sdk_exceptions` always reads `infrahub_sdk/exceptions/base.py`, while the SDK defines `GraphQLError` in its existing monolithic module at opsmill/infrahub-sdk-python/infrahub_sdk/exceptions.py:60-67. Add and select the SDK layout expected by this generator, or point the generator at the actual exception module before relying on this path.</comment>
<file context>
@@ -0,0 +1,500 @@
+ from pathlib import Path
+
+CATALOGUE_SOURCE = "schema/error-catalogue.json"
+SDK_EXCEPTIONS_BASE = "infrahub_sdk/exceptions/base.py"
+SDK_ERROR_BINDINGS = "infrahub_sdk/exceptions/catalogue.py"
+TEMPLATE_NAME = "generate_sdk_errors.j2"
</file context>
The build fixture parsed the SDK's exceptions module out of python_sdk, so every test in the file needed whatever commit the pointer happened to carry. The pointer predates the exceptions package, so all 75 errored in CI and the huge-runner gate stayed shut behind them. The generator's rules are about the surface it is handed, not about which SDK commit is pinned, so that surface is now stated in the test module. One test still parses the submodule and holds the two together; it skips until the pointer carries the package.
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Confidence score: 4/5
- In
backend/tests/unit/errors/test_sdk_bindings_generator.py, the current assertion can miss module-level classes because coverage continues to rely only onDEFINED_NAMES, allowing collision regressions to go undetected; require exact equality so the stand-in reflects the full generated module.
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="backend/tests/unit/errors/test_sdk_bindings_generator.py">
<violation number="1" location="backend/tests/unit/errors/test_sdk_bindings_generator.py:221">
P2: When the SDK adds a module-level class, this assertion still passes while the generator tests keep using only `DEFINED_NAMES`; collision coverage can silently become incomplete. Require exact equality so the stand-in remains a complete SDK surface.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| assert adopted == ADOPTED_CODES | ||
| # It subclasses an adopted class and clears CODE, so it represents no code of its own. | ||
| assert "NodeInvalidError" not in adopted.values() | ||
| assert defined >= DEFINED_NAMES |
Contributor
There was a problem hiding this comment.
P2: When the SDK adds a module-level class, this assertion still passes while the generator tests keep using only DEFINED_NAMES; collision coverage can silently become incomplete. Require exact equality so the stand-in remains a complete SDK surface.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/tests/unit/errors/test_sdk_bindings_generator.py, line 221:
<comment>When the SDK adds a module-level class, this assertion still passes while the generator tests keep using only `DEFINED_NAMES`; collision coverage can silently become incomplete. Require exact equality so the stand-in remains a complete SDK surface.</comment>
<file context>
@@ -178,19 +207,18 @@ def test_required_field_carries_no_default_and_optional_field_carries_its_own()
# It subclasses an adopted class and clears CODE, so it represents no code of its own.
assert "NodeInvalidError" not in adopted.values()
- assert {"NodeInvalidError", "GraphQLError", "ApiError", "UNDEFINED_ERROR_CODE"} <= defined
+ assert defined >= DEFINED_NAMES
</file context>
Suggested change
| assert defined >= DEFINED_NAMES | |
| assert defined == DEFINED_NAMES |
A payload field named from_payload rendered `self.from_payload = ...` over the classmethod the factory dispatches through, and one named __init__ rendered over the constructor. Both produced a module that parses, so nothing downstream would have caught them. Reserve from_payload alongside the other members the class binds, and refuse any field starting with an underscore, which covers the dunders. The tests stopped at the render context and never ran the template, which is why this got through; they now render and assert against the module.
Contributor
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
This branch has not been deployed
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 Python SDK has no way to tell one server failure from another without reading
message strings. Infrahub already publishes an error catalogue and generates
TypeScript bindings for the frontend from it; the SDK was left to string-match.
This PR makes Infrahub generate the SDK's error bindings from that same catalogue,
so a caller can branch on a typed exception instead of a message.
Non-goals: the SDK-side changes that consume these bindings (IHS-295), and the
submodule pointer bump.
Part of IFC-3034. Covers T052-T062.
What changed
Behavioral changes:
uv run invoke backend.generatenow also rendersinfrahub_sdk/exceptions/catalogue.pyinto thepython_sdksubmodule, alongsidethe schema models and protocols it already produces there.
uv run invoke backend.validate-generatedfails when those bindings are staleor were never committed.
git diffreports nothing for an untracked file, soa diff-only check would pass a freshly generated artefact that nobody committed.
backend-validate-generatednow also triggers on a change toschema/error-catalogue.jsonalone, so a hand-edit of the JSON cannot slip past.Implementation notes:
schema/error-catalogue.json; there is no per-code tableon either side. Classes the SDK already ships are adopted by parsing their
CODEdeclarations out of
exceptions/base.pywithast, so adopting a further codeneeds no generator change.
name, produce an unusable identifier, declare a payload field that collides with
the exception's own members, use an unsupported schema construct, or omit a
status, title or version. Two of these were found to fail silently before the
guards existed: a payload field named
codecompiled cleanly and overwrote theclass's catalogue code.
infrahub/errors/sdk_bindings.py, beside theexporter that produces its input, so
tasks/backend.pystays the thin invokewrapper this pipeline already uses.
What stayed the same: no schema changes, no API contract changes. The SDK submodule
pointer is untouched; that bump belongs with the SDK merge.
Suggested review order
python_sdk/specs/ifc-3034-error-catalogue/contracts/generator-contract.md- the contractbackend/infrahub/errors/sdk_bindings.py- the derivation rules and refusalsbackend/templates/generate_sdk_errors.j2- what gets emittedtasks/backend.py- invoke plumbing onlyHow to review
Warning
backend-validate-generatedis expected to fail on this PR. It reportscatalogue.py is generated but is not committed in the SDK. That is this PR's owngate working correctly: the artefact is committed by IHS-295 in the SDK repo, which
merges first. This PR then bumps the pointer and the job goes green.
uv run invoke backend.generate # writes the bindings into python_sdk/ uv run pytest backend/tests/unit/errors/ -qGenerating twice is byte-identical. The output is 9 generated classes, 3 adopted
imports, 15 payload models, and no class for the three 401/403 codes.
Documentation Updates
dev/knowledge/backend/code-generation.md- the pipeline diagram, theregeneration table and the
validate-generatedlist all enumerate what isgenerated into the submodule, so all three were stale.
No changelog fragment: the generator, the CI trigger and the validation are
internal, and the artefact ships from the SDK repo, whose own fragment covers what
users see.
Test Plan
backend/tests/unit/errors/test_sdk_bindings_generator.py)covering name derivation, the JSON Schema vocabulary, the adoption walk, every
abort condition, and the staleness gate's decision.
untracked artefact read as committed, each fail the suite.
Assisted-by: opsmill-dev-commit 0.1.0
Assisted-by: opsmill-dev-pr 0.2.0