feat(protocols): support generating the SDK core protocols module [IFC-3054] - #1273
feat(protocols): support generating the SDK core protocols module [IFC-3054]#1273ogenstad wants to merge 2 commits into
Conversation
…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.
Deploying infrahub-sdk-python with
|
| 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 |
Codecov Report❌ Patch coverage is
@@ 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
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
All reported issues were addressed across 9 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…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.
|
Thanks, the finding is valid and is fixed in 516f735. The guard checked only One clarification for anyone reading later: only generics carry Added |
Why
The code generator behind
infrahubctl protocolscan only render protocols for a user's ownschema, importing the core kinds it references from
infrahub_sdk.protocols. It cannot renderthat module itself, which is why
infrahub_sdk/protocols.pyis instead generated from a separateJinja template that lives in the Infrahub repository. That second template has drifted: it emits
bare
RelatedNode/RelationshipManagerwith no peer type, so the protocols we ship lose thepeer of every relationship, while this generator has emitted
RelationshipManager[Peer]andRelationshipAttribute[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 thematching 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:
parentandchildrentwice. When the schema alreadyexposes 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 protocolswrites.Implementation notes:
ProtocolTargetselects betweenUSER_SCHEMA(the default, so every existing caller isunaffected) and
SDK_CORE.template.j2intoheader.j2, so the core module can renderone header followed by two bodies. The user branch of that header is byte-identical to before.
Synccounterpart. They are separatebecause 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 everyclass in it is local. That asymmetry is preserved rather than fixed, to keep user output stable.
What stayed the same:
infrahubctl protocolsoutput, apart from the duplicate lines above. This is enforced, notclaimed:
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.
CodeGenerator(schema=...)andrender(sync=...)are unchanged.How to review
Suggested order:
target.pyandgenerator.pyfor the mechanism.template.j2, which is only the header being removed,{{ suffix }}on the four class-namelines, and the hierarchical guard.
header.j2, where the user branch should match the block deleted fromtemplate.j2.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,CoreMenuand others.How to test
If
test_repository_app,test_task_app::test_task_list_commandortest_config::test_missing_passwordfail for you locally, that is environment leakage rather thanthis branch: they fail the same way on a clean
origin/infrahub-developcheckout, and CI is greenon all five Python versions.
Impact & rollout
duplicate
parent/childrenpair described above.the follow-up PR that regenerates
infrahub_sdk/protocols.py.Checklist