From 2b9edcf358c5ed2c5e2241788c553c3bef740a3d Mon Sep 17 00:00:00 2001 From: Damien Garros Date: Sun, 12 Jul 2026 08:51:40 +0000 Subject: [PATCH 1/4] fix: correct drifted async/sync twins and add parity regression tests The hand-maintained async/sync client twins had silently diverged. Fix the behavioral and annotation drift and lock the two behavioral fixes with tests: - branch: sync merge now applies the 120s minimum timeout floor like async - node: sync create(allow_upsert=True) excludes the hfid to avoid upsert overhead - node: async artifact fetch uses the FETCH (not GENERATE) unsupported message - node: fix sync resource-pool error text typo ("Allocate" -> "Allocated") - client: fix async filters docstring return type (InfrahubNode, not the Sync) - schema/node: converge private-helper annotations that drifted between twins Add regression tests covering the upsert-hfid exclusion and the merge timeout floor for both the async and sync clients. Co-Authored-By: Claude Opus 4.8 --- infrahub_sdk/branch.py | 4 +++- infrahub_sdk/client.py | 2 +- infrahub_sdk/node/node.py | 14 ++++++++------ infrahub_sdk/schema/__init__.py | 2 +- tests/unit/sdk/test_branch.py | 24 ++++++++++++++++++++++++ tests/unit/sdk/test_node.py | 33 +++++++++++++++++++++++++++++++++ 6 files changed, 70 insertions(+), 9 deletions(-) diff --git a/infrahub_sdk/branch.py b/infrahub_sdk/branch.py index 5f0eb8543..d62ef23df 100644 --- a/infrahub_sdk/branch.py +++ b/infrahub_sdk/branch.py @@ -323,7 +323,9 @@ def merge(self, branch_name: str) -> bool: } } query = Mutation(mutation="BranchMerge", input_data=input_data, query=MUTATION_QUERY_DATA) - response = self.client.execute_graphql(query=query.render(), tracker="mutation-branch-merge") + response = self.client.execute_graphql( + query=query.render(), tracker="mutation-branch-merge", timeout=max(120, self.client.default_timeout) + ) return response["BranchMerge"]["ok"] diff --git a/infrahub_sdk/client.py b/infrahub_sdk/client.py index a948ecc91..0b09bdcce 100644 --- a/infrahub_sdk/client.py +++ b/infrahub_sdk/client.py @@ -1073,7 +1073,7 @@ async def filters( # noqa: C901 **kwargs (Any): Additional filter criteria for the query. Returns: - list[InfrahubNodeSync]: List of Nodes that match the given filters. + list[InfrahubNode]: List of Nodes that match the given filters. """ branch = branch or self.default_branch diff --git a/infrahub_sdk/node/node.py b/infrahub_sdk/node/node.py index 9bdaa20cc..ebf8bb26b 100644 --- a/infrahub_sdk/node/node.py +++ b/infrahub_sdk/node/node.py @@ -1034,7 +1034,7 @@ async def artifact_fetch(self, name: str) -> str | dict[str, Any]: FeatureNotSupportedError: If this node does not inherit from ``CoreArtifactTarget``. """ - self._validate_artifact_support(ARTIFACT_GENERATE_FEATURE_NOT_SUPPORTED_MESSAGE) + self._validate_artifact_support(ARTIFACT_FETCH_FEATURE_NOT_SUPPORTED_MESSAGE) artifact = await self._client.get(kind="CoreArtifact", name__value=name, object__ids=[self.id]) return await self._client.object_store.get(identifier=artifact._get_attribute(name="storage_id").value) @@ -2083,7 +2083,7 @@ def from_graphql( return cls(client=client, schema=schema, branch=branch, data=cls._strip_alias(data)) - def _init_relationships(self, data: dict | None = None) -> None: + def _init_relationships(self, data: dict | RelatedNodeSync | None = None) -> None: for rel_schema in self._schema.relationships: rel_data = data.get(rel_schema.name, None) if isinstance(data, dict) else None @@ -2820,8 +2820,10 @@ def create( mutation_query = self._generate_mutation_query() + # Upserting means we may want to create, meaning payload contains all mandatory fields required for a creation, + # so hfid is just redondant information. Currently, upsert mutation has performance overhead if `hfid` is filled. if allow_upsert: - input_data = self._generate_input_data(exclude_hfid=False, request_context=request_context) + input_data = self._generate_input_data(exclude_hfid=True, request_context=request_context) mutation_name = f"{self._schema.kind}Upsert" tracker = f"mutation-{str(self._schema.kind).lower()}-upsert" else: @@ -2993,7 +2995,7 @@ def get_pool_allocated_resources(self, resource: InfrahubNodeSync) -> list[Infra """ if not self.is_resource_pool(): - raise ValueError("Allocate resources can only be fetched from resource pool nodes.") + raise ValueError("Allocated resources can only be fetched from resource pool nodes.") graphql_query_name = "InfrahubResourcePoolAllocated" node_ids_per_kind: dict[str, list[str]] = {} @@ -3089,13 +3091,13 @@ def get_pool_resources_utilization(self) -> list[dict[str, Any]]: return [edge["node"] for edge in response[graphql_query_name]["edges"]] return [] - def _get_relationship_many(self, name: str) -> RelationshipManager | RelationshipManagerSync: + def _get_relationship_many(self, name: str) -> RelationshipManagerSync: if name in self._relationship_cardinality_many_data: return self._relationship_cardinality_many_data[name] raise ResourceNotDefinedError(message=f"The node doesn't have a cardinality=many relationship for {name}") - def _get_relationship_one(self, name: str) -> RelatedNode | RelatedNodeSync: + def _get_relationship_one(self, name: str) -> RelatedNodeSync: if name in self._relationship_cardinality_one_data: return self._relationship_cardinality_one_data[name] diff --git a/infrahub_sdk/schema/__init__.py b/infrahub_sdk/schema/__init__.py index 82292e2a3..5343f24cd 100644 --- a/infrahub_sdk/schema/__init__.py +++ b/infrahub_sdk/schema/__init__.py @@ -408,7 +408,7 @@ async def check(self, schemas: list[dict], branch: str | None = None) -> tuple[b async def _get_kind_and_attribute_schema( self, kind: str | InfrahubNodeTypes, attribute: str, branch: str | None = None - ) -> tuple[str, AttributeSchema]: + ) -> tuple[str, AttributeSchemaAPI]: node_kind: str = kind._schema.kind if not isinstance(kind, str) else kind node_schema = await self.client.schema.get(kind=node_kind, branch=branch) schema_attr = node_schema.get_attribute(name=attribute) diff --git a/tests/unit/sdk/test_branch.py b/tests/unit/sdk/test_branch.py index 88f4a5305..a86182e9e 100644 --- a/tests/unit/sdk/test_branch.py +++ b/tests/unit/sdk/test_branch.py @@ -47,3 +47,27 @@ async def test_get_branches(clients: BothClients, mock_branches_list_query: HTTP assert len(branches) == 2 assert isinstance(branches["main"], BranchData) + + +@pytest.mark.parametrize("client_type", client_types) +async def test_branch_merge_enforces_minimum_timeout( + httpx_mock: HTTPXMock, clients: BothClients, client_type: str +) -> None: + """Both clients must apply the 120s minimum timeout floor when merging a branch. + + The default client timeout is 60s, so a correct merge sends max(120, 60) == 120. + """ + httpx_mock.add_response( + method="POST", + json={"data": {"BranchMerge": {"ok": True}}}, + match_headers={"X-Infrahub-Tracker": "mutation-branch-merge"}, + ) + + if client_type == "standard": + await clients.standard.branch.merge(branch_name="branch01") + else: + clients.sync.branch.merge(branch_name="branch01") + + post_requests = [r for r in httpx_mock.get_requests() if r.method == "POST"] + assert len(post_requests) == 1 + assert post_requests[0].extensions["timeout"]["read"] == 120 diff --git a/tests/unit/sdk/test_node.py b/tests/unit/sdk/test_node.py index ab0d72ec0..0a30cb7e3 100644 --- a/tests/unit/sdk/test_node.py +++ b/tests/unit/sdk/test_node.py @@ -2,6 +2,7 @@ import inspect import ipaddress +import json import tempfile from io import BytesIO from pathlib import Path @@ -177,6 +178,38 @@ async def test_node_hfid(client: InfrahubClient, schema_with_hfid: dict[str, Nod assert rack.hfid_str == "BuiltinRack__RACK1__JFK1" +@pytest.mark.parametrize("client_type", client_types) +async def test_node_create_upsert_excludes_hfid( + httpx_mock: HTTPXMock, + clients: BothClients, + schema_with_hfid: dict[str, NodeSchemaAPI], + client_type: str, +) -> None: + """Upsert must never send the hfid in the mutation payload, for both async and sync clients. + + Sending the hfid on an upsert adds server-side performance overhead, so create(allow_upsert=True) + excludes it. The async and sync clients must behave identically here. + """ + httpx_mock.add_response( + method="POST", + json={"data": {"BuiltinLocationUpsert": {"ok": True, "object": {"id": "abcabc"}}}}, + match_headers={"X-Infrahub-Tracker": "mutation-builtinlocation-upsert"}, + ) + location_data = {"name": {"value": "JFK1"}, "description": {"value": "JFK Airport"}, "type": {"value": "SITE"}} + + if client_type == "standard": + location = InfrahubNode(client=clients.standard, schema=schema_with_hfid["location"], data=location_data) + await location.create(allow_upsert=True) + else: + location = InfrahubNodeSync(client=clients.sync, schema=schema_with_hfid["location"], data=location_data) + location.create(allow_upsert=True) + + post_requests = [r for r in httpx_mock.get_requests() if r.method == "POST"] + assert len(post_requests) == 1 + payload = json.loads(post_requests[0].content) + assert "hfid" not in payload["query"] + + @pytest.mark.parametrize("client_type", client_types) async def test_init_node_data_user(client: InfrahubClient, location_schema: NodeSchemaAPI, client_type: str) -> None: data = { From 6cd5f047dd5fd72f8c7121f4c5a0a0a9c2cf0d8e Mon Sep 17 00:00:00 2001 From: Damien Garros Date: Sun, 12 Jul 2026 09:29:33 +0000 Subject: [PATCH 2/4] chore(changelog): add fragment for async/sync twin drift fixes Co-Authored-By: Claude Opus 4.8 --- changelog/+async-sync-twin-drift.fixed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/+async-sync-twin-drift.fixed.md diff --git a/changelog/+async-sync-twin-drift.fixed.md b/changelog/+async-sync-twin-drift.fixed.md new file mode 100644 index 000000000..4fb0d9fe7 --- /dev/null +++ b/changelog/+async-sync-twin-drift.fixed.md @@ -0,0 +1 @@ +Fixed several cases where `InfrahubClientSync` had drifted from `InfrahubClient`: sync `branch.merge` now applies the same 120-second minimum timeout floor as the async client, and sync `create(allow_upsert=True)` now excludes the `hfid` from the mutation payload to avoid server-side upsert overhead. Also corrected an incorrect "feature not supported" message on async artifact fetch and a resource-pool error message typo. From 51be5b353c9a7803399e6339002367c48b15eda6 Mon Sep 17 00:00:00 2001 From: Damien Garros Date: Sun, 12 Jul 2026 09:45:43 +0000 Subject: [PATCH 3/4] docs: regenerate SDK reference for client.filters return type Regenerated docs/docs/python-sdk/sdk_ref/infrahub_sdk/client.mdx to match the corrected InfrahubClient.filters docstring (list[InfrahubNode] instead of list[InfrahubNodeSync]) from the async/sync twin drift fix. Co-Authored-By: Claude Opus 4.8 --- docs/docs/python-sdk/sdk_ref/infrahub_sdk/client.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/python-sdk/sdk_ref/infrahub_sdk/client.mdx b/docs/docs/python-sdk/sdk_ref/infrahub_sdk/client.mdx index b4ff54106..e858f2179 100644 --- a/docs/docs/python-sdk/sdk_ref/infrahub_sdk/client.mdx +++ b/docs/docs/python-sdk/sdk_ref/infrahub_sdk/client.mdx @@ -312,7 +312,7 @@ Retrieve nodes of a given kind based on provided filters. **Returns:** -- list\[InfrahubNodeSync]: List of Nodes that match the given filters. +- list\[InfrahubNode]: List of Nodes that match the given filters. From 251c60846e994e9c105e768a6fa8b160c0b90088 Mon Sep 17 00:00:00 2001 From: Damien Garros Date: Sun, 12 Jul 2026 12:01:59 +0200 Subject: [PATCH 4/4] Update infrahub_sdk/node/node.py Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> --- infrahub_sdk/node/node.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infrahub_sdk/node/node.py b/infrahub_sdk/node/node.py index ebf8bb26b..b23f6f865 100644 --- a/infrahub_sdk/node/node.py +++ b/infrahub_sdk/node/node.py @@ -2821,7 +2821,7 @@ def create( mutation_query = self._generate_mutation_query() # Upserting means we may want to create, meaning payload contains all mandatory fields required for a creation, - # so hfid is just redondant information. Currently, upsert mutation has performance overhead if `hfid` is filled. + # so hfid is just redundant information. Currently, upsert mutation has performance overhead if `hfid` is filled. if allow_upsert: input_data = self._generate_input_data(exclude_hfid=True, request_context=request_context) mutation_name = f"{self._schema.kind}Upsert"