Add ability to order results by metadata created_at or updated_at #698
Conversation
WalkthroughThis pull request introduces functionality to order nodes by metadata fields. It adds a new OrderDirection enum with ASC and DESC values, creates a NodeMetaOrder model with created_at and updated_at fields that enforces mutual exclusivity, and extends the Order model with a node_metadata field. The GraphQL renderer is updated to serialize Order objects and dictionaries. A changelog entry documents this new feature addition. Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: |
fb52237
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8734dd29.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pog-metadata-ordering-ihs-17.infrahub-sdk-python.pages.dev |
ogenstad
force-pushed
the
pog-metadata-ordering-IHS-172
branch
from
bc94b72 to
d473f3c
Compare
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## infrahub-develop #698 +/- ##
=================================================
Coverage 76.03% 76.04%
=================================================
Files 113 114 +1
Lines 9742 9779 +37
Branches 1490 1496 +6
=================================================
+ Hits 7407 7436 +29
- Misses 1840 1846 +6
- Partials 495 497 +2
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:
|
ogenstad
force-pushed
the
pog-metadata-ordering-IHS-172
branch
2 times, most recently
from
ccbdefe to
71de96f
Compare
ogenstad
force-pushed
the
pog-metadata-ordering-IHS-172
branch
from
71de96f to
fb52237
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
infrahub_sdk/graphql/renderers.py (1)
58-86: Consider reducing code duplication.The rendering logic for
Order,BaseModel, anddictis nearly identical (lines 58-67, 68-77, 78-86), differing only in how the data is obtained (model_dump(exclude_none=True),model_dump(), or direct dict access). While the functionality is correct, this duplication could be refactored to improve maintainability.🔎 Optional refactor to reduce duplication
+def _render_dict_as_graphql(data: dict[str, Any], convert_enum: bool = False) -> str: + """Helper to render a dictionary as GraphQL object syntax.""" + return ( + "{ " + + ", ".join( + f"{key}: {convert_to_graphql_as_string(value=val, convert_enum=convert_enum)}" + for key, val in data.items() + ) + + " }" + ) + def convert_to_graphql_as_string(value: Any, convert_enum: bool = False) -> str: ... if isinstance(value, Order): - data = value.model_dump(exclude_none=True) - return ( - "{ " - + ", ".join( - f"{key}: {convert_to_graphql_as_string(value=val, convert_enum=convert_enum)}" - for key, val in data.items() - ) - + " }" - ) + return _render_dict_as_graphql(value.model_dump(exclude_none=True), convert_enum) if isinstance(value, BaseModel): - data = value.model_dump() - return ( - "{ " - + ", ".join( - f"{key}: {convert_to_graphql_as_string(value=val, convert_enum=convert_enum)}" - for key, val in data.items() - ) - + " }" - ) + return _render_dict_as_graphql(value.model_dump(), convert_enum) if isinstance(value, dict): - return ( - "{ " - + ", ".join( - f"{key}: {convert_to_graphql_as_string(value=val, convert_enum=convert_enum)}" - for key, val in value.items() - ) - + " }" - ) + return _render_dict_as_graphql(value, convert_enum)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
changelog/+86c0992a.added.mdinfrahub_sdk/enums.pyinfrahub_sdk/graphql/renderers.pyinfrahub_sdk/types.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use type hints on all function signatures
Never mix async/sync inappropriately
Never bypass type checking without justification
Files:
infrahub_sdk/types.pyinfrahub_sdk/enums.pyinfrahub_sdk/graphql/renderers.py
infrahub_sdk/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Follow async/sync dual pattern for new features in the Python SDK
Files:
infrahub_sdk/types.pyinfrahub_sdk/enums.pyinfrahub_sdk/graphql/renderers.py
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{md,mdx}: Usetextlanguage for directory structure code blocks in markdown documentation
Add blank lines before and after lists in markdown documentation
Always specify language in fenced code blocks in markdown documentation
Files:
changelog/+86c0992a.added.md
🧠 Learnings (1)
📓 Common learnings
Learnt from: wvandeun
Repo: opsmill/infrahub-sdk-python PR: 637
File: infrahub_sdk/ctl/branch.py:290-290
Timestamp: 2025-11-25T13:29:23.062Z
Learning: In infrahub_sdk/ctl/branch.py, the report command uses pc.created_by.updated_at to display "Created at" for proposed changes as a workaround because the SDK doesn't have easy access to the creation time of the proposed change. This will be replaced with proper object-level metadata implementation in version 1.7 of Infrahub.
🧬 Code graph analysis (3)
infrahub_sdk/types.py (1)
infrahub_sdk/enums.py (1)
OrderDirection(4-6)
infrahub_sdk/enums.py (1)
infrahub_sdk/protocols_base.py (1)
Enum(110-111)
infrahub_sdk/graphql/renderers.py (1)
infrahub_sdk/types.py (1)
Order(84-88)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
changelog/+86c0992a.added.md (1)
1-1: LGTM!The changelog entry accurately describes the new feature for ordering nodes by metadata fields.
infrahub_sdk/enums.py (1)
4-6: LGTM! Well-designed enum.The
strinheritance is a good choice as it enables seamless serialization to GraphQL queries while maintaining type safety through the Enum.infrahub_sdk/graphql/renderers.py (1)
58-67: Theexclude_none=Truebehavior is appropriate for GraphQL.Handling
Orderseparately withexclude_none=Trueprevents sending null fields to GraphQL, which aligns with typical GraphQL practices where absent fields are preferred over explicit nulls for optional parameters.infrahub_sdk/types.py (3)
73-81: LGTM! Validation logic is correct.The
NodeMetaOrdermodel is well-designed:
- The mutual exclusivity validation correctly uses truthiness checks (
Noneis falsy,OrderDirectionmembers are truthy)- The constraint makes sense for ordering by a single metadata field at a time
- Using Pydantic's
model_validatorprovides clear error messages at instantiation time
9-9: Thenoqa: TC001comment is appropriate.
OrderDirectionis used in runtime type annotations for Pydantic fields, not just static type checking, so it cannot be moved into aTYPE_CHECKINGblock. The suppression is correct.
84-88: Backend API support confirmed; add integration tests for node_metadata ordering.The dependent backend PR (opsmill/infrahub#7904) has been merged, confirming the backend API now supports the
node_metadataordering parameter.Integration tests for the new
node_metadataordering functionality are still pending. The commented-out tests intests/integration/test_infrahub_client.pyshow the test pattern. Once the testcontainers issue is resolved, add integration tests covering the newnode_metadataparameter to ensure the ordering functionality works end-to-end.
| disable: bool | None = Field( | ||
| default=None, description="Disable default ordering, can be used to improve performance" | ||
| ) | ||
| node_metadata: NodeMetaOrder | None = Field(default=None, description="Order by node meta fields") |
There was a problem hiding this comment.
Nice one integrating into this pydantic model 👍
This PR depends on opsmill/infrahub#7904 being merged first.
I wanted to add some integration tests to this but had some issues with testcontainers for the pre-release (opsmill/infrahub#8004). Will add those later.
Summary by CodeRabbit
created_atorupdated_at).✏️ Tip: You can customize this high-level summary in your review settings.