From df30f2747b0974539d83c2d0ff40253d753ac7db Mon Sep 17 00:00:00 2001 From: Phillip Simonds Date: Thu, 9 Jul 2026 11:56:05 -0600 Subject: [PATCH] fix: clearer error when fetching/editing a relationship on an id-less node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fetching or editing a cardinality-many relationship on a node that has no ID (e.g. created with client.create() but never saved) produced confusing errors: fetch() surfaced the generic "At least one filter must be provided to get()" from the internal client.get(id=None) call, and add()/remove() told the user to call fetch() — which could not succeed either, sending them in a circle. fetch(), add(), and remove() now detect the missing node ID up front and raise a clear UninitializedError that nudges the user to call .save() first. When the node does have an ID, the existing "call fetch()" guidance is preserved. Applied to both RelationshipManager and RelationshipManagerSync. Fixes #1153 Co-Authored-By: Claude Opus 4.8 (1M context) --- changelog/1153.fixed.md | 1 + infrahub_sdk/node/relationship.py | 33 ++++++++++++++++++++++- tests/unit/sdk/test_node.py | 44 ++++++++++++++++++++++++++++++- 3 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 changelog/1153.fixed.md diff --git a/changelog/1153.fixed.md b/changelog/1153.fixed.md new file mode 100644 index 000000000..60411a1d0 --- /dev/null +++ b/changelog/1153.fixed.md @@ -0,0 +1 @@ +Fetching or editing a relationship on a node that has no ID (e.g. a node created locally but not yet saved) now raises a clear error explaining the node needs to be saved first, instead of the generic `At least one filter must be provided to get()` message (for `fetch()`) or sending the caller in a circle between `add()` and `fetch()`. diff --git a/infrahub_sdk/node/relationship.py b/infrahub_sdk/node/relationship.py index 7069b13ee..f4bc6f4c0 100644 --- a/infrahub_sdk/node/relationship.py +++ b/infrahub_sdk/node/relationship.py @@ -2,7 +2,7 @@ from collections import defaultdict from collections.abc import Iterable -from typing import TYPE_CHECKING, Any, Generic, cast +from typing import TYPE_CHECKING, Any, Generic, NoReturn, cast from ..exceptions import ( Error, @@ -19,6 +19,25 @@ from .node import InfrahubNode, InfrahubNodeSync +def _raise_missing_identifier(node: InfrahubNode | InfrahubNodeSync, name: str) -> NoReturn: + """Raise a clear error for fetching/editing a relationship on a node with no ID. + + A relationship can only be fetched or edited once the parent node has an ID to look it up + by. When it does not, the underlying ``client.get()`` call would otherwise fail with a + generic "At least one filter must be provided to get()" message that gives the caller no + hint about the real cause. + + Raises: + UninitializedError: Always; the parent node has no ID to look the relationship up by. + + """ + raise UninitializedError( + f"Cannot access the '{name}' relationship because the {node._schema.kind} node has no ID to " + f"look it up by. This usually means the node was created locally but not saved yet — call " + f".save() on it first (or fetch it from Infrahub) before fetching or editing its relationships." + ) + + class RelationshipManagerBase(Generic[PeerT]): """Base class for :class:`RelationshipManager` and :class:`RelationshipManagerSync`. @@ -243,6 +262,8 @@ async def fetch(self) -> None: """ if not self.initialized: + if not self.node.id: + _raise_missing_identifier(self.node, self.name) exclude = self.node._schema.relationship_names + self.node._schema.attribute_names exclude.remove(self.schema.name) node = await self.client.get( @@ -293,6 +314,8 @@ def add(self, data: str | RelatedNode | dict) -> None: """ if not self.initialized: + if not self.node.id: + _raise_missing_identifier(self.node, self.name) raise UninitializedError("Must call fetch() on RelationshipManager before editing members") new_node = cast( "RelatedNode[PeerT]", RelatedNode(schema=self.schema, client=self.client, branch=self.branch, data=data) @@ -337,6 +360,8 @@ def remove(self, data: str | RelatedNode | dict) -> None: """ if not self.initialized: + if not self.node.id: + _raise_missing_identifier(self.node, self.name) raise UninitializedError("Must call fetch() on RelationshipManager before editing members") node_to_remove = RelatedNode(schema=self.schema, client=self.client, branch=self.branch, data=data) @@ -440,6 +465,8 @@ def fetch(self) -> None: """ if not self.initialized: + if not self.node.id: + _raise_missing_identifier(self.node, self.name) exclude = self.node._schema.relationship_names + self.node._schema.attribute_names exclude.remove(self.schema.name) node = self.client.get( @@ -490,6 +517,8 @@ def add(self, data: str | RelatedNodeSync | dict) -> None: """ if not self.initialized: + if not self.node.id: + _raise_missing_identifier(self.node, self.name) raise UninitializedError("Must call fetch() on RelationshipManager before editing members") new_node = cast( "RelatedNodeSync[PeerTSync]", @@ -535,6 +564,8 @@ def remove(self, data: str | RelatedNodeSync | dict) -> None: """ if not self.initialized: + if not self.node.id: + _raise_missing_identifier(self.node, self.name) raise UninitializedError("Must call fetch() on RelationshipManager before editing members") node_to_remove = RelatedNodeSync(schema=self.schema, client=self.client, branch=self.branch, data=data) diff --git a/tests/unit/sdk/test_node.py b/tests/unit/sdk/test_node.py index ab0d72ec0..1d44ce01b 100644 --- a/tests/unit/sdk/test_node.py +++ b/tests/unit/sdk/test_node.py @@ -9,7 +9,7 @@ import pytest -from infrahub_sdk.exceptions import FeatureNotSupportedError, NodeNotFoundError +from infrahub_sdk.exceptions import FeatureNotSupportedError, NodeNotFoundError, UninitializedError from infrahub_sdk.node import ( InfrahubNode, InfrahubNodeBase, @@ -342,6 +342,48 @@ async def test_cardinality_many_accepts_list( assert len(node.tags.peers) == 2 +@pytest.mark.parametrize("client_type", client_types) +async def test_fetch_relationship_without_node_id_raises_clear_error( + client: InfrahubClient, location_schema: NodeSchemaAPI, client_type: str +) -> None: + """fetch() on a node with no ID explains it isn't saved instead of raising the generic get() filter error.""" + if client_type == "standard": + node = InfrahubNode(client=client, schema=location_schema, data={"name": {"value": "JFK1"}}) + with pytest.raises(UninitializedError, match=r"has no ID"): + await node.tags.fetch() + else: + node = InfrahubNodeSync(client=client, schema=location_schema, data={"name": {"value": "JFK1"}}) + with pytest.raises(UninitializedError, match=r"has no ID"): + node.tags.fetch() + + +@pytest.mark.parametrize("client_type", client_types) +async def test_edit_relationship_without_node_id_raises_clear_error( + client: InfrahubClient, location_schema: NodeSchemaAPI, client_type: str +) -> None: + """add() on a node with no ID points the user at .save() instead of at fetch(), avoiding the circular guidance.""" + if client_type == "standard": + node = InfrahubNode(client=client, schema=location_schema, data={"name": {"value": "JFK1"}}) + else: + node = InfrahubNodeSync(client=client, schema=location_schema, data={"name": {"value": "JFK1"}}) + with pytest.raises(UninitializedError, match=r"has no ID"): + node.tags.add("11111111-1111-1111-1111-111111111111") + + +@pytest.mark.parametrize("client_type", client_types) +async def test_edit_relationship_with_node_id_still_requires_fetch( + client: InfrahubClient, location_schema: NodeSchemaAPI, client_type: str +) -> None: + """When the node has an ID but the relationship isn't loaded, the original "call fetch()" guidance is preserved.""" + data = {"id": "22222222-2222-2222-2222-222222222222", "name": {"value": "JFK1"}} + if client_type == "standard": + node = InfrahubNode(client=client, schema=location_schema, data=data) + else: + node = InfrahubNodeSync(client=client, schema=location_schema, data=data) + with pytest.raises(UninitializedError, match=r"Must call fetch"): + node.tags.add("11111111-1111-1111-1111-111111111111") + + @pytest.mark.parametrize("client_type", client_types) async def test_query_data_no_filters_property( clients: BothClients, location_schema: NodeSchemaAPI, client_type: str