Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/+async-sync-twin-drift.fixed.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion docs/docs/python-sdk/sdk_ref/infrahub_sdk/client.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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.

</details>

Expand Down
4 changes: 3 additions & 1 deletion infrahub_sdk/branch.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

Expand Down
2 changes: 1 addition & 1 deletion infrahub_sdk/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 8 additions & 6 deletions infrahub_sdk/node/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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]] = {}
Expand Down Expand Up @@ -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]

Expand Down
2 changes: 1 addition & 1 deletion infrahub_sdk/schema/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
24 changes: 24 additions & 0 deletions tests/unit/sdk/test_branch.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
33 changes: 33 additions & 0 deletions tests/unit/sdk/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import inspect
import ipaddress
import json
import tempfile
from io import BytesIO
from pathlib import Path
Expand Down Expand Up @@ -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 = {
Expand Down