Skip to content

fix(protocols): syncify all inherited bases with a Sync variant - #1331

Open
ogenstad wants to merge 2 commits into
infrahub-developfrom
pog-em/generated-sync-ihs-279-pfv2d
Open

fix(protocols): syncify all inherited bases with a Sync variant#1331
ogenstad wants to merge 2 commits into
infrahub-developfrom
pog-em/generated-sync-ihs-279-pfv2d

Conversation

@ogenstad

@ogenstad ogenstad commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Why

Protocols generated for a sync client could inherit an async base class. In an inheritance list the generator only switched CoreNode, CoreProfile and CoreObjectTemplate to their *Sync variant. Every other core base (for example BuiltinIPAddress or LineageSource) kept its async name, so the generated sync class ended up with an async base in its MRO. Its save, delete and create then type checked as async, and a correct sync call was reported as a missing await. Only typing was affected; nothing changed at runtime.

Closes #1276

What changed

  • Sync protocols from infrahubctl protocols now inherit the *Sync variant of every base that has one, not just the three special-cased names. Sync classes that inherited an async base (BuiltinIPAddress, BuiltinIPPrefix, LineageSource, ...) now expose the sync save/delete/create.
  • Collapsed the two separate name sets (_inherited_sync_names, _peer_sync_names) into a single _sync_names used by both the inheritance and the peer branch. CORE_BASE_CLASS_TO_SYNCIFY is now only used to order the core bases last.

What stayed the same

  • Async output is byte-identical: user_schema_async.txt does not change.
  • The SDK core protocols path (ProtocolTarget.SDK_CORE, which generates the committed infrahub_sdk/protocols.py) already switched every local base and is unchanged.

How to review

The review surface is the golden fixture diff in tests/fixtures/protocols_generator/user_schema_sync.txt: exactly 14 references change (BuiltinIPAddress and BuiltinIPPrefix once each, LineageSource twelve times) and nothing else.

How to test

uv run pytest tests/unit/sdk/test_protocols_generator.py

Impact & rollout

  • Backward compatibility: No runtime change. Users who regenerate protocols after this fix may see new type errors in their own code, because a sync class that previously exposed async save/delete/create now exposes the sync ones. That is the intended correction and is called out in the changelog.

Checklist

  • Tests added/updated
  • Changelog entry added
  • External docs updated (not applicable)
  • Internal .md docs updated (not applicable)

Summary by cubic

Fixes generated sync protocols that could inherit an async base class. Previously only CoreNode, CoreProfile, and CoreObjectTemplate were switched to their *Sync variant in inheritance lists, so a class inheriting BuiltinIPAddress or LineageSource kept async save/delete/create and type checkers reported a missing await on correct sync calls. Now every inherited base with a *Sync variant is switched, and CoreNode is also switched as a relationship peer. Regenerating protocols may surface new type errors in code that relied on the old async signatures; runtime behavior is unchanged.

Refactors

  • Merges the inheritance and relationship-peer name sets into a single _sync_names set used by both positions.
  • Includes CoreNode explicitly because it is the implicit base and is not part of base_protocols.
  • Adds test cases pinning peer sync-switching, including a CoreNode peer.

Written for commit 7c11882. Summary will update on new commits.

Review in cubic

Sync protocols generated by infrahubctl protocols only switched CoreNode,
CoreProfile and CoreObjectTemplate to their Sync variant in an inheritance
list. Any other core base (for example BuiltinIPAddress or LineageSource)
kept its async name, so the sync class inherited async save/delete/create
and type checked as needing an await.

Collapse the two name sets into one that covers both peers and inheritance
bases, switching any base that has a Sync counterpart.
@ogenstad ogenstad added the type/bug Something isn't working as expected label Sep 7, 2026
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 7, 2026

Copy link
Copy Markdown

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7c11882
Status: ✅  Deploy successful!
Preview URL: https://c645223f.infrahub-sdk-python.pages.dev
Branch Preview URL: https://pog-em-generated-sync-ihs-27.infrahub-sdk-python.pages.dev

View logs

@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@                 Coverage Diff                  @@
##           infrahub-develop    #1331      +/-   ##
====================================================
+ Coverage             85.33%   85.35%   +0.01%     
====================================================
  Files                   148      148              
  Lines                 14085    14082       -3     
  Branches               1936     1936              
====================================================
- Hits                  12020    12019       -1     
+ Misses                 1499     1498       -1     
+ Partials                566      565       -1     
Flag Coverage Δ
integration-tests 43.43% <0.00%> (+0.02%) ⬆️
python-3.10 60.15% <100.00%> (-0.03%) ⬇️
python-3.11 60.16% <100.00%> (+<0.01%) ⬆️
python-3.12 60.15% <100.00%> (-0.01%) ⬇️
python-3.13 60.16% <100.00%> (-0.01%) ⬇️
python-3.14 60.16% <100.00%> (-0.01%) ⬇️
python-filler-3.12 21.94% <0.00%> (+<0.01%) ⬆️

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

Files with missing lines Coverage Δ
infrahub_sdk/protocols_generator/generator.py 87.96% <100.00%> (-0.33%) ⬇️

... and 2 files with indirect coverage changes

🚀 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.

No issues found across 4 files

Re-trigger cubic

@ogenstad
ogenstad marked this pull request as ready for review September 8, 2026 07:41
@ogenstad
ogenstad requested a review from a team as a code owner September 8, 2026 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/bug Something isn't working as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants