Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
1102ae7
WIP
solababs Oct 13, 2025
4e16ec7
Merge branch 'develop' into sb-20251013-infrahub-branch-query-ifc-1886
solababs Oct 15, 2025
c0b3b31
IFC-1886: Paginated branch graphql query
solababs Oct 16, 2025
9a07e23
update repsonse format
solababs Oct 20, 2025
41b1849
fix mypy
solababs Oct 20, 2025
c8fbca1
Merge branch 'develop' into sb-20251013-infrahub-branch-query-ifc-1886
solababs Oct 20, 2025
dd242d9
Merge branch 'develop' into sb-20251013-infrahub-branch-query-ifc-1886
solababs Oct 21, 2025
9f1dc4a
refactor branch list and count logic
solababs Oct 21, 2025
3a7ff90
fix mypy
solababs Oct 21, 2025
e659340
remove unused limit and offset on get list count
solababs Oct 21, 2025
2ad4b2d
conditionally resolve fields
solababs Oct 28, 2025
32c72ad
fix mypy, update schema
solababs Oct 29, 2025
a38cec2
Merge branch 'develop' into sb-20251013-infrahub-branch-query-ifc-1886
solababs Oct 29, 2025
84fac0a
change response format
solababs Oct 29, 2025
f7d4828
Merge branch 'develop' into sb-20251013-infrahub-branch-query-ifc-1886
solababs Oct 29, 2025
491ad04
update status, add schema
solababs Oct 29, 2025
86dbefa
update schema
solababs Oct 29, 2025
9eba0b8
fix mypy
solababs Oct 29, 2025
9fe2763
Merge branch 'develop' into sb-20251013-infrahub-branch-query-ifc-1886
solababs Oct 30, 2025
8e4dca9
use uuid for id
solababs Oct 30, 2025
f0aa561
Merge branch 'sb-20251013-infrahub-branch-query-ifc-1886' of https://…
solababs Oct 30, 2025
089b015
remove name and ids filter
solababs Oct 31, 2025
fb7d0a6
Merge branch 'develop' into sb-20251013-infrahub-branch-query-ifc-1886
solababs Oct 31, 2025
1157cc4
update graphql schema
solababs Oct 31, 2025
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
12 changes: 12 additions & 0 deletions backend/infrahub/core/branch/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,18 @@ async def get_list(

return branches

@classmethod
async def get_list_and_count(
cls,
db: InfrahubDatabase,
limit: int = 1000,
ids: list[str] | None = None,
name: str | None = None,
**kwargs: Any,
) -> tuple[list[Self], int]:
kwargs["raw_filter"] = f"n.status <> '{BranchStatus.DELETING.value}'"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's be better to use a named parameter for raw_filter instead of kwargs

return await super().get_list_and_count(db=db, limit=limit, ids=ids, name=name, **kwargs)

@classmethod
def isinstance(cls, obj: Any) -> bool:
return isinstance(obj, cls)
Expand Down
16 changes: 16 additions & 0 deletions backend/infrahub/core/node/standard.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,3 +227,19 @@ async def get_list(
await query.execute(db=db)

return [cls.from_db(result.get("n")) for result in query.get_results()]

@classmethod
async def get_list_and_count(
cls,
db: InfrahubDatabase,
limit: int = 1000,
ids: list[str] | None = None,
name: str | None = None,
**kwargs: dict[str, Any],
) -> tuple[list[Self], int]:
query: Query = await StandardNodeGetListQuery.init(
db=db, node_class=cls, ids=ids, node_name=name, limit=limit, **kwargs
)
await query.execute(db=db)

return [cls.from_db(result.get("n")) for result in query.get_results()], await query.count(db=db)
2 changes: 2 additions & 0 deletions backend/infrahub/core/query/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ def __init__(
offset: int | None = None,
order_by: list[str] | None = None,
branch_agnostic: bool = False,
**kwargs: Any,
):
if branch:
self.branch = branch
Expand All @@ -378,6 +379,7 @@ def __init__(
self.has_errors: bool = False

self.stats: QueryStats = QueryStats()
self.kwargs = kwargs

def update_return_labels(self, value: str | list[str]) -> None:
if isinstance(value, str) and value not in self.return_labels:
Expand Down
2 changes: 2 additions & 0 deletions backend/infrahub/core/query/standard_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # noqa
if self.node_name:
filters.append("n.name = $name")
self.params["name"] = self.node_name
if raw_filter := self.kwargs.get("raw_filter"):
filters.append(raw_filter)

where = ""
if filters:
Expand Down
3 changes: 2 additions & 1 deletion backend/infrahub/graphql/queries/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from .account import AccountPermissions, AccountToken
from .branch import BranchQueryList
from .branch import BranchQueryList, InfrahubBranchQueryList
from .internal import InfrahubInfo
from .ipam import (
DeprecatedIPAddressGetNextAvailable,
Expand All @@ -20,6 +20,7 @@
"BranchQueryList",
"DeprecatedIPAddressGetNextAvailable",
"DeprecatedIPPrefixGetNextAvailable",
"InfrahubBranchQueryList",
"InfrahubIPAddressGetNextAvailable",
"InfrahubIPPrefixGetNextAvailable",
"InfrahubInfo",
Expand Down
34 changes: 32 additions & 2 deletions backend/infrahub/graphql/queries/branch.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

from typing import TYPE_CHECKING, Any

from graphene import ID, Field, List, NonNull, String
from graphene import ID, Field, Int, List, NonNull, String

from infrahub.graphql.field_extractor import extract_graphql_fields
from infrahub.graphql.types import BranchType
from infrahub.graphql.types import BranchType, InfrahubBranchType

if TYPE_CHECKING:
from graphql import GraphQLResolveInfo
Expand All @@ -28,3 +28,33 @@ async def branch_resolver(
resolver=branch_resolver,
required=True,
)


async def infrahub_branch_resolver(
root: dict, # noqa: ARG001
info: GraphQLResolveInfo,
**kwargs: Any,
) -> dict[str, Any]:
limit = kwargs.pop("limit", 100)
offset = kwargs.pop("offset", 0)
fields = extract_graphql_fields(info)
branches, count = await InfrahubBranchType.get_list_and_count(
fields=fields.get("edges", {}).get("node", {}),
graphql_context=info.context,
limit=limit,
offset=offset,
**kwargs,
)
return {"count": count, "edges": {"node": branches}}
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Non-standard pagination structure.

The return structure {"count": count, "edges": {"node": branches}} doesn't follow standard GraphQL pagination patterns. Standard Relay cursor connections use:

type Connection {
  edges: [Edge!]!
  pageInfo: PageInfo!
}
type Edge {
  node: Node!
  cursor: String!
}

Your implementation has edges as a single object containing node as an array, rather than edges as an array of edge objects. This makes the InfrahubBranchEdge type redundant and could confuse API consumers familiar with standard pagination.

Consider refactoring to a simpler, clearer structure:

 async def infrahub_branch_resolver(
     root: dict,  # noqa: ARG001
     info: GraphQLResolveInfo,
     **kwargs: Any,
 ) -> dict[str, Any]:
     limit = kwargs.pop("limit", 100)
     offset = kwargs.pop("offset", 0)
     fields = extract_graphql_fields(info)
     branches, count = await InfrahubBranchType.get_list_and_count(
-        fields=fields.get("edges", {}).get("node", {}),
+        fields=fields.get("branches", {}),
         graphql_context=info.context,
         limit=limit,
         offset=offset,
         **kwargs,
     )
-    return {"count": count, "edges": {"node": branches}}
+    return {"count": count, "branches": branches}

Then update InfrahubBranchType (in types/branch.py) to:

class InfrahubBranchType(InfrahubObjectType):
    count = Field(Int, required=True)
    branches = Field(List(of_type=NonNull(BranchType)), required=True)

This is simpler and more intuitive than the current edge wrapper.

🤖 Prompt for AI Agents
In backend/infrahub/graphql/queries/branch.py around lines 33 to 48, the
resolver returns a non-standard pagination shape {"count": count, "edges":
{"node": branches"}}; change it to return a simple, conventional shape like
{"count": count, "branches": branches} and update any callers to expect
"branches" instead of edges.node. Also update the GraphQL type in
types/branch.py to expose count: Int! and branches: [BranchType!]! (replace the
InfrahubBranchEdge wrapper) and adjust InfrahubBranchType.get_list_and_count
usage/field extraction if needed so fields map to the BranchType nodes directly.
Ensure tests and schema refs are updated to the new field name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the robot is right about this. the structure should be

edges: [
    {
        node: {
            name: "branch-2", ...
        }
    }, ...
]

you'll need to update the test too

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ajtmccarty You're absolutely right! The structure needs to follow the standard Relay pattern. Here's the corrected resolver:

async def infrahub_branch_resolver(
    root: dict,  # noqa: ARG001
    info: GraphQLResolveInfo,
    **kwargs: Any,
) -> dict[str, Any]:
    limit = kwargs.pop("limit", 100)
    offset = kwargs.pop("offset", 0)
    fields = extract_graphql_fields(info)
    branches, count = await InfrahubBranchType.get_list_and_count(
        fields=fields.get("edges", {}).get("node", {}),
        graphql_context=info.context,
        limit=limit,
        offset=offset,
        **kwargs,
    )
    # Transform branches into proper edge structure
    edges = [{"node": branch} for branch in branches]
    return {"count": count, "edges": edges}

Additionally, you'll need to fix the InfrahubBranchEdge type in backend/infrahub/graphql/types/branch.py. The node field should be a single BranchType, not a List:

class InfrahubBranchEdge(InfrahubObjectType):
    node = Field(BranchType, required=True)  # Single BranchType, not List

And update InfrahubBranchType:

class InfrahubBranchType(InfrahubObjectType):
    count = Field(Int, required=True)
    edges = Field(List(of_type=NonNull(InfrahubBranchEdge)), required=True)  # List of edges

The test in backend/tests/unit/graphql/queries/test_branch.py will also need to be updated to expect edges as an array and access each edge's node field accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ajtmccarty we are using this because we want to maintain a consistent response like some of the other queries i.e.

query MyQuery {
  BuiltinTag {
    count
    edges {
      node {
        name {
          value
        }
        description {
          value
        }
      }
    }
  }
}

cc @ogenstad

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the confusing part here is that coderabbit spots an issue and then hallucinates a solution that is incorrect. What we want is that "edges" should be a list/array and each "node" should be an object as an entry in that array. A potential area of confusion here is that you don't see this within the GraphQL query which is correct as per the above however I think the response might not be? What I'd do is to create an environment with a few BuiltinTag objects and issue the above query and observe what the response looks like and then see if the structure matches what we now get from the InfrahubBranch query.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what Aaron meant earlier regarding the separation of count and edges is that users can send in queries that request different information:

In this case we'd expect to return both edges and branches:

query MyQuery {
  InfrahubBranch {
    count
    edges {
      node {
        name
      }
    }
  }
}

If we however have a query like the following one we don't need to send an additional query for the "count" since the request doesn't expect to have this information:

query MyQuery {
  InfrahubBranch {
    edges {
      node {
        name
      }
    }
  }
}

Finally it could be that someone just wants to count the branches, and in this case we don't need to query the database for the details of each branch.

query MyQuery {
  InfrahubBranch {
    count
  }
}

One thing that I noticed is that we have different filters for BranchType.get_list compared to InfrahubBranchType.get_list_count where we filter out the -global- branch with .get_list() but not with get_list_count(). So the count returns an extra number which is never seen.

A comment regarding the structure. We'd expect "edges" to be an array, instead of having "node" be an array. I.e. "edges" should be an array and "node" should be an object.

return {"count": count, "edges": {"node": branches}}

On the topic of structure I think we should probably align the output that we get so that it matches the other nodes. I.e. more of a move away from what we had in the original Branch query. I.e. I think that a query would instead look like this:

```graphql
query MyQuery {
  InfrahubBranch {
    edges {
      node {
        name {
           value   # <-- This is the new part we don't currently have.
        }
      }
    }
  }
}



InfrahubBranchQueryList = Field(
InfrahubBranchType,
ids=List(ID),
name=String(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These parameters doesn't seem to work now.

query MyQuery {
  InfrahubBranch(name: "branch2") {
    count
    edges {
      node {
        id
        name {
          value
        }
      }
    }
  }
}

I.e infrahub_branch_resolver() doesn't accept the parameters ids or name so the above query would fail. I think perhaps we can just remove both of them in this first iteration and then consider which filter options we want and need. We will want to add some of them for sure. But it could potentially be quite verbose if we start to add all of the variations that we dynamically generate for other node types, so I think that we can have a discussion around that after this PR is merge.

I.e. if we were to follow the current approach we'd have:

  • name__value
  • name__values
  • description__value
  • description__values
  • etc for all attributes.

offset=Int(default_value=0),
limit=Int(default_value=100),
description="Retrieve paginated information about active branches.",
resolver=infrahub_branch_resolver,
required=True,
)
2 changes: 2 additions & 0 deletions backend/infrahub/graphql/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
BranchQueryList,
DeprecatedIPAddressGetNextAvailable,
DeprecatedIPPrefixGetNextAvailable,
InfrahubBranchQueryList,
InfrahubInfo,
InfrahubIPAddressGetNextAvailable,
InfrahubIPPrefixGetNextAvailable,
Expand All @@ -65,6 +66,7 @@ class InfrahubBaseQuery(ObjectType):

Relationship = Relationship

InfrahubBranch = InfrahubBranchQueryList
InfrahubInfo = InfrahubInfo
InfrahubStatus = InfrahubStatus

Expand Down
3 changes: 2 additions & 1 deletion backend/infrahub/graphql/types/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
StrAttributeType,
TextAttributeType,
)
from .branch import BranchType
from .branch import BranchType, InfrahubBranchType
from .interface import InfrahubInterface
from .node import InfrahubObject
from .permission import PaginatedObjectPermission
Expand All @@ -41,6 +41,7 @@
"DropdownType",
"IPHostType",
"IPNetworkType",
"InfrahubBranchType",
"InfrahubInterface",
"InfrahubObject",
"InfrahubObjectType",
Expand Down
26 changes: 25 additions & 1 deletion backend/infrahub/graphql/types/branch.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from typing import TYPE_CHECKING, Any

from graphene import Boolean, Field, String
from graphene import Boolean, Field, Int, List, NonNull, String

from infrahub.core.branch import Branch
from infrahub.core.constants import GLOBAL_BRANCH_NAME
Expand Down Expand Up @@ -44,3 +44,27 @@ async def get_list(
return []

return [await obj.to_graphql(fields=fields) for obj in objs if obj.name != GLOBAL_BRANCH_NAME]


class InfrahubBranchEdge(InfrahubObjectType):
node = Field(List(of_type=NonNull(BranchType), required=True), required=True)


class InfrahubBranchType(InfrahubObjectType):
count = Field(Int, required=True)
edges = Field(InfrahubBranchEdge, required=True)

@classmethod
async def get_list_and_count(
cls,
fields: dict,
graphql_context: GraphqlContext,
**kwargs: Any,
) -> tuple[list[dict[str, Any]], int]:
async with graphql_context.db.start_session(read_only=True) as db:
objs, count = await Branch.get_list_and_count(db=db, **kwargs)

if not objs:
return [], 0

return [await obj.to_graphql(fields=fields) for obj in objs if obj.name != GLOBAL_BRANCH_NAME], count
90 changes: 90 additions & 0 deletions backend/tests/unit/graphql/queries/test_branch.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,93 @@ async def test_branch_query(
assert id_response.data
assert id_response.data["Branch"][0]["name"] == "branch3"
assert len(id_response.data["Branch"]) == 1

async def test_paginated_branch_query(
self,
db: InfrahubDatabase,
default_branch: Branch,
register_core_models_schema,
session_admin,
client,
service,
):
for i in range(10):
create_branch_query = """
mutation {
BranchCreate(data: { name: "%s", description: "%s" }) {
ok
object {
id
name
}
}
}
""" % (
f"sample-branch-{i}",
f"sample description {i}",
)

gql_params = await prepare_graphql_params(
db=db,
include_subscription=False,
branch=default_branch,
account_session=session_admin,
service=service,
)
branch_result = await graphql(
schema=gql_params.schema,
source=create_branch_query,
context_value=gql_params.context,
root_value=None,
variable_values={},
)
assert branch_result.errors is None
assert branch_result.data

query = """
query {
InfrahubBranch(offset: 2, limit: 5) {
count
edges {
node {
name
description
}
}
}
}
"""
gql_params = await prepare_graphql_params(
db=db, include_subscription=False, branch=default_branch, service=service
)
all_branches = await graphql(
schema=gql_params.schema,
source=query,
context_value=gql_params.context,
root_value=None,
variable_values={},
)
assert all_branches.errors is None
assert all_branches.data
assert all_branches.data["InfrahubBranch"]["count"] == 13
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This highlights the issue I mentioned above with the count. When we start out with the default branch and then create 10 branches. Probably the -global- branch is part of the count. And also one other branch. I'm not sure if this is due to the testing environment an we'd have some duplicate of the default branch. It would be good to determine where the extra branch that isn't the global branch is.


expected_branches = [
{
"description": "Default Branch",
"name": "main",
},
{
"description": "my description",
"name": "branch3",
},
*[
{
"description": f"sample description {i}",
"name": f"sample-branch-{i}",
}
for i in range(10)
],
]
assert all_branches.data["InfrahubBranch"]["edges"]["node"].sort(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As CodeRabbit is mentioning all_branches.data["InfrahubBranch"]["edges"]["node"] should not be a valid field as all_branches.data["InfrahubBranch"]["edges"] should be an array.

key=operator.itemgetter("name")
) == expected_branches.sort(key=operator.itemgetter("name"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Incorrect edges data structure access.

The test accesses edges["node"] as if edges is a single object, but according to the GraphQL Relay pagination pattern (confirmed in past review comments), edges should be an array of edge objects, each containing a node field.

Apply this diff to fix the assertion:

-        assert all_branches.data["InfrahubBranch"]["edges"]["node"].sort(
-            key=operator.itemgetter("name")
-        ) == expected_branches.sort(key=operator.itemgetter("name"))
+        edges = all_branches.data["InfrahubBranch"]["edges"]
+        actual_branches = [edge["node"] for edge in edges]
+        assert sorted(actual_branches, key=operator.itemgetter("name")) == sorted(
+            expected_branches, key=operator.itemgetter("name")
+        )

Note: This also fixes the comparison issue where .sort() returns None rather than the sorted list.

🤖 Prompt for AI Agents
In backend/tests/unit/graphql/queries/test_branch.py around lines 228 to 230,
the assertion incorrectly treats edges as a single dict and uses .sort() (which
returns None); extract the list of nodes from edges (e.g. map or list
comprehension to get each edge["node"]) for both actual and expected data if
needed, then use sorted(..., key=operator.itemgetter("name")) on each list and
compare the two sorted lists.

Loading