fix(ctl): handle extensions paths in display_schema_load_errors (#1007) by iddocohen · Pull Request #1008 · opsmill/infrahub-sdk-python

iddocohen · 2026-05-10T05:20:47Z

Why

infrahubctl schema load crashes with an unhandled ValueError when the
server rejects a schema that uses the extensions: block. The traceback replaces the
human-readable rejection message, so users have no idea what the server
actually rejected.

Goal: Render extensions-block rejections in the same one-line format as
top-level rejections.

Non-goals: No new error categories, no changes to infrahubctl schema check or to the server's rejection format, no broader refactor of the CLI
schema commands.

Closes #1007

What changed

Behavioral changes

Schema rejections originating inside an extensions: block now render as
Node: <kind> (extensions/<container>) | <field> (<input>) | <msg> (<type>)
instead of crashing with ValueError: invalid literal for int() with base 10.
Unknown / malformed loc_path shapes (non-int schema index, unrecognised
container) are skipped silently instead of crashing the renderer.
Top-level (non-extension) error rendering is byte-for-byte unchanged —
preserved by the existing tests.

Implementation notes

display_schema_load_errors now branches on loc_path[3] == "extensions",
computes container / node_index / tail once, and dispatches on len(tail)
instead of len(loc_path) so the offset shift doesn't have to be repeated.
Per-error rendering extracted into _render_schema_error to keep the main
loop within the project's branch-count limits.
valid_error_path now whitelists loc_path[3] in {nodes, generics, extensions} and requires loc_path[2] (and the node-index slot) to be int.
get_node gained container and is_extension kwargs (defaults match the
old behaviour) and uses payload.get(...) so a schema file with only
extensions: no longer raises KeyError.

What stayed the same

API contract of display_schema_load_errors unchanged.
get_node signature is backward-compatible (new params have defaults).
Output format for non-extension errors is unchanged.

How to review

infrahub_sdk/ctl/schema.py — start with _render_schema_error, then
valid_error_path, then get_node.
tests/unit/sdk/test_schema.py — three new tests cover top-level extensions
errors, nested-attribute extensions errors, and the malformed-path filter.
The pre-existing tests for non-extension paths are intentionally untouched
and still pass — that's the regression guard for the format-stability claim
above.

The valid_error_path whitelist is the area I'd most want a second opinion
on: if the server later adds a new top-level container, errors against it
will now be silently skipped instead of bubbling up. The trade-off is
intentional for a CLI rendering helper, but worth flagging.

How to test

# Repro from the issue (requires a running Infrahub 1.5+ with DcimGenericDevice
# already loaded, e.g. opsmill/schema-library base/dcim.yml):
cat > /tmp/repro_extension.yml <<'YAML'
---
version: "1.0"
extensions:
  generics:
    - kind: DcimGenericDevice
      include_in_menu: false
YAML
infrahubctl schema load /tmp/repro_extension.yml
# Before: ValueError traceback. After: one-line "Node: DcimGenericDevice (extensions/generics) | ..." rendering.

# Unit tests:
uv run pytest tests/unit/sdk/test_schema.py -k display_schema_load_errors -v

# Lint:
uv run invoke format lint-code

Impact & rollout

Backward compatibility: No breaking changes. CLI output for non-extension
errors is unchanged. get_node gained two optional kwargs with defaults.
Performance: N/A — only touched on the error path of schema load.
Config/env changes: None.
Deployment notes: Safe to deploy. Pure SDK/CLI fix, no server-side change.

Checklist

Tests added/updated
Changelog entry added (uv run towncrier create +fix-schema-load-extensions.fixed.md)
External docs updated (if user-facing or ops-facing change)
Internal .md docs updated (internal knowledge and AI code tools knowledge)

codecov · 2026-05-10T05:23:34Z

Codecov Report

❌ Patch coverage is 70.49180% with 18 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
infrahub_sdk/ctl/schema.py	70.49%	11 Missing and 7 partials ⚠️

@@            Coverage Diff             @@
##           stable    #1008      +/-   ##
==========================================
- Coverage   81.56%   81.52%   -0.04%     
==========================================
  Files         134      134              
  Lines       11442    11467      +25     
  Branches     1730     1735       +5     
==========================================
+ Hits         9333     9349      +16     
- Misses       1564     1570       +6     
- Partials      545      548       +3

Flag	Coverage Δ
integration-tests	`41.71% <55.73%> (+0.22%)`	⬆️
python-3.10	`54.57% <63.93%> (-0.02%)`	⬇️
python-3.11	`54.57% <63.93%> (-0.02%)`	⬇️
python-3.12	`54.57% <63.93%> (+<0.01%)`	⬆️
python-3.13	`54.57% <63.93%> (-0.02%)`	⬇️
python-3.14	`54.57% <63.93%> (-0.02%)`	⬇️
python-filler-3.12	`22.66% <0.00%> (-0.05%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
infrahub_sdk/ctl/schema.py	`62.82% <70.49%> (+1.09%)`	⬆️

... and 17 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ogenstad · 2026-05-10T07:47:01Z

Not a review but just highlighting that we also want a towncrier newsfragment under /changelog so that we include what was fixed in the release notes.

Also in general when we're fixing bugs it's fine to target the stable release directly. That part is more important in the Infrahub repo than it is in the SDK though as in the SDK we can merge develop directly into stable for a patch release but that would never happen within the main Infrahub project.

cloudflare-workers-and-pages · 2026-05-10T09:10:57Z

Deploying infrahub-sdk-python with Cloudflare Pages

Latest commit:	`3a0584a`
Status:	✅ Deploy successful!
Preview URL:	https://4c10cdcc.infrahub-sdk-python.pages.dev
Branch Preview URL:	https://ic-fix-display-schema-load-e.infrahub-sdk-python.pages.dev

View logs

ajtmccarty · 2026-05-11T16:11:18Z

I think you'll need to rebase this on stable to remove the extra commits currently included

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

ajtmccarty

thanks for the PR
most of these comments are little things. the testing structure one is larger though

ajtmccarty · 2026-05-11T19:11:48Z

this would be easier to test if this code was written using dependency injection and then you could skip the patch() calls. sometimes patching is necessary for testing, but we generally try to avoid it when you can achieve a more robust test by moving the test up a layer or two in the test pyramid. in this case testing just display_schema_load_errors is fast, but brittle b/c it assumes a certain response structure from the schema load endpoint. I think that these would be better as integration tests similar to those in python_sdk/tests/integration/test_schema.py, then the test really ensures that the response from the server and the client handling work together correctly. sadly, integration tests are slower, but I think it is worth it in this case b/c we have clearly left schema extensions behind for a while and don't want to again

Completed a full refactor, particularly around where to display the error, which was needed some time to think about how.

ajtmccarty · 2026-05-11T19:21:04Z

+    schemas_data: list[SchemaFile],
+    schema_index: int,
+    node_index: int,
+    container: str = "nodes",


optional, b/c we didn't have the before and it's not really your job to clean up the typing, but it would be nice if this were either an Enum or a Literal to make it clear exactly what values are expected here

ajtmccarty · 2026-05-11T19:25:05Z

+
+    if len(tail) == 1:
+        loc_type = tail[0]
+        error_message = f"{loc_type} ({input_str}) | {error['msg']} ({error['type']})"


are we sure that the msg and type keys exist here? if not this will raise a KeyError. it would be safer, and more reasable, to do err_msg = error.get("msg", "" \ "No error message") and then use err_msg and err_type in the f-string.

ajtmccarty · 2026-05-11T19:27:28Z

+            if data.get(attribute) is not None:
+                return data.get("name", None)
+        return None
+    return error_data[attribute].get("name", None)


not your fault, but error_data.get(attribute,{}).get(...) would be safer

I believe error_data is a list and not a dict hence .get() will not work but understood the spirt of the concern and will do an out of bound check.

ajtmccarty · 2026-05-11T19:28:25Z

+    path_suffix = f" (extensions/{container})" if is_extension else ""
+    input_str = error.get("input")
+
+    if len(tail) == 1:


also not your fault, but adding a few one-line comments in here would make it easier to understand this pretty opaque error string parsing

…ests Replace patch()-heavy unit tests for display_schema_load_errors with integration tests that exercise the real server response → renderer path, so we catch any drift between the schema-load endpoint's error shape and CLI handling (particularly for extensions paths). Add an optional Console parameter to display_schema_load_errors so tests can capture rendered output via dependency injection instead of patching the module-level console. Keep valid_error_path covered as a fast parametrized unit test with no patchin

Replace on with a Literal of nodes, generics, relationships so the accepted values are explicit at the type level. Matches the set already enforced at runtime by valid_error_path

…notate _render_schema_error parsing branches

cubic-dev-ai

No issues found across 4 files

ajtmccarty

thanks for the updates. looking good. just a couple more little comments

ajtmccarty · 2026-05-12T18:29:59Z

+    return loc_path[3] in {"nodes", "generics"} and isinstance(loc_path[4], int)
+
+
+SchemaContainer = Literal["nodes", "generics", "relationships"]


generally, this kind of definition goes near the top of the file. but if it is only used in one place, you should be able to do get_node(..., container: Literal["nodes", "generics", "relationships"] = "nodes", ...)

ajtmccarty · 2026-05-12T18:36:16Z

+        schemas_data = [SchemaFile(location=Path("broken.yml"), content=broken_schema)]
+        buffer = StringIO()
+        output = Console(file=buffer, width=1000, force_terminal=False)
+        display_schema_load_errors(response=response.errors, schemas_data=schemas_data, output=output)
+        rendered = buffer.getvalue()


I did not realize it would be such a pain to get the output. is it possible to wrap client.schema.load in something like this from the rich documentation?

from rich.console import Console console = Console() with console.capture() as capture: console.print("[bold red]Hello[/] World") str_output = capture.get()

I tried to do that but it didn't work. I will ask Claude for help here. Stay tuned.

…le.capture() and hoist SchemaContainer alias

iddocohen marked this pull request as ready for review May 10, 2026 05:52

iddocohen requested a review from a team as a code owner May 10, 2026 05:52

iddocohen changed the base branch from develop to stable May 10, 2026 09:11

iddocohen and others added 2 commits May 11, 2026 18:39

fix(ctl): handle extensions paths in display_schema_load_errors (#1007)

98b4f4f

chore(changelog): add newsfragment for #1007

0ab1e4d

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

iddocohen force-pushed the ic-fix-display-schema-load-error branch from 90c534d to 0ab1e4d Compare May 11, 2026 16:41

ajtmccarty reviewed May 11, 2026

View reviewed changes

iddocohen added 4 commits May 12, 2026 07:25

refactor(ctl): type schema get_node container as Literal

b78e546

Replace on with a Literal of nodes, generics, relationships so the accepted values are explicit at the type level. Matches the set already enforced at runtime by valid_error_path

fix(ctl): default missing msg/type keys in schema error renderer

1f85ac5

fix(ctl): guard out-of-range index in _resolve_attribute_label and an…

72947dc

…notate _render_schema_error parsing branches

cubic-dev-ai Bot reviewed May 12, 2026

View reviewed changes

iddocohen requested a review from ajtmccarty May 12, 2026 13:36

ajtmccarty reviewed May 12, 2026

View reviewed changes

refactor(ctl): address schema-load review feedback and use rich conso…

3a0584a

…le.capture() and hoist SchemaContainer alias

		return loc_path[3] in {"nodes", "generics"} and isinstance(loc_path[4], int)


		SchemaContainer = Literal["nodes", "generics", "relationships"]

Conversation

iddocohen commented May 10, 2026

Why

What changed

How to review

How to test

Impact & rollout

Checklist

Uh oh!

codecov Bot commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ogenstad commented May 10, 2026

Uh oh!

cloudflare-workers-and-pages Bot commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying infrahub-sdk-python with Cloudflare Pages

Uh oh!

ajtmccarty commented May 11, 2026

Uh oh!

ajtmccarty left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ajtmccarty left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented May 10, 2026 •

edited

Loading

cloudflare-workers-and-pages Bot commented May 10, 2026 •

edited

Loading