IFC-2297 / Partial fix of issue #7036 : Identifier is not retrieved when queried from the Number Pools related requests#8417
Conversation
github-actions
Bot
added
the
group/backend
WalkthroughThe change propagates an 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
polmichel
force-pushed
the
pmi-20260218-identifier-missing-on-query
branch
from
423666a to
a1ef5aa
Compare
polmichel
force-pushed
the
pmi-20260218-identifier-missing-on-query
branch
from
a1ef5aa to
9ec4d3e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/infrahub/core/query/resource_manager.py (1)
219-237:⚠️ Potential issue | 🟠 MajorInclude
irin the branch filter and addir.status = "active"check to prevent duplicate allocations afterPoolChangeReservedThe
ir(IS_RESERVED) relationship is matched but excluded from both the branch filter and status validation. This creates a critical asymmetry:NumberPoolGetUsed(line 414) andNumberPoolGetFree(line 482) both include IS_RESERVED in their branch filters, butNumberPoolGetAllocatedexcludes it.When
PoolChangeReservedis executed, it sets.to = $aton the old IS_RESERVED relationship and creates a new one—both remain in the database. Becauseiris not subject to the branch filter's temporal checks (which filter out relationships where.tois set), both the expired and active IS_RESERVED relationships match, producing two duplicate result rows per allocation with different identifiers. This inflates count and returns stale identifier values.🐛 Proposed fix
AND av.value >= $start_range and av.value <= $end_range - AND all(r in [ha, hv, hs] WHERE (%(branch_filter)s)) + AND all(r in [ha, hv, hs, ir] WHERE (%(branch_filter)s)) AND ha.status = "active" AND hv.status = "active" AND hs.status = "active" + AND ir.status = "active"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/infrahub/core/query/resource_manager.py` around lines 219 - 237, The MATCH includes the IS_RESERVED relationship alias ir but the branch/temporal filter and status checks omit ir, allowing both old and new IS_RESERVED edges to match and produce duplicate rows; update the generated query (the block building query in resource_manager.py where query is defined and branch_filter is injected) to include ir in the branch_filter expression (so the same temporal predicate applied to ha, hv, hs also covers ir) and add an explicit AND ir.status = "active" alongside ha.status/hv.status/hs.status so only the current reservation is returned; adjust any place that composes branch_filter to accept ir or append the ir predicate when constructing this NumberPoolGetAllocated query.
🧹 Nitpick comments (2)
backend/infrahub/core/query/resource_manager.py (1)
63-74:identifier: strnon-optional field assumes IS_RESERVED always hasidentifierpropertyIf any IS_RESERVED relationship lacks the
identifierproperty (e.g., records predating identifier support or created by a code path that doesn't set it),get_as_type("identifier", str)will fail or silently return a wrong value. Consider making the fieldOptional[str]with a corresponding guard infrom_db, or add a comment asserting the invariant that all IS_RESERVED for number pools carry theidentifierproperty.Also note: per coding guidelines,
NumberPoolAllocatedResult(and sibling dataclasses in this file) should use Pydantic models rather than@dataclass(frozen=True).As per coding guidelines: "Use Pydantic models for dataclasses in Python."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/infrahub/core/query/resource_manager.py` around lines 63 - 74, The NumberPoolAllocatedResult currently declares identifier: str and calls result.get_as_type("identifier", str) in NumberPoolAllocatedResult.from_db which will fail if older IS_RESERVED relationships lack that property; change the model to allow optional values (identifier: Optional[str]) and update from_db to defensively read the field via result.get_as_type("identifier", str) into a local, handle None (e.g., keep None or set a safe default) or raise a clear assertion if you choose to enforce the invariant, and add a short comment explaining the chosen invariant; additionally, per guidelines migrate the dataclass to a Pydantic model (rename class if needed) so validation is provided by the model rather than plain `@dataclass`(frozen=True).backend/tests/component/core/resource_manager/test_number_pool_query.py (1)
123-152: Good coverage of the happy path — consider adding aPoolChangeReservedscenarioThe test correctly verifies identifiers for freshly allocated numbers. However, it doesn't cover the case where
PoolChangeReservedhas been called beforeNumberPoolGetAllocated, which is the scenario where the duplicate-row risk flagged inresource_manager.pywould manifest. Adding that path would catch regressions if the IS_RESERVED temporal filtering is ever changed.Minor: Line 135 uses
next(...)with no guard — aStopIterationhere would produce an unhelpful error. Consider replacing with:-incident_pool = next(pool for pool in pools if pool.node.value == INCIDENT.kind) +incident_pool = next((pool for pool in pools if pool.node.value == INCIDENT.kind), None) +assert incident_pool is not None, "Expected a NumberPool for Incident to be created"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/component/core/resource_manager/test_number_pool_query.py` around lines 123 - 152, Add a test path that exercises a prior PoolChangeReserved update before invoking NumberPoolGetAllocated to catch the duplicate-row temporal filtering issue in resource_manager.py: inside test_NumberPoolGetAllocated_returns_identifier call the PoolChangeReserved flow (using the same incident_pool/CoreNumberPool and one of the created incidents) to change a reservation, then run NumberPoolGetAllocated.execute() and verify results still have one row per allocated value and correct identifiers; also replace the unsafe next(...) used to find incident_pool with a guarded lookup (e.g., next(..., None)) and an explicit assertion or helpful failure message if the pool isn't found so StopIteration is avoided.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@backend/infrahub/core/query/resource_manager.py`:
- Around line 219-237: The MATCH includes the IS_RESERVED relationship alias ir
but the branch/temporal filter and status checks omit ir, allowing both old and
new IS_RESERVED edges to match and produce duplicate rows; update the generated
query (the block building query in resource_manager.py where query is defined
and branch_filter is injected) to include ir in the branch_filter expression (so
the same temporal predicate applied to ha, hv, hs also covers ir) and add an
explicit AND ir.status = "active" alongside ha.status/hv.status/hs.status so
only the current reservation is returned; adjust any place that composes
branch_filter to accept ir or append the ir predicate when constructing this
NumberPoolGetAllocated query.
---
Nitpick comments:
In `@backend/infrahub/core/query/resource_manager.py`:
- Around line 63-74: The NumberPoolAllocatedResult currently declares
identifier: str and calls result.get_as_type("identifier", str) in
NumberPoolAllocatedResult.from_db which will fail if older IS_RESERVED
relationships lack that property; change the model to allow optional values
(identifier: Optional[str]) and update from_db to defensively read the field via
result.get_as_type("identifier", str) into a local, handle None (e.g., keep None
or set a safe default) or raise a clear assertion if you choose to enforce the
invariant, and add a short comment explaining the chosen invariant;
additionally, per guidelines migrate the dataclass to a Pydantic model (rename
class if needed) so validation is provided by the model rather than plain
`@dataclass`(frozen=True).
In `@backend/tests/component/core/resource_manager/test_number_pool_query.py`:
- Around line 123-152: Add a test path that exercises a prior PoolChangeReserved
update before invoking NumberPoolGetAllocated to catch the duplicate-row
temporal filtering issue in resource_manager.py: inside
test_NumberPoolGetAllocated_returns_identifier call the PoolChangeReserved flow
(using the same incident_pool/CoreNumberPool and one of the created incidents)
to change a reservation, then run NumberPoolGetAllocated.execute() and verify
results still have one row per allocated value and correct identifiers; also
replace the unsafe next(...) used to find incident_pool with a guarded lookup
(e.g., next(..., None)) and an explicit assertion or helpful failure message if
the pool isn't found so StopIteration is avoided.
ajtmccarty
left a comment
ajtmccarty
left a comment
There was a problem hiding this comment.
I think the original issue is that we want to support the user passing in a custom identifier for an attribute provisioned from a NumberPool (from the bug {from_pool: {id: "185b9728-1b76-dda7-d13d-106529b1bcd9", identifier: "test-identifier"}}) and I'm not sure these changes accomplish this, but there will need to be a component/graphql or integration test for it to make sure
You are right, I was focused on the identifier being retrieved by the query. I will check the custom identifier management and do a related integration test. Thanks for the feedback |
|
As the custom identifier management seems to have a larger impact than initially expected, I will limit this PR to the resolution of identifier from GraphQL NumberPools queries, I created a Jira issue related to this new limited perimeter https://opsmill.atlassian.net/browse/IFC-2297. This is a partial fix of #7036. This fix will also unblock the fix regarding this issue on |
polmichel
changed the title
polmichel
force-pushed
the
pmi-20260218-identifier-missing-on-query
branch
2 times, most recently
from
b98a5dc to
60ffe10
Compare
polmichel
changed the title
polmichel
changed the title
polmichel
changed the title
polmichel
force-pushed
the
pmi-20260218-identifier-missing-on-query
branch
from
60ffe10 to
b9e628d
Compare
| @@ -0,0 +1 @@ | |||
| Fix identifier not being returned when querying number pool allocations via GraphQL API. | |||
There was a problem hiding this comment.
It is a partial fix of #7036. I am not sure if I should mention it into the changelog.
polmichel
force-pushed
the
pmi-20260218-identifier-missing-on-query
branch
from
b9e628d to
96e5a08
Compare
polmichel
force-pushed
the
pmi-20260218-identifier-missing-on-query
branch
3 times, most recently
from
8f92c20 to
60e8778
Compare
| result = results_by_value[idx] | ||
| assert result.identifier == incident.get_id() | ||
|
|
||
| async def test_identifier_after_delete_and_reallocation( |
There was a problem hiding this comment.
This is testing the case where there is a value allocation deletion, which leaves an orphan node.
This is a valid test case anyway, but it will have less value once IS_RESERVED relationships deletion handled.
There was a problem hiding this comment.
can you elaborate on what you mean by orphan node here? is there a vertex left on the database with no linked edges? if so, we'll need to fix it. it doesn't need to be in this PR, but those orphan nodes eventually come back to cause problems for us
There was a problem hiding this comment.
This may not be the correct term. I wanted to point out that relationships 'IS_RESERVED' relationships are never deleted. I will change the test in order to make it easier to understand
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/tests/component/graphql/queries/test_resource_pool.py (1)
795-909:⚠️ Potential issue | 🟡 MinorAdd a Google‑style docstring to
test_number_pool_utilization.This test lacks the required docstring structure (summary + Args/Returns/Raises/Examples).
✍️ Suggested docstring skeleton
async def test_number_pool_utilization( db: InfrahubDatabase, default_branch: Branch, register_core_models_schema: SchemaBranch ) -> None: + """ + Validate number pool utilization and allocation identifiers. + + Args: + db: Database handle. + default_branch: Default branch fixture. + register_core_models_schema: Schema registration fixture. + + Returns: + None. + + Raises: + AssertionError: If expectations are not met. + + Examples: + >>> await test_number_pool_utilization(...) + """ await load_schema(db=db, schema=SchemaRoot(nodes=[TICKET]))As per coding guidelines:
**/*.py: Always use triple quotes (""") for Python docstrings; Follow Google-style docstring format in Python; Include Args/Parameters, Returns, Raises, Examples sections in Python docstrings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/component/graphql/queries/test_resource_pool.py` around lines 795 - 909, The test function test_number_pool_utilization is missing a Google-style triple-quote docstring; add a top-of-function docstring using triple double-quotes (""") that starts with a one-line summary, then Google-style sections (Args: listing db: InfrahubDatabase, default_branch: Branch, register_core_models_schema: SchemaBranch), Returns: None, and optionally Raises/Examples as needed, ensuring formatting follows the project's docstring conventions and is placed immediately under the def line in test_number_pool_utilization.backend/infrahub/graphql/queries/resource_manager.py (1)
274-301:⚠️ Potential issue | 🟡 MinorAdd a Google‑style docstring for
resolve_number_pool_allocation.This function is now being modified but still lacks the required docstring format (summary + Args/Returns/Raises/Examples). Please add it for compliance.
✍️ Suggested docstring skeleton
async def resolve_number_pool_allocation( db: InfrahubDatabase, graphql_context: GraphqlContext, pool: Node, fields: dict, offset: int, limit: int ) -> dict: + """ + Resolve number pool allocation edges for GraphQL. + + Args: + db: Database handle. + graphql_context: GraphQL execution context. + pool: Number pool node. + fields: Requested GraphQL fields. + offset: Pagination offset. + limit: Pagination limit. + + Returns: + Response payload containing count and edges. + + Raises: + Exception: Propagated from database or query operations. + + Examples: + >>> await resolve_number_pool_allocation(...) + """ response: dict[str, Any] = {}As per coding guidelines:
**/*.py: Always use triple quotes (""") for Python docstrings; Follow Google-style docstring format in Python; Include Args/Parameters, Returns, Raises, Examples sections in Python docstrings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/infrahub/graphql/queries/resource_manager.py` around lines 274 - 301, Add a Google-style triple-quoted docstring to the resolve_number_pool_allocation function that begins with a one-line summary, then An "Args" section describing db: InfrahubDatabase, graphql_context: GraphqlContext, pool: Node, fields: dict, offset: int, limit: int and their roles, a "Returns" section describing the dict with possible "count" and "edges" keys and their shapes, a "Raises" section for any errors the function may propagate (e.g., database/query errors), and an "Examples" section showing a minimal call and expected return shape; place this directly above the resolve_number_pool_allocation def and use triple quotes (""") following Google-style formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/infrahub/core/query/resource_manager.py`:
- Around line 55-57: The reservation identifier field should allow nulls: change
the type annotation from identifier: str to identifier: Optional[str] and import
typing.Optional; when reading the Cypher result (where you currently call
get_as_type(..., str) for the 'ir' / reservation identifier), replace that call
with get_as_optional_type(..., str) so None is preserved instead of being cast
to "None"; apply the same Optional type + get_as_optional_type change to the
other similar fields mentioned around lines 231-245.
In
`@backend/tests/component/core/resource_manager/number_pool_queries/conftest.py`:
- Around line 39-85: Add Google-style triple-quoted docstrings to the fixtures
and helper functions: register_test_schema, run_number_pool_validation,
create_objects, get_used_numbers_in_pool, and get_reservations. For each
function add a short summary line, an Args section documenting parameter names
and types (e.g., db: InfrahubDatabase, schema: NodeSchema, branch: str, start:
int, end: int, pool: CoreNumberPool), a Returns section describing the return
type and content, a Raises section for any exceptions that may be propagated (if
none, state None), and an Examples section with a minimal usage snippet; ensure
you use triple double-quotes (""") and follow Google-style formatting
consistently across all added docstrings.
In
`@backend/tests/component/core/resource_manager/number_pool_queries/test_number_pool_get_allocated.py`:
- Around line 19-156: The test methods in class TestNumberPoolGetAllocated
(e.g., test_no_allocations, test_returns_identifier,
test_identifier_after_delete_and_reallocation,
test_multiple_allocations_across_pools_and_branches) use short triple-quoted
descriptions that don't follow Google-style docstrings; update each test
method's docstring to a full Google-style docstring (triple quotes """)
including short summary plus Args: (list fixtures like db, register_test_schema,
default_branch, run_number_pool_validation), Returns: (None), and Raises: (if
any) sections and an Example: section if helpful, keeping content concise and
relevant to the test behavior.
---
Outside diff comments:
In `@backend/infrahub/graphql/queries/resource_manager.py`:
- Around line 274-301: Add a Google-style triple-quoted docstring to the
resolve_number_pool_allocation function that begins with a one-line summary,
then An "Args" section describing db: InfrahubDatabase, graphql_context:
GraphqlContext, pool: Node, fields: dict, offset: int, limit: int and their
roles, a "Returns" section describing the dict with possible "count" and "edges"
keys and their shapes, a "Raises" section for any errors the function may
propagate (e.g., database/query errors), and an "Examples" section showing a
minimal call and expected return shape; place this directly above the
resolve_number_pool_allocation def and use triple quotes (""") following
Google-style formatting.
In `@backend/tests/component/graphql/queries/test_resource_pool.py`:
- Around line 795-909: The test function test_number_pool_utilization is missing
a Google-style triple-quote docstring; add a top-of-function docstring using
triple double-quotes (""") that starts with a one-line summary, then
Google-style sections (Args: listing db: InfrahubDatabase, default_branch:
Branch, register_core_models_schema: SchemaBranch), Returns: None, and
optionally Raises/Examples as needed, ensuring formatting follows the project's
docstring conventions and is placed immediately under the def line in
test_number_pool_utilization.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/infrahub/core/query/resource_manager.pybackend/infrahub/graphql/queries/resource_manager.pybackend/tests/component/core/resource_manager/number_pool_queries/__init__.pybackend/tests/component/core/resource_manager/number_pool_queries/conftest.pybackend/tests/component/core/resource_manager/number_pool_queries/test_number_pool_get_allocated.pybackend/tests/component/core/resource_manager/number_pool_queries/test_number_pool_query.pybackend/tests/component/graphql/queries/test_resource_pool.pychangelog/+identifier-number-pool-queries.fixed.md
✅ Files skipped from review due to trivial changes (1)
- changelog/+identifier-number-pool-queries.fixed.md
polmichel
force-pushed
the
pmi-20260218-identifier-missing-on-query
branch
from
513905a to
0a854d7
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/infrahub/core/query/resource_manager.py">
<violation number="1" location="backend/infrahub/core/query/resource_manager.py:203">
P1: Custom agent: **Check Cypher queries that update attributes/edges for optional matching and active-edge checks**
The nested MATCH clause for `IS_RESERVED` lacks an `OPTIONAL MATCH` and does not check for active edges (`ir.to IS NULL` or `ir.status = "active"`). This will match deleted or inactive relationships, causing incorrect results.
(Based on your team's feedback about checking edge status and activity in graph queries.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…or the reservation identifier
… database query to the query itself, rather than to the response dataclass
… NumberPoolGetReserved
polmichel
force-pushed
the
pmi-20260218-identifier-missing-on-query
branch
from
0a854d7 to
bf2b805
Compare
|
Last feedbacks are gathered in this squashed commit b556b7f |
| WHERE (%(hs_branch_filter)s) AND hs_int.status = "active" | ||
| RETURN true AS hs_active | ||
| LIMIT 1 | ||
| } |
There was a problem hiding this comment.
determining if an active HAS_SOURCE edge exists that links a to pool on any branch is actually kind of difficult. I think the real fix is probably to make this HAS_SOURCE relationship branch-agnostic, but that feels like a much larger change that this is supposed to be. I think the best fix for this case is to just check if any HAS_SOURCE edge links the two vertices and is (status="active" and to IS NULL).
you could add another EXISTS {} to the WHERE clause that checks for a status="deleted" edge on the same branch as the active one with a later from time and I think that would exclude all deleted HAS_SOURCE edges correctly.
there's also one more case that I don't think we can reasonably cover without making these HAS_SOURCE edges branch-agnostic. given the following scenario: I create a HAS_SOURCE edge on the default branch, I create a new branch br1, I delete the HAS_SOURCE edge on the default branch. in this case, the HAS_SOURCE edge is still active on br1 and we don't have a good way to check for this case within this query without it getting way too big for its simple purpose
in conclusion, something like this
CALL (a, pool) {
MATCH (a)-[hs_int:HAS_SOURCE]->(pool)
WHERE hs_int.status = "active"
AND hs_int.to IS NULL
AND NOT EXISTS {
MATCH (a)-[hs_deleted:HAS_SOURCE {branch: hs_int.branch, status: "deleted"}]->(pool)
WHERE hs_deleted.from > hs_int.from
}
RETURN true AS hs_active
LIMIT 1
}
There was a problem hiding this comment.
Thanks for the feedback.
About the hs_int to IS NULL, I do agree. This condition was a part of the agnostic branch filter. I have added an integration test regarding this: tests.component.core.resource_manager.test_number_pool_query.test_NumberPoolGetAllocated_requires_to_is_null_after_cross_branch_and_same_branch_clear.
About the NOT EXISTS I am not sure how such situation would happen. It would mean that we can actually create two relations which are
-
active
-
on the same branch
-
with a
tovalue to NULL -
BUT which would have different
fromvalues?
There was a problem hiding this comment.
And would the LIMIT 1 return True would not be equivalent to an EXISTS?
MATCH (n:%(node)s)-[ha:HAS_ATTRIBUTE]-(a:Attribute {name: $node_attribute})-[hv:HAS_VALUE]-(av:AttributeValueIndexed)
MATCH (a)-[hs:HAS_SOURCE]-(pool:%(number_pool_kind)s)-[ir:IS_RESERVED]->(av)
WHERE
pool.uuid = $pool_id
AND av.value >= $start_range and av.value <= $end_range
AND all(r in [ha, hv, hs] WHERE (%(branch_filter)s))
AND ha.status = "active"
AND hv.status = "active"
AND EXISTS {
MATCH (a)-[hs_int:HAS_SOURCE]->(pool)
WHERE hs_int.status = "active"
AND hs_int.to IS NULL
AND NOT EXISTS {
MATCH (a)-[hs_deleted:HAS_SOURCE {branch: hs_int.branch, status: "deleted"}]->(pool)
WHERE hs_deleted.from > hs_int.from
}
}
There was a problem hiding this comment.
the scenario I am considering is
- attr.source = pool
- attr.source = null
- attr.source = pool again
so there could be an old "deleted" edge that should be ignored b/c the HAS_SOURCE edge is active again
I think that your query would work, but putting an EXISTS subquery within an EXISTS subquery seems unnecessarily confusing
There was a problem hiding this comment.
Ok for the EXISTS part.
Regarding the scenario, If I understood correctly, I just tried to make it happen in an integration test, but the NOT EXISTS part does not change the test result -> see tests.component.core.resource_manager.test_number_pool_query.TestNumberPoolGetAllocated.test_NumberPoolGetAllocated_includes_allocation_when_source_cleared_then_reassigned_on_branch
BUT, I managed to trigger the NOT EXISTS condition in another test which forks a branch, clears the source on this branch and merges it back into main branch -> tests.component.core.resource_manager.test_number_pool_query.TestNumberPoolGetAllocated.test_NumberPoolGetAllocated_excludes_allocation_after_merging_cleared_source
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/infrahub/core/query/resource_manager.py">
<violation number="1" location="backend/infrahub/core/query/resource_manager.py:207">
P1: The new `hs_active` check is not branch/time-scoped, so `numberpool_get_allocated` can return incorrect results for historical or branch-specific queries.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
ajtmccarty
left a comment
ajtmccarty
left a comment
There was a problem hiding this comment.
looks good
this might query might have the most tests per lines of cypher in the codebase now
| run_number_pool_validation: None, | ||
| ) -> None: | ||
| """Scenario: on a child branch, source=pool -> source=null -> source=pool again; then the | ||
| main-branch active edge is closed so that it cannot satisfy the query on its own.""" |
There was a problem hiding this comment.
I think this is a display issue, this is written own
3 participants
Why
Problems statement:
NumberPool, the identifier is not returned.Query
Reponse
This query
infrahub.core.query.resource_manager.NumberPoolSetReserved.query_initsets an identifier into the IS_RESERVED relationship , which can be either a custom identifier or the uuid.Currently, when one runs the
InfrahubResourcePoolAllocatedGraphQL query, no identifier is returnedinfrahub.core.query.resource_manager.NumberPoolGetAllocated.query_init.Goal: Add the IS_RESERVED identifier to the return of the Cypher query used to retrieve information from the NumberPools allocations.
What is not included
Custom identifiers management on the number pool allocations.
Closes IFC-2297
Partially fix #7036
What changed
infrahub.core.query.resource_manager.NumberPoolGetAllocatedCypher query, which matchs an additional relationship in order to return the identifier attached to this relationship.How to review
infrahub.core.query.resource_manager.NumberPoolGetAllocatedalongside with performance considerationinfrahub.graphql.queries.resource_manager.PoolAllocated.resolveHow to test
Impact & rollout
Checklist
uv run towncrier create ...)Summary by CodeRabbit
Bug Fixes
Tests
Chore