-
Notifications
You must be signed in to change notification settings - Fork 6
Merge 'develop' into 'infrahub-develop' with resolved conflicts #728
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: infrahub-develop
Are you sure you want to change the base?
Merge 'develop' into 'infrahub-develop' with resolved conflicts #728
Conversation
Merge stable into develop
Upgrade ruff=0.14.10 and fix new violation
Merge stable into develop
WalkthroughThis pull request adds GraphQL typename-stripping functionality to the SDK's code generation pipeline. Three utility functions are introduced to recursively remove \__typename fields from GraphQL operations and fragments while preserving other query structure. The graphql.py CLI module integrates this stripping before code generation. Documentation is updated with expanded usage examples and warnings about unique query names. Supporting test fixtures and comprehensive unit tests validate the typename-stripping behavior across simple and nested query structures. A minor dependency version bump and code style improvement are also included. Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: |
51d4feb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3d6b13c0.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pog-develop-to-infrahub-deve-5qnk.infrahub-sdk-python.pages.dev |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## infrahub-develop #728 +/- ##
====================================================
+ Coverage 76.29% 77.08% +0.78%
====================================================
Files 114 114
Lines 9831 9854 +23
Branches 1509 1515 +6
====================================================
+ Hits 7501 7596 +95
+ Misses 1834 1754 -80
- Partials 496 504 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/unit/ctl/test_graphql_utils.py (1)
1-242: LGTM! Comprehensive test coverage for typename-stripping utilities.The test suite thoroughly validates the typename-stripping functionality across various GraphQL structures including nested queries, inline fragments, fragment definitions, and preservation of aliases, arguments, and directives. All tests follow the coding guidelines with proper type hints and no external dependencies.
💡 Optional: Consider adding a test for the TypeError case
The implementation in
infrahub_sdk/graphql/utils.py(line 54) raises aTypeErrorwhen encountering an unexpected selection node type. While this is defensive programming for a case that shouldn't occur with valid GraphQL, consider adding a test to verify this behavior:def test_strip_typename_raises_on_invalid_node_type(self) -> None: """Verify that invalid selection node types raise TypeError.""" from graphql import SelectionSetNode from unittest.mock import Mock # Create a mock selection with an invalid type invalid_selection = Mock() invalid_selection.__class__.__name__ = "InvalidNode" selection_set = SelectionSetNode(selections=(invalid_selection,)) with pytest.raises(TypeError, match="Unexpected GraphQL selection node type 'InvalidNode'"): strip_typename_from_selection_set(selection_set)This would provide explicit coverage of the error path, though it's optional since the graphql-core library should never produce invalid node types.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
docs/docs/python-sdk/guides/python-typing.mdxinfrahub_sdk/ctl/graphql.pyinfrahub_sdk/graphql/utils.pyinfrahub_sdk/schema/repository.pypyproject.tomltests/fixtures/unit/test_infrahubctl/graphql/invalid_query.gqltests/fixtures/unit/test_infrahubctl/graphql/query_with_typename.gqltests/fixtures/unit/test_infrahubctl/graphql/test_schema.graphqltests/fixtures/unit/test_infrahubctl/graphql/valid_query.gqltests/unit/ctl/test_graphql_app.pytests/unit/ctl/test_graphql_utils.py
🧰 Additional context used
📓 Path-based instructions (8)
**/*.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/ctl/graphql.pyinfrahub_sdk/graphql/utils.pyinfrahub_sdk/schema/repository.pytests/unit/ctl/test_graphql_app.pytests/unit/ctl/test_graphql_utils.py
infrahub_sdk/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Follow async/sync dual pattern for new features in the Python SDK
Files:
infrahub_sdk/ctl/graphql.pyinfrahub_sdk/graphql/utils.pyinfrahub_sdk/schema/repository.py
infrahub_sdk/ctl/**/*.py
📄 CodeRabbit inference engine (infrahub_sdk/ctl/AGENTS.md)
infrahub_sdk/ctl/**/*.py: Use@catch_exception(console=console)decorator on all async CLI commands
IncludeCONFIG_PARAMin all CLI command function parameters, even if unused
Useinitialize_client()orinitialize_client_sync()from ctl.client for client creation in CLI commands
Use Rich library for output formatting in CLI commands (tables, panels, console.print) instead of plain print() statements
Do not instantiate InfrahubClient directly; always use initialize_client() or initialize_client_sync() helper functions
Do not use plain print() statements in CLI commands; use Rich console.print() instead
Files:
infrahub_sdk/ctl/graphql.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:
docs/docs/python-sdk/guides/python-typing.mdx
docs/docs/**/*.mdx
📄 CodeRabbit inference engine (docs/AGENTS.md)
docs/docs/**/*.mdx: Create MDX files in appropriate directory within docs structure (guides, topics, or reference)
Add frontmatter withtitlefield to MDX documentation files
Use callouts (:::warning, :::note, etc.) for important notes and information in documentation
Files:
docs/docs/python-sdk/guides/python-typing.mdx
docs/docs/python-sdk/{guides,topics}/**/*.mdx
📄 CodeRabbit inference engine (docs/AGENTS.md)
docs/docs/python-sdk/{guides,topics}/**/*.mdx: Use Tabs component from '@theme/Tabs' for async/sync examples in documentation
Include both async/sync examples using Tabs in documentation
Files:
docs/docs/python-sdk/guides/python-typing.mdx
tests/**/*.py
📄 CodeRabbit inference engine (tests/AGENTS.md)
tests/**/*.py: Usehttpx_mockfixture for HTTP mocking in tests instead of making real HTTP requests
Do not add@pytest.mark.asynciodecorator to async test functions (async auto-mode is globally enabled)
Files:
tests/unit/ctl/test_graphql_app.pytests/unit/ctl/test_graphql_utils.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (tests/AGENTS.md)
Unit tests must be fast, mocked, and have no external dependencies
Files:
tests/unit/ctl/test_graphql_app.pytests/unit/ctl/test_graphql_utils.py
🧠 Learnings (2)
📚 Learning: 2025-12-10T17:13:21.977Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: infrahub_sdk/ctl/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:21.977Z
Learning: Applies to infrahub_sdk/ctl/**/*.py : Use `initialize_client()` or `initialize_client_sync()` from ctl.client for client creation in CLI commands
Applied to files:
infrahub_sdk/ctl/graphql.py
📚 Learning: 2025-12-10T17:13:14.678Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: docs/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:14.678Z
Learning: Applies to docs/python-sdk/reference/config.mdx : Do not edit `docs/python-sdk/reference/config.mdx` directly; regenerate with `uv run invoke generate-sdk` instead
Applied to files:
docs/docs/python-sdk/guides/python-typing.mdx
🧬 Code graph analysis (3)
infrahub_sdk/ctl/graphql.py (1)
infrahub_sdk/graphql/utils.py (4)
insert_fragments_inline(98-116)remove_fragment_import(119-125)strip_typename_from_fragment(74-86)strip_typename_from_operation(59-71)
tests/unit/ctl/test_graphql_app.py (2)
infrahub_sdk/ctl/graphql.py (2)
find_gql_files(43-59)get_graphql_query(62-79)tests/helpers/cli.py (1)
remove_ansi_color(4-6)
tests/unit/ctl/test_graphql_utils.py (1)
infrahub_sdk/graphql/utils.py (3)
strip_typename_from_fragment(74-86)strip_typename_from_operation(59-71)strip_typename_from_selection_set(14-56)
⏰ 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 (17)
infrahub_sdk/schema/repository.py (1)
154-154: LGTM!The refactor to use
read_text(encoding="UTF-8")is more concise and idiomatic for pathlib usage, with explicit encoding specification.tests/fixtures/unit/test_infrahubctl/graphql/invalid_query.gql (1)
1-9: LGTM!This test fixture appropriately provides an invalid query for testing error handling in the GraphQL CLI tooling.
tests/fixtures/unit/test_infrahubctl/graphql/query_with_typename.gql (1)
1-16: LGTM!This fixture appropriately tests the typename-stripping functionality with
__typenamefields at multiple levels of the query structure.docs/docs/python-sdk/guides/python-typing.mdx (1)
114-177: LGTM!The documentation improvements are excellent:
- Clearer usage examples showing both directory and file usage
- Well-structured numbered workflow steps
- Important warning about unique query names to prevent file overrides
- Consistent example using GetAllTags that aligns with test fixtures
tests/fixtures/unit/test_infrahubctl/graphql/valid_query.gql (1)
1-15: LGTM!This test fixture provides a valid GraphQL query for testing successful query parsing and code generation.
infrahub_sdk/ctl/graphql.py (5)
25-30: LGTM!The imports for typename-stripping utilities are correctly added to support the new functionality.
160-164: LGTM!The typename stripping logic is correctly implemented with excellent inline documentation explaining why this preprocessing is necessary (to prevent ariadne-codegen failures with the
__typenameintrospection field).
168-168: LGTM!Correctly passes
stripped_fragmentsto the package generator, consistent with the typename-stripping approach.
171-171: LGTM!The improved default naming logic for
target_package_nameusingdirectory.nameor falling back to"graphql_client"provides better naming conventions for generated packages.
180-180: LGTM!Correctly iterates over
stripped_queriesinstead of the original queries, ensuring typename fields are not present in the generated code.tests/unit/ctl/test_graphql_app.py (1)
1-266: LGTM!Excellent comprehensive test coverage for the GraphQL CLI tooling:
- Thorough testing of
find_gql_fileswith various scenarios (single file, directory, nested, empty)- Complete testing of
get_graphql_querywith valid/invalid cases- End-to-end CLI command tests covering success and error paths
- Proper verification of typename stripping behavior
- Well-structured with clear docstrings and proper use of fixtures
pyproject.toml (1)
83-83: ruff 0.14.10 is available and secure. Version exists on PyPI with no known security vulnerabilities or reported regressions. Released Dec 18, 2025.infrahub_sdk/graphql/utils.py (4)
3-11: LGTM! Necessary imports for typename-stripping functionality.The imports include all required GraphQL AST node types for manipulating selection sets.
59-71: LGTM! Clean wrapper for operation-level typename stripping.The function correctly preserves all operation attributes while delegating the selection set processing to
strip_typename_from_selection_set.
74-86: LGTM! Clean wrapper for fragment-level typename stripping.The function correctly preserves all fragment attributes while delegating the selection set processing to
strip_typename_from_selection_set.
14-56: graphql-core API compatibility confirmed across version range 3.1.x to 3.2.x.The implementation correctly handles all GraphQL selection node types and recursively strips
__typenamefields while preserving query structure (aliases, arguments, directives). The defensiveTypeErroron line 54 is good practice.The node constructor signatures (FieldNode, InlineFragmentNode, SelectionSetNode) are stable across graphql-core versions 3.1 and 3.2. The code uses tuple-based selection lists (
selections=tuple(...)) which is compatible with both versions, and creates new node instances rather than mutating existing ones—a safe approach that avoids issues from 3.2's immutable tuple transition.tests/fixtures/unit/test_infrahubctl/graphql/test_schema.graphql (1)
1-47: LGTM! Well-structured GraphQL schema fixture for testing.The schema provides appropriate test data with sufficient complexity (interfaces, nested objects, relay-style pagination) to validate the typename-stripping utilities. The structure aligns well with the test cases in
test_graphql_utils.py.
Fixed conflict in uv.lock, also added 51d4feb to fix a new violation in ruff.
Summary by CodeRabbit
Release Notes
Documentation
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.