fix: resolve from_pool string names to pool IDs in object spec#862
fix: resolve from_pool string names to pool IDs in object spec#862FragmentedPacket wants to merge 1 commit intoinfrahub-developfrom
Conversation
…sing
When loading objects via YAML spec files, attributes using `from_pool`
with a plain string pool name (e.g. `{"from_pool": "my-vlan-pool"}`)
were passed through as-is, causing failures because the API expects
`{"from_pool": {"id": "<uuid>"}}`.
Add `_resolve_attribute_pool` to `InfrahubObjectFileData` which looks up
the pool by HFID and replaces the string with the resolved `{id: uuid}`
form before creating the node. Currently supports Number attributes
via `CoreNumberPool`.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
github-actions
bot
added
the
type/documentation
WalkthroughThe changes introduce functionality to resolve attribute values that reference pools during node creation. A module-level mapping is added to associate attribute kinds with their corresponding pool kinds. A new classmethod 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying infrahub-sdk-python with
|
| Latest commit: |
db8b798
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://27ded294.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://object-templates-vlan-pools.infrahub-sdk-python.pages.dev |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## infrahub-develop #862 +/- ##
====================================================
- Coverage 80.64% 80.57% -0.07%
====================================================
Files 118 118
Lines 10246 10262 +16
Branches 1534 1538 +4
====================================================
+ Hits 8263 8269 +6
- Misses 1455 1465 +10
Partials 528 528
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infrahub_sdk/spec/object.py`:
- Around line 433-455: The _resolve_attribute_pool flow currently always treats
a string from_pool as an HFID lookup; update
InfrahubObject._resolve_attribute_pool to first detect if from_pool is a UUID
string and, if so, return the value with from_pool set to {"id": from_pool}
immediately (before calling client.get). Use the existing local variable
from_pool and the attr_schema/_ATTRIBUTE_KIND_TO_POOL_KIND checks as before;
only call client.get(kind=pool_kind, hfid=[from_pool], ...) when from_pool is
not a UUID.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b7bdfb09-e8e1-448e-a558-25ffa43574d9
📒 Files selected for processing (1)
infrahub_sdk/spec/object.py
| @classmethod | ||
| async def _resolve_attribute_pool( | ||
| cls, | ||
| client: InfrahubClient, | ||
| schema: MainSchemaTypesAPI, | ||
| name: str, | ||
| value: Any, | ||
| branch: str | None = None, | ||
| ) -> Any: | ||
| """If an attribute value has from_pool as a plain string name, resolve it to {id: uuid}.""" | ||
| if not isinstance(value, dict) or "from_pool" not in value: | ||
| return value | ||
| from_pool = value["from_pool"] | ||
| if not isinstance(from_pool, str): | ||
| return value | ||
| attr_schema = schema.get_attribute_or_none(name=name) | ||
| if attr_schema is None: | ||
| return value | ||
| pool_kind = _ATTRIBUTE_KIND_TO_POOL_KIND.get(attr_schema.kind) | ||
| if pool_kind is None: | ||
| return value | ||
| pool = await client.get(kind=pool_kind, hfid=[from_pool], branch=branch, raise_when_missing=True) | ||
| return {**value, "from_pool": {"id": pool.id}} |
There was a problem hiding this comment.
Handle UUID from_pool strings before doing an HFID lookup.
Right now every plain string goes through hfid=[from_pool] on Line 454. If callers pass a pool UUID string instead of a name, this path misses the direct-id case even though the mutation payload wants {"id": "<uuid>"}.
💡 Suggested fix
async def _resolve_attribute_pool(
cls,
client: InfrahubClient,
schema: MainSchemaTypesAPI,
name: str,
value: Any,
branch: str | None = None,
) -> Any:
"""If an attribute value has from_pool as a plain string name, resolve it to {id: uuid}."""
if not isinstance(value, dict) or "from_pool" not in value:
return value
from_pool = value["from_pool"]
if not isinstance(from_pool, str):
return value
+ if is_valid_uuid(from_pool):
+ return {**value, "from_pool": {"id": from_pool}}
attr_schema = schema.get_attribute_or_none(name=name)
if attr_schema is None:
return value
pool_kind = _ATTRIBUTE_KIND_TO_POOL_KIND.get(attr_schema.kind)
if pool_kind is None:
return value
pool = await client.get(kind=pool_kind, hfid=[from_pool], branch=branch, raise_when_missing=True)
return {**value, "from_pool": {"id": pool.id}}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infrahub_sdk/spec/object.py` around lines 433 - 455, The
_resolve_attribute_pool flow currently always treats a string from_pool as an
HFID lookup; update InfrahubObject._resolve_attribute_pool to first detect if
from_pool is a UUID string and, if so, return the value with from_pool set to
{"id": from_pool} immediately (before calling client.get). Use the existing
local variable from_pool and the attr_schema/_ATTRIBUTE_KIND_TO_POOL_KIND checks
as before; only call client.get(kind=pool_kind, hfid=[from_pool], ...) when
from_pool is not a UUID.
There was a problem hiding this comment.
I think the bot highlights a valid point here, since we have is_valid_uuid, we could check if the string is a valid UUID prior to making a HFID-based query. That would spare us a possible useless query.
Also can you add a test to validate how the fix should behave and prevent possible regressions?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find test files related to spec/object or InfrahubObjectFileData
fd -e py . tests/ | xargs grep -l "object\|InfrahubObjectFileData\|create_node\|resolve_attribute" 2>/dev/null | head -20
# Also look at the test directory structure
tree tests/ -L 3 2>/dev/null || find tests/ -type f -name "*.py" | head -30
Length of output: 5195
🏁 Script executed:
#!/bin/bash
cat tests/unit/sdk/spec/test_object.py
echo "---SEPARATOR---"
cat tests/unit/sdk/pool/test_attribute_from_pool.py
echo "---SEPARATOR---"
cat tests/unit/sdk/pool/conftest.py
Length of output: 26453
🏁 Script executed:
#!/bin/bash
# Check if there's a conftest in the spec directory
cat tests/unit/sdk/spec/conftest.py 2>/dev/null || echo "NO SPEC CONFTEST"
# Also check the unit/sdk conftest.py for client_with_schema_01 and schema setup
cat tests/unit/sdk/conftest.py
echo "---SEPARATOR---"
# Look at the pool conftest for vlan_schema
cat tests/unit/sdk/pool/conftest.py | grep -A 60 "vlan_schema"
Length of output: 88643
@gmazoyer, agreed on both points — the UUID fast-path is a valid improvement that avoids a redundant network round-trip. Here's the plan:
- Apply UUID fast-path in
_resolve_attribute_poolbefore the HFID query - Add unit tests for
_resolve_attribute_pooldirectly, covering all four cases
💡 Use @coderabbitai plan to create a plan for code changes
2 participants
Problem
When loading objects via YAML spec files, attributes using
from_poolwith a plain string pool name are passed through unchanged:This causes failures at the API level because Infrahub expects the pool reference in
{id: uuid}form:Fix
Add
_resolve_attribute_pooltoInfrahubObjectFileDatathat:from_poolas a plain stringclient.get(kind=pool_kind, hfid=[name]){"id": pool.id}before the node is createdA
_ATTRIBUTE_KIND_TO_POOL_KINDmapping controls which attribute kinds are eligible — currentlyNumber→CoreNumberPool.Values that are already
{"from_pool": {"id": ...}}(dict form) pass through unchanged, so existing usage is unaffected.Changes
infrahub_sdk/spec/object.py: add_ATTRIBUTE_KIND_TO_POOL_KINDmapping and_resolve_attribute_poolclassmethod; call it increate_nodewhen processing attributes🤖 Generated with Claude Code
Summary by CodeRabbit