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. 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 038599660..0d9445a17 100644 --- a/docs/docs/python-sdk/sdk_ref/infrahub_sdk/client.mdx +++ b/docs/docs/python-sdk/sdk_ref/infrahub_sdk/client.mdx @@ -320,7 +320,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. 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 ac61abdc2..7c5c10bbd 100644 --- a/infrahub_sdk/client.py +++ b/infrahub_sdk/client.py @@ -1104,7 +1104,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..b23f6f865 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 redundant 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 = {