Skip to content

feat(protocols): support generating the SDK core protocols module [IFC-3054] - #1273

Draft
ogenstad wants to merge 2 commits into
infrahub-developfrom
pog-sdk-core-protocols-IFC-3054
Draft

feat(protocols): support generating the SDK core protocols module [IFC-3054]#1273
ogenstad wants to merge 2 commits into
infrahub-developfrom
pog-sdk-core-protocols-IFC-3054

Conversation

@ogenstad

@ogenstad ogenstad commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Why

The code generator behind infrahubctl protocols can only render protocols for a user's own
schema, importing the core kinds it references from infrahub_sdk.protocols. It cannot render
that module itself, which is why infrahub_sdk/protocols.py is instead generated from a separate
Jinja template that lives in the Infrahub repository. That second template has drifted: it emits
bare RelatedNode / RelationshipManager with no peer type, so the protocols we ship lose the
peer of every relationship, while this generator has emitted RelationshipManager[Peer] and
RelationshipAttribute[Peer] for a while now.

Goal: teach this generator to render the core module too, so there is one renderer and the two
cannot drift again.

Non-goals: this PR does not regenerate infrahub_sdk/protocols.py. Producing it needs the
matching change on the Infrahub side, and it is a ~2,300-line generated diff that deserves its own
review. It follows in a second PR, with its own changelog entry.

Part of IFC-3054.

What changed

Behavioral changes, for users:

  • A hierarchical kind no longer declares parent and children twice. When the schema already
    exposes them as relationships, which is the case for anything read from the API, they were
    emitted once from the relationship list and again from the hierarchy, with the second pair
    overriding the first. This is the only change to what infrahubctl protocols writes.

Implementation notes:

  • ProtocolTarget selects between USER_SCHEMA (the default, so every existing caller is
    unaffected) and SDK_CORE.
  • The per-file header moved out of template.j2 into header.j2, so the core module can render
    one header followed by two bodies. The user branch of that header is byte-identical to before.
  • Two name sets decide which referenced classes switch to a Sync counterpart. They are separate
    because the two positions already resolved names differently for a user schema: a peer may be
    any core kind, while an inheritance list only ever switches the three names in
    CORE_BASE_CLASS_TO_SYNCIFY. Generating the core module makes both sets the same, since every
    class in it is local. That asymmetry is preserved rather than fixed, to keep user output stable.

What stayed the same:

  • infrahubctl protocols output, apart from the duplicate lines above. This is enforced, not
    claimed: tests/fixtures/protocols_generator/ pins the full rendered file for both variants.
    The fixtures were captured from the generator before any change and the test confirmed green
    on the untouched code, so the two removed lines are the entire diff.
  • The public API. CodeGenerator(schema=...) and render(sync=...) are unchanged.

How to review

Suggested order:

  1. target.py and generator.py for the mechanism.
  2. template.j2, which is only the header being removed, {{ suffix }} on the four class-name
    lines, and the hierarchical guard.
  3. header.j2, where the user branch should match the block deleted from template.j2.
  4. The tests, with the golden fixtures being generated output rather than hand-written.

The part worth extra scrutiny is the hierarchical guard, since it is the one behavioural change.
I chose to fix it here rather than leave it because the regenerated core module would otherwise
ship duplicated annotations for CoreGroup, BuiltinIPPrefix, CoreMenu and others.

How to test

uv run pytest tests/unit/sdk/test_protocols_generator.py
uv run invoke format lint-code

If test_repository_app, test_task_app::test_task_list_command or
test_config::test_missing_password fail for you locally, that is environment leakage rather than
this branch: they fail the same way on a clean origin/infrahub-develop checkout, and CI is green
on all five Python versions.

Impact & rollout

  • Backward compatibility: no API change. Generated output changes only by dropping the
    duplicate parent/children pair described above.
  • Deployment notes: safe to merge on its own. The submodule pointer bump in Infrahub waits on
    the follow-up PR that regenerates infrahub_sdk/protocols.py.

Checklist

  • Tests added/updated
  • Changelog entry added
  • External docs updated (if user-facing or ops-facing change)
  • Internal .md docs updated (internal knowledge and AI code tools knowledge)

…C-3054]

The code generator could only render protocols for a user's own schema, importing
the core kinds it referenced from infrahub_sdk.protocols. It could not render that
module itself: every kind in it is local, so nothing can be imported from it, both
the async and sync variants share one file, and the sync classes need a suffix to
keep their names distinct.

Add a ProtocolTarget to select between the two, splitting the per-file header out
of the per-variant body so the core module can emit one header and two bodies.

Output for a user schema is unchanged, with one exception: a hierarchical kind no
longer declares parent and children twice when the schema already exposes them as
relationships. Golden fixtures now pin the rendered output for both variants so any
further change to what infrahubctl writes has to be accepted deliberately.
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 24, 2026

Copy link
Copy Markdown

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 516f735
Status: ✅  Deploy successful!
Preview URL: https://2e2a57b4.infrahub-sdk-python.pages.dev
Branch Preview URL: https://pog-sdk-core-protocols-ifc-3.infrahub-sdk-python.pages.dev

View logs

@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 9 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
infrahub_sdk/protocols_generator/generator.py 77.50% 6 Missing and 3 partials ⚠️
@@                 Coverage Diff                  @@
##           infrahub-develop    #1273      +/-   ##
====================================================
- Coverage             84.16%   84.13%   -0.03%     
====================================================
  Files                   147      148       +1     
  Lines                 13047    13081      +34     
  Branches               1930     1936       +6     
====================================================
+ Hits                  10981    11006      +25     
- Misses                 1503     1509       +6     
- Partials                563      566       +3     
Flag Coverage Δ
integration-tests 38.98% <28.88%> (-0.04%) ⬇️
python-3.10 57.02% <80.00%> (+0.04%) ⬆️
python-3.11 57.04% <80.00%> (+0.04%) ⬆️
python-3.12 57.02% <80.00%> (+0.02%) ⬆️
python-3.13 57.04% <80.00%> (+0.04%) ⬆️
python-3.14 57.04% <80.00%> (+0.04%) ⬆️
python-filler-3.12 23.62% <0.00%> (-0.07%) ⬇️

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

Files with missing lines Coverage Δ
infrahub_sdk/protocols_generator/constants.py 100.00% <100.00%> (ø)
infrahub_sdk/protocols_generator/target.py 100.00% <100.00%> (ø)
infrahub_sdk/protocols_generator/generator.py 88.28% <77.50%> (-6.84%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 9 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread infrahub_sdk/protocols_generator/template.j2 Outdated
…chy guard

The guard suppressed both members when the schema already declared "parent" as a
relationship. A schema declaring only one of the pair therefore either got a
duplicate "children" or lost it entirely.

Only generics carry "hierarchical", so the equivalent blocks in the node and profile
loops are unreachable today, but the guard is kept identical across the three copies
rather than reasoning about which one is live.
@ogenstad

Copy link
Copy Markdown
Contributor Author

Thanks, the finding is valid and is fixed in 516f735.

The guard checked only parent before deciding to emit the pair, so a schema declaring children but not parent would have got a duplicate children, and one declaring parent but not children would have lost children entirely. Each name is now checked independently.

One clarification for anyone reading later: only generics carry hierarchical (NodeSchemaAPI has hierarchy instead, and profiles and templates have neither), so the equivalent blocks in the node and profile loops cannot fire today. I applied the same guard to all three copies anyway rather than making a reader work out which one is live.

Added test_hierarchy_members_are_declared_once, which covers all four combinations of the pair being declared as relationships and asserts each is emitted exactly once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant