Make RelatedNode/RelationshipManager generic over peer type (#1063)#1065
Open
FragmentedPacket wants to merge 1 commit into
Open
Conversation
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 Report❌ Patch coverage is
@@ 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
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 30 files with indirect coverage changes 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1063. Stacked on top of #763 (base branch is #763's
re-sb-20260119-fix-typing-for-protocols-ihs-183); retarget todeveloponce #763 merges.Problem
RelatedNode.peer/RelationshipManagerare typed against the dynamicInfrahubNode, so traversing a relationship loses the peer's type. Any chain through.peer/.peerscollapses to the catch-allAttribute | RelationshipManager | RelatedNodeunion and fails under mypy/ty withunion-attr. This became visible to downstream consumers whenpy.typedwas added in 1.19.0.Change
RelatedNode,RelatedNodeSync,RelationshipManager,RelationshipManagerSyncare now generic over the peer type (PeerT/PeerTSync), defaulting toInfrahubNode/InfrahubNodeSync(PEP 696) so all existing un-parameterised usage is unchanged.infrahubctl protocolsparameterises 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
mypy(125 files) ✅,ty✅,ruff✅, generator test updated, 252 node/relationship/store unit tests ✅.union-attrerrors went from 52 → 0. (Remaining downstream errors are a separate relationship-assignmentissue, bug: assigning to a relationship attribute fails mypy (assignment) for id strings and nodes #1064, plus genuinely-unsound accesses now correctly surfaced, e.g. accessingNetworkInterface-only attributes on a genericNetworkEndpointpeer.)Notes
infrahub_sdk/protocols.pyis left as-is (bare relationships still resolve via theInfrahubNodedefault); it can be regenerated with the new generator separately.🤖 Generated with Claude Code
Summary by cubic
Make
RelatedNode/RelatedNodeSyncandRelationshipManager/RelationshipManagerSyncgeneric over their peer type so.peer,.peers, and indexing preserve the peer's concrete type. Also updateinfrahubctl protocolsto emit parameterized relationships; existing code stays compatible viaInfrahubNode/InfrahubNodeSyncdefaults, fixing #1063.InfrahubNode/InfrahubNodeSync, so un-parameterized usage continues to work.RelatedNode[Peer]andRelationshipManager[Peer]; sync output referencesPeerSyncwhen available.Written for commit 49bd3ed. Summary will update on new commits.