Repository navigation
feat(node): number pool attach/detach, provenance and declared ranges - #1459
Open
ajtmccarty wants to merge 4 commits into
Open
ajtmccarty wants to merge 4 commits into
ajtmccarty wants to merge 4 commits into
Conversation
…333] Number attributes have attach_to_pool() and detach_from_pool(), sent on the next save. An attribute built with both value and from_pool now sends both, so the pool tracks the given number instead of allocating a new one. A pool write is sent once: a pending flag is cleared after a successful mutation, while _from_pool stays set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resource pool nodes have get_pool_allocations(), which returns PoolAllocation entries: the node id, kind, branch, display label, identifier and whether the pool allocated the number or a user provided it (None for IP pools). It needs Infrahub 1.12 or later. get_pool_allocated_resources() shares the paging loop but still leaves provenance out of its query, so it keeps working on older servers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…3369] A relationship whose peer kind is CoreNumberPoolRange takes the peer's fields as mutation input instead of a reference, as Infrahub's NumberPoolRangeInput does. A CoreNumberPool can therefore be created with ranges given as dicts of start, end and allocation_weight, and an upsert sets an existing pool's ranges to the declared list. Before, each range was sent as an empty object, which Infrahub read as one range without a start or an end. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n Infrahub 1.12 Covers creating a pool with declared ranges and setting them by upsert, attaching and detaching numbers (including a detach before the first save), the provenance get_pool_allocations reports, and the attribute's source after an attach. The class is skipped unless INFRAHUB_TESTING_IMAGE_VER is 1.12 or later. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
the
type/documentation
Deploying infrahub-sdk-python with
|
| Latest commit: |
2ed2245
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fe4bfd45.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://ajtm-10262026-number-pool-sd.infrahub-sdk-python.pages.dev |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## release-1.12 #1459 +/- ##
===============================================
Coverage ? 87.52%
===============================================
Files ? 154
Lines ? 15347
Branches ? 2073
===============================================
Hits ? 13433
Misses ? 1355
Partials ? 559
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
ajtmccarty
marked this pull request as ready for review
October 27, 2026 05:54
polmichel
self-requested a review
October 27, 2026 13:34
polmichel
approved these changes
Oct 27, 2026
| @@ -0,0 +1 @@ | |||
| An attribute created with both `value` and `from_pool` now sends both, so the number pool tracks the given number instead of allocating a new one. A `from_pool` request is sent once: after a successful save, the next update no longer sends it again. | |||
Contributor
There was a problem hiding this comment.
Does this fix two different issues? If so, should we split them into separate changelogs?
Regarding the first one, since the new SDK version will ship with Infrahub 1.12, shouldn’t we include that fix changelog in the 1.12 release notes since it was not even possible to do so before 1.12? I’m not entirely sure how we handle release notes in this case.
| def _pool_write_payload(self) -> _GraphQLPayloadAttribute: | ||
| # A pending write that names no pool is a detach. | ||
| if self._from_pool is None: | ||
| # A node that is not saved yet must still send the number it is created with. |
Contributor
There was a problem hiding this comment.
Should this comment be just before the next block? If so, it may mislead human and agents
| node._get_attribute("vlan_id").detach_from_pool() | ||
|
|
||
| input_data = node._generate_input_data(exclude_unmodified=True)["data"]["data"] | ||
| assert input_data == {"id": FETCHED_VLAN_ID, "vlan_id": {"from_pool": None}} |
Contributor
There was a problem hiding this comment.
Not sure if this is useful, but would an additional assertion verifying that the value is still attached to the node be a good idea?
| @@ -0,0 +1 @@ | |||
| Resource pool nodes have `get_pool_allocations()`, which lists the resources a pool holds as `PoolAllocation` entries: the node ID, kind, branch, display label (the number itself for a number pool), identifier and provenance. The provenance is `PoolAllocationProvenance.ALLOCATED` when the number pool allocated the number, `PoolAllocationProvenance.PROVIDED` when a user attached it, and `None` for IP pools. It requires Infrahub 1.12 or later. `get_pool_allocated_resources()` is unchanged. | |||
Contributor
There was a problem hiding this comment.
It may be subjective: maybe is this changelog too detailed?
|
|
||
| Fetch the resources the pool holds for a given resource, with how each one came to be held. | ||
|
|
||
| Requires Infrahub 1.12 or later. Unlike :meth:`get_pool_allocated_resources`, it does not fetch the |
Contributor
There was a problem hiding this comment.
More a question than a review comment, but I’m not sure we should mention the Infrahub release in each of these methods’ docstrings. We would likely forget to remove them later on, and if we do this for every method, it would be hard to read. Wdyt?
Contributor
There was a problem hiding this comment.
maybe it's a recommendation from /infrahub-writing-docs skill?
Comment on lines
+2217
to
+2218
| if response[POOL_ALLOCATED_QUERY_NAME].get("count", 0) <= page_offset + self._client.pagination_size: | ||
| return rows |
Contributor
There was a problem hiding this comment.
Optional comment: maybe not that important but it looks like sending a pagination equals to 0 creates a never ending loop. I think this is preexisting.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Infrahub 1.12 adds number-pool features that SDK users cannot use today:
from_pool: nullon an update (IFC-3333). The SDK has no way to send it. Attaching a number the user already holds (valueandfrom_pooltogether) has the same problem: on create, the SDK sends onlyfrom_pool, so the pool allocates a new number instead of attaching the given one.provenancefield ofInfrahubResourcePoolAllocated(IFC-3367).get_pool_allocated_resources()returns only the nodes and drops every row field.rangesasNumberPoolRangeInput(start,end,allocation_weight), but the SDK sends every relationship peer as a reference, soranges=[{"start": 1, "end": 10}]reaches Infrahub as[{}]: one range with no start and no end.Closes IFC-3333, IFC-3367, IFC-3369
What changed
Attach and detach a number (IFC-3333)
attach_to_pool(pool)anddetach_from_pool(). Both are sent on the next save.attach_to_pool()sends the number the attribute holds withfrom_pool, so the pool tracks it as provided. It raisesValueErrorwhen the attribute holds no number, or when it is given a node that is not a resource pool or has not been saved.detach_from_pool()sendsfrom_pool: null. The attribute keeps its number. On a node that is not saved yet, the number is sent with the detach so the node is created with it.valueandfrom_poolnow sends both, so the pool attaches the given number instead of allocating a new one.from_poolrequest is sent once. A private pending flag is cleared after a successful mutation, so the next update does not send it again._from_poolstays set, sois_from_pool_attribute()still returnsTrueafter a save.IntegerandIntegerOptionalprotocols declare the two methods for typed code.List a pool's allocations with their provenance (IFC-3367)
get_pool_allocations(resource), async and sync. It returnsPoolAllocationentries (node ID, kind, branch, display label, identifier, provenance) without fetching the nodes.provenanceisPoolAllocationProvenance.ALLOCATED,PoolAllocationProvenance.PROVIDED, orNonefor IP pools.get_pool_allocated_resources()is unchanged. It shares the paging loop but does not requestprovenance, so it keeps working against servers older than 1.12.Create a number pool with its ranges (IFC-3369)
CoreNumberPoolRangesends the peer's fields instead of a reference, matching how Infrahub picksNumberPoolRangeInputfor that peer kind. The rule lives in one place:INLINE_INPUT_PEER_KINDSandRelatedNodeBase.client.create(kind="CoreNumberPool", ..., ranges=[{"start": 100, "end": 199, "allocation_weight": 10}])creates the pool and its ranges in one mutation.save(allow_upsert=True)sets an existing pool's ranges to the declared list.CoreNumberPoolRangenodes, which already worked. Infrahub refuses edits made through the pool'srangesrelationship (Field 'id' is not defined by type 'NumberPoolRangeInput').What stayed the same
CoreNumberPoolRangeis serialized as before.How to review
Suggested order, one commit per ticket:
1d55171battach and detach:node/attribute.pyand the twonode.pyhunks (clearing the pending flag in_process_mutation_result, and keepingvaluenext tofrom_poolin_strip_unmodified_dict).ef09423fprovenance:PoolAllocation, the shared query builder and paging loop innode.py.3204d16cranges:node/related_node.pyandnode/constants.py. It also removes a secondself._relationship_metadata = NoneinRelatedNodeBase.__init__, which the constructor already sets just above.2ed22456integration test.docs/docs/python-sdk/sdk_ref/is generated.Impact & rollout
from_poolafter it succeeded. Before, the next update sent{"from_pool": ...}again, which Infrahub ignored.rangesdicts on aCoreNumberPoolare now sent as range definitions instead of empty objects.Checklist
🤖 Generated with Claude Code