Skip to content

Commit d748a00

Browse files
authored
IHS-156 / #497 : support from_pool attributes on Python SDK queries (#850)
* IHS-156 fix SDK from_pool attribute management before querying GraphQL API * IHS-156 refactor the test cases to have a better view on the tests perimeter * towncrier regarding Github issue #497 * IHS-156 update AGENTS doc using the command /feedback * IHS-156 is_from_pool_attribute typing and naming * IHS-156 refactor generate_input_data * IHS-156 remove Jira issue related comment in the tests * IHS-156 removed a part of fixtures to upper level * IHS-156 tested all cases of generated_input_data * IHS-156 fixed a bug in test_relationship_from_pool.py * IHS-156 side effect bugs regarding from_pool attributes * IHS-156 last feedbacks regarding documentation and variable naming * IHS-156 fix typing error * IHS-156 renamed payload_dict -> payload
1 parent 998e3fe commit d748a00

16 files changed

Lines changed: 1386 additions & 657 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ Infrahub Python SDK - async/sync client for Infrahub infrastructure management.
77
```bash
88
uv sync --all-groups --all-extras # Install all deps
99
uv run invoke format # Format code
10-
uv run invoke lint # All linters (code + yamllint + documentation)
10+
uv run invoke lint # Full pipeline: ruff, yamllint, ty, mypy, markdownlint, vale
1111
uv run invoke lint-code # All linters for Python code
1212
uv run pytest tests/unit/ # Unit tests
1313
uv run pytest tests/integration/ # Integration tests

changelog/497.fixed.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed Python SDK query generation regarding from_pool generated attribute value

docs/docs/python-sdk/sdk_ref/infrahub_sdk/node/attribute.mdx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,15 @@ value(self) -> Any
2424
```python
2525
value(self, value: Any) -> None
2626
```
27+
28+
#### `is_from_pool_attribute`
29+
30+
```python
31+
is_from_pool_attribute(self) -> bool
32+
```
33+
34+
Check whether this attribute's value is sourced from a resource pool.
35+
36+
**Returns:**
37+
38+
- True if the attribute value is a resource pool node or was explicitly allocated from a pool.

infrahub_sdk/node/attribute.py

Lines changed: 89 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import ipaddress
44
from collections.abc import Callable
5-
from typing import TYPE_CHECKING, Any, get_args
5+
from typing import TYPE_CHECKING, Any, NamedTuple, get_args
66

77
from ..protocols_base import CoreNodeBase
88
from ..uuidt import UUIDT
@@ -13,6 +13,33 @@
1313
from ..schema import AttributeSchemaAPI
1414

1515

16+
class _GraphQLPayloadAttribute(NamedTuple):
17+
"""Result of resolving an attribute value for a GraphQL mutation.
18+
19+
Attributes:
20+
payload: Key/value entries to include in the mutation payload
21+
(e.g. ``{"value": ...}`` or ``{"from_pool": ...}``).
22+
variables: GraphQL variable bindings for unsafe string values.
23+
needs_metadata: When ``True``, the payload needs to append property flags/objects
24+
"""
25+
26+
payload: dict[str, Any]
27+
variables: dict[str, Any]
28+
needs_metadata: bool
29+
30+
def to_dict(self) -> dict[str, Any]:
31+
return {"data": self.payload, "variables": self.variables}
32+
33+
def add_properties(self, properties_flag: dict[str, Any], properties_object: dict[str, str | None]) -> None:
34+
if not self.needs_metadata:
35+
return
36+
for prop_name, prop in properties_flag.items():
37+
self.payload[prop_name] = prop
38+
39+
for prop_name, prop in properties_object.items():
40+
self.payload[prop_name] = prop
41+
42+
1643
class Attribute:
1744
"""Represents an attribute of a Node, including its schema, value, and properties."""
1845

@@ -25,8 +52,12 @@ def __init__(self, name: str, schema: AttributeSchemaAPI, data: Any | dict) -> N
2552
"""
2653
self.name = name
2754
self._schema = schema
55+
self._from_pool: dict[str, Any] | None = None
2856

29-
if not isinstance(data, dict) or "value" not in data:
57+
if isinstance(data, dict) and "from_pool" in data:
58+
self._from_pool = data.pop("from_pool")
59+
data.setdefault("value", None)
60+
elif not isinstance(data, dict) or "value" not in data:
3061
data = {"value": data}
3162

3263
self._properties_flag = PROPERTIES_FLAG
@@ -76,38 +107,55 @@ def value(self, value: Any) -> None:
76107
self._value = value
77108
self.value_has_been_mutated = True
78109

79-
def _generate_input_data(self) -> dict | None:
80-
data: dict[str, Any] = {}
81-
variables: dict[str, Any] = {}
82-
83-
if self.value is None:
84-
if self._schema.optional and self.value_has_been_mutated:
85-
data["value"] = None
86-
return data
87-
88-
if isinstance(self.value, str):
89-
if SAFE_VALUE.match(self.value):
90-
data["value"] = self.value
91-
else:
92-
var_name = f"value_{UUIDT.new().hex}"
93-
variables[var_name] = self.value
94-
data["value"] = f"${var_name}"
95-
elif isinstance(self.value, get_args(IP_TYPES)):
96-
data["value"] = self.value.with_prefixlen
97-
elif isinstance(self.value, CoreNodeBase) and self.value.is_resource_pool():
98-
data["from_pool"] = {"id": self.value.id}
99-
else:
100-
data["value"] = self.value
101-
102-
for prop_name in self._properties_flag:
103-
if getattr(self, prop_name) is not None:
104-
data[prop_name] = getattr(self, prop_name)
110+
def _initialize_graphql_payload(self) -> _GraphQLPayloadAttribute:
111+
"""Resolve the attribute value into a GraphQL mutation payload object."""
105112

106-
for prop_name in self._properties_object:
107-
if getattr(self, prop_name) is not None:
108-
data[prop_name] = getattr(self, prop_name)._generate_input_data()
113+
# Pool-based allocation (dict data or resource-pool node)
114+
if self._from_pool is not None:
115+
return _GraphQLPayloadAttribute(payload={"from_pool": self._from_pool}, variables={}, needs_metadata=True)
116+
if isinstance(self.value, CoreNodeBase) and self.value.is_resource_pool():
117+
return _GraphQLPayloadAttribute(
118+
payload={"from_pool": {"id": self.value.id}}, variables={}, needs_metadata=True
119+
)
109120

110-
return {"data": data, "variables": variables}
121+
# Null value
122+
if self.value is None:
123+
data = {"value": None} if (self._schema.optional and self.value_has_been_mutated) else {}
124+
return _GraphQLPayloadAttribute(payload=data, variables={}, needs_metadata=False)
125+
126+
# Unsafe strings need a variable binding to avoid injection
127+
if isinstance(self.value, str) and not SAFE_VALUE.match(self.value):
128+
var_name = f"value_{UUIDT.new().hex}"
129+
return _GraphQLPayloadAttribute(
130+
payload={"value": f"${var_name}"},
131+
variables={var_name: self.value},
132+
needs_metadata=True,
133+
)
134+
135+
# Safe strings, IP types, and everything else
136+
value = self.value.with_prefixlen if isinstance(self.value, get_args(IP_TYPES)) else self.value
137+
return _GraphQLPayloadAttribute(payload={"value": value}, variables={}, needs_metadata=True)
138+
139+
def _generate_input_data(self) -> _GraphQLPayloadAttribute:
140+
"""Build the input payload for a GraphQL mutation on this attribute.
141+
142+
Returns a ResolvedValue object, which contains all the data required.
143+
"""
144+
graphql_payload = self._initialize_graphql_payload()
145+
146+
properties_flag: dict[str, Any] = {
147+
property_name: getattr(self, property_name)
148+
for property_name in self._properties_flag
149+
if getattr(self, property_name) is not None
150+
}
151+
properties_object: dict[str, str | None] = {
152+
property_name: getattr(self, property_name)._generate_input_data()
153+
for property_name in self._properties_object
154+
if getattr(self, property_name) is not None
155+
}
156+
graphql_payload.add_properties(properties_flag, properties_object)
157+
158+
return graphql_payload
111159

112160
def _generate_query_data(self, property: bool = False, include_metadata: bool = False) -> dict | None:
113161
data: dict[str, Any] = {"value": None}
@@ -128,7 +176,15 @@ def _generate_query_data(self, property: bool = False, include_metadata: bool =
128176
return data
129177

130178
def _generate_mutation_query(self) -> dict[str, Any]:
131-
if isinstance(self.value, CoreNodeBase) and self.value.is_resource_pool():
179+
if self.is_from_pool_attribute():
132180
# If it points to a pool, ask for the value of the pool allocated resource
133181
return {self.name: {"value": None}}
134182
return {}
183+
184+
def is_from_pool_attribute(self) -> bool:
185+
"""Check whether this attribute's value is sourced from a resource pool.
186+
187+
Returns:
188+
True if the attribute value is a resource pool node or was explicitly allocated from a pool.
189+
"""
190+
return (isinstance(self.value, CoreNodeBase) and self.value.is_resource_pool()) or self._from_pool is not None

infrahub_sdk/node/node.py

Lines changed: 10 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ def is_resource_pool(self) -> bool:
216216
def get_raw_graphql_data(self) -> dict | None:
217217
return self._data
218218

219-
def _generate_input_data( # noqa: C901, PLR0915
219+
def _generate_input_data( # noqa: C901
220220
self,
221221
exclude_unmodified: bool = False,
222222
exclude_hfid: bool = False,
@@ -228,27 +228,18 @@ def _generate_input_data( # noqa: C901, PLR0915
228228
dict[str, Dict]: Representation of an input data in dict format
229229
"""
230230

231-
data = {}
232-
variables = {}
231+
data: dict[str, Any] = {}
232+
variables: dict[str, Any] = {}
233233

234234
for item_name in self._attributes:
235235
attr: Attribute = getattr(self, item_name)
236236
if attr._schema.read_only:
237237
continue
238-
attr_data = attr._generate_input_data()
239-
240-
# NOTE, this code has been inherited when we splitted attributes and relationships
241-
# into 2 loops, most likely it's possible to simply it
242-
if attr_data and isinstance(attr_data, dict):
243-
if variable_values := attr_data.get("data"):
244-
data[item_name] = variable_values
245-
else:
246-
data[item_name] = attr_data
247-
if variable_names := attr_data.get("variables"):
248-
variables.update(variable_names)
249-
250-
elif attr_data and isinstance(attr_data, list):
251-
data[item_name] = attr_data
238+
graphql_payload = attr._generate_input_data()
239+
if graphql_payload.payload:
240+
data[item_name] = graphql_payload.payload
241+
if graphql_payload.variables:
242+
variables.update(graphql_payload.variables)
252243

253244
for item_name in self._relationships:
254245
allocate_from_pool = False
@@ -1011,11 +1002,7 @@ async def _process_mutation_result(
10111002

10121003
for attr_name in self._attributes:
10131004
attr = getattr(self, attr_name)
1014-
if (
1015-
attr_name not in object_response
1016-
or not isinstance(attr.value, InfrahubNodeBase)
1017-
or not attr.value.is_resource_pool()
1018-
):
1005+
if attr_name not in object_response or not attr.is_from_pool_attribute():
10191006
continue
10201007

10211008
# Process allocated resource from a pool and update attribute
@@ -1819,11 +1806,7 @@ def _process_mutation_result(
18191806

18201807
for attr_name in self._attributes:
18211808
attr = getattr(self, attr_name)
1822-
if (
1823-
attr_name not in object_response
1824-
or not isinstance(attr.value, InfrahubNodeBase)
1825-
or not attr.value.is_resource_pool()
1826-
):
1809+
if attr_name not in object_response or not attr.is_from_pool_attribute():
18271810
continue
18281811

18291812
# Process allocated resource from a pool and update attribute

tests/AGENTS.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@ uv run pytest tests/unit/test_client.py # Single file
1717
```text
1818
tests/
1919
├── unit/ # Fast, mocked, no external deps
20+
│ ├── ctl/ # CLI command tests
21+
│ └── sdk/ # SDK tests
22+
│ ├── pool/ # Resource pool allocation tests
23+
│ ├── spec/ # Object spec tests
24+
│ ├── checks/ # InfrahubCheck tests
25+
│ └── ... # Core SDK tests (client, node, schema, etc.)
2026
├── integration/ # Real Infrahub via testcontainers
2127
├── fixtures/ # Test data (JSON, YAML)
2228
└── helpers/ # Test utilities

0 commit comments

Comments
 (0)