Skip to content

Make RelatedNode/RelationshipManager generic over peer type (#1063)#1065

Open
FragmentedPacket wants to merge 1 commit into
re-sb-20260119-fix-typing-for-protocols-ihs-183from
ihs-relatednode-generic-1063
Open

Make RelatedNode/RelationshipManager generic over peer type (#1063)#1065
FragmentedPacket wants to merge 1 commit into
re-sb-20260119-fix-typing-for-protocols-ihs-183from
ihs-relatednode-generic-1063

Conversation

@FragmentedPacket
Copy link
Copy Markdown
Contributor

@FragmentedPacket FragmentedPacket commented Jun 5, 2026

Fixes #1063. Stacked on top of #763 (base branch is #763's re-sb-20260119-fix-typing-for-protocols-ihs-183); retarget to develop once #763 merges.

Problem

RelatedNode.peer / RelationshipManager are typed against the dynamic InfrahubNode, so traversing a relationship loses the peer's type. Any chain through .peer / .peers collapses to the catch-all Attribute | RelationshipManager | RelatedNode union and fails under mypy/ty with union-attr. This became visible to downstream consumers when py.typed was added in 1.19.0.

Change

  • RelatedNode, RelatedNodeSync, RelationshipManager, RelationshipManagerSync are now generic over the peer type (PeerT / PeerTSync), defaulting to InfrahubNode / InfrahubNodeSync (PEP 696) so all existing un-parameterised usage is unchanged.
  • infrahubctl protocols parameterises each relationship: device: RelatedNode[NetworkDevice], interfaces: RelationshipManager[NetworkInterface], etc. Core peers are syncified in sync output (tags: RelationshipManagerSync[BuiltinTagSync]); locally generated classes keep their name.
  • Generic[...] is a typing-only change — zero runtime cost.

Verification

  • SDK: mypy (125 files) ✅, ty ✅, ruff ✅, generator test updated, 252 node/relationship/store unit tests ✅.
  • Against a downstream repo (the AI/DC solution) on the released line: relationship traversal union-attr errors went from 52 → 0. (Remaining downstream errors are a separate relationship-assignment issue, bug: assigning to a relationship attribute fails mypy (assignment) for id strings and nodes #1064, plus genuinely-unsound accesses now correctly surfaced, e.g. accessing NetworkInterface-only attributes on a generic NetworkEndpoint peer.)

Notes

  • The bundled infrahub_sdk/protocols.py is left as-is (bare relationships still resolve via the InfrahubNode default); it can be regenerated with the new generator separately.

🤖 Generated with Claude Code


Summary by cubic

Make RelatedNode/RelatedNodeSync and RelationshipManager/RelationshipManagerSync generic over their peer type so .peer, .peers, and indexing preserve the peer's concrete type. Also update infrahubctl protocols to emit parameterized relationships; existing code stays compatible via InfrahubNode/InfrahubNodeSync defaults, fixing #1063.

  • New Features
    • Defaults use PEP 696 to InfrahubNode/InfrahubNodeSync, so un-parameterized usage continues to work.
    • Protocol generator now outputs RelatedNode[Peer] and RelationshipManager[Peer]; sync output references PeerSync when available.

Written for commit 49bd3ed. Summary will update on new commits.

Review in cubic

Parameterise RelatedNode, RelatedNodeSync, RelationshipManager and
RelationshipManagerSync over the peer type (defaulting to InfrahubNode/
InfrahubNodeSync for backward compatibility) and have infrahubctl protocols
emit the peer type for each relationship. Relationship traversal via .peer/
.peers/indexing now preserves the peer type instead of collapsing to the
dynamic InfrahubNode.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 5, 2026

Codecov Report

❌ Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
infrahub_sdk/node/related_node.py 87.50% 2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (6c5bd98) and HEAD (49bd3ed). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (6c5bd98) HEAD (49bd3ed)
integration-tests 1 0
@@                                 Coverage Diff                                 @@
##           re-sb-20260119-fix-typing-for-protocols-ihs-183    #1065      +/-   ##
===================================================================================
- Coverage                                            80.38%   72.38%   -8.01%     
===================================================================================
  Files                                                  115      115              
  Lines                                                 9894     9899       +5     
  Branches                                              1517     1518       +1     
===================================================================================
- Hits                                                  7953     7165     -788     
- Misses                                                1418     2228     +810     
+ Partials                                               523      506      -17     
Flag Coverage Δ
integration-tests ?
python-3.10 51.40% <42.42%> (-0.01%) ⬇️
python-3.11 51.40% <42.42%> (-0.03%) ⬇️
python-3.12 51.42% <42.42%> (+0.01%) ⬆️
python-3.13 51.40% <42.42%> (-0.01%) ⬇️
python-3.14 53.07% <42.42%> (+0.01%) ⬆️
python-filler-3.12 24.06% <51.51%> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
infrahub_sdk/node/relationship.py 70.87% <100.00%> (-8.80%) ⬇️
infrahub_sdk/protocols_generator/generator.py 95.12% <100.00%> (+0.12%) ⬆️
infrahub_sdk/node/related_node.py 84.57% <87.50%> (-7.88%) ⬇️

... and 30 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

No issues found across 6 files

Re-trigger cubic

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