-
Notifications
You must be signed in to change notification settings - Fork 6
Fix merge conflict between develop and infrahub-develop
#566
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
Fix merge conflict between develop and infrahub-develop
#566
Conversation
…o times. (#531) * feat: add create_diff method and update get_diff_summary to accept name parameter * feat: enhance get_diff_summary to support time range filtering * fix: update action field in diff summary to reflect correct status * feat: add create_diff method and update get_diff_summary for time range support * feat: add create_diff method and enhance get_diff_summary to support time range filtering * fix: remove unnecessary blank lines in InfrahubClientSync class * feat: update create_diff method to validate time range and change return type
…ables (#536) * Fix branch handling in `_run_transform` and `execute_graphql_query` functions * Add changelog entry for fixing branch handling in `_run_transform` and `execute_graphql_query` functions * Remove unused import of `get_branch` from `utils.py` * Add jinja2 transform and GraphQL query for tags, and implement branch selection test * Update changelog to specify environment variable usage for branch management in `_run_transform` and `execute_graphql_query` functions. * Remove unused `get_branch` import and set default branch in `validate_graphql` function
Remove unnecessary assignments before `return` statement
add livestream to docs
fix improperly escaped special characters in `HFID`
* Replace toml package with tomllib and tomli optionally * fix for mypy errors * update config, add pre-commit config * Make tomli package optional * Add towncrier housekeeping message * update python to uppercase p
Merge 'stable' into 'develop' with resolved conflicts
Co-authored-by: BeArchiTek <1334310+BeArchiTek@users.noreply.github.com>
* add support for clearing optional dropdown attributes and enhance tests for dropdown handling * add changelog entry for clearing optional dropdown attributes * refactor: update branch handling in changelog and allow clearing optional attributes if mutated
* add support for clearing optional dropdown attributes and enhance tests for dropdown handling * add changelog entry for clearing optional dropdown attributes * refactor: update branch handling in changelog and allow clearing optional attributes if mutated Co-authored-by: Alex Gittings <agitting96@gmail.com>
* Fix schema load failure exception * Add towncrier housekeeping message
Co-authored-by: BeArchiTek <1334310+BeArchiTek@users.noreply.github.com>
Merge stable into develop
* Add range expansion feature for object files and update documentation - Implemented range expansion for string fields in object files, allowing patterns like [1-5] to create multiple objects. - Added validation to ensure expanded lists have the same length. - Updated documentation to include usage examples and details on the new feature. - Added unit tests to verify range expansion functionality and error handling for mismatched lengths. * Refactor range expansion tests to clarify expected behavior for edge cases * Refactor range expansion logic to a standalone function for improved reusability and clarity * Rename variable for clarity in data expansion process within InfrahubObjectFileData class * Remove unnecessary range expansion in InfrahubObjectFileData class to streamline data handling
Fixes conflict in infrahub_sdk/client.py
WalkthroughThis PR adds object-type conversion (convert_object_type) and diff operations (create_diff, get_diff_summary with optional time range). It introduces range expansion for object files, integrating expansion into validation and processing. TOML handling switches to tomllib with tomli fallback; CI and pyproject are updated accordingly and pre-commit is added. GraphQL string serialization now uses JSON-style escaping. Branch resolution defaults to configuration in CLI utilities, and schema error display logic is adjusted. Error reporting for JSON decode includes server content. Optional mutated attributes can be cleared with None. Docs gain a VideoPlayer component. Multiple minor refactors and comprehensive tests/changelogs are included. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: |
cb4c330
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fbec3c05.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pog-infrahub-develop-merge-c.infrahub-sdk-python.pages.dev |
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## infrahub-develop #566 +/- ##
====================================================
- Coverage 75.97% 75.91% -0.07%
====================================================
Files 100 102 +2
Lines 9345 9134 -211
Branches 1835 1811 -24
====================================================
- Hits 7100 6934 -166
+ Misses 1754 1706 -48
- Partials 491 494 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 9 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
infrahub_sdk/ctl/config.py (1)
20-20: Remove or integrate unused constant
INFRAHUB_REPO_CONFIG_FILE_ALTis defined (config.py:20) but never used; either incorporate it into the config‐file discovery logic or remove it.infrahub_sdk/ctl/validate.py (1)
77-84: Fix the misleading branch display.The console print statement at Line 77 displays the branch value before the fallback to
default_infrahub_branchis applied at Lines 83-84. IfbranchisNone, users will see "Query 'X' will be validated on branch 'None'." even though the query actually executes on the default branch.Apply this diff to fix the display:
- console.print(f"[purple]Query '{query}' will be validated on branch '{branch}'.") - variables_dict = parse_cli_vars(variables) client = initialize_client_sync() if not branch: branch = client.config.default_infrahub_branch + + console.print(f"[purple]Query '{query}' will be validated on branch '{branch}'.")
🧹 Nitpick comments (13)
docs/src/components/VideoPlayer/index.tsx (1)
9-31: Consider adding accessibility and error handling enhancements.The component implementation is clean and functional. To improve user experience and accessibility, consider these optional enhancements:
- Accessibility: Add descriptive aria-label or title props to help screen reader users understand the video content.
- Error handling: Wrap ReactPlayer with error boundaries or add onError callback to handle loading failures gracefully.
- Flexible styling: Consider accepting additional ReactPlayer props (e.g.,
playing,muted,config) to allow callers more control without modifying the component.Example for accessibility:
interface VideoPlayerProps { url: string; light?: boolean; + ariaLabel?: string; } -export default function VideoPlayer({ url, light = false }: VideoPlayerProps) { +export default function VideoPlayer({ url, light = false, ariaLabel = 'Video player' }: VideoPlayerProps) { return ( - <div style={{ + <div aria-label={ariaLabel} style={{ border: '3px solid #eee',docs/docs/python-sdk/introduction.mdx (1)
10-12: Replace deprecated<center>tag with CSS styling.The
<center>tag is deprecated in HTML5. For better maintainability and modern HTML practices, use CSS styling instead.Apply this diff to use CSS styling:
-<center> - <VideoPlayer url='https://www.youtube.com/live/RbRz8_t0FBs?feature=shared' light /> -</center> +<div style={{ display: 'flex', justifyContent: 'center' }}> + <VideoPlayer url='https://www.youtube.com/live/RbRz8_t0FBs?feature=shared' light /> +</div>infrahub_sdk/repository.py (1)
32-32: LGTM! Good refactor eliminates unnecessary variable.Directly returning the result improves code conciseness without changing behavior.
infrahub_sdk/timestamp.py (1)
54-54: LGTM! Good refactor eliminates unnecessary variable.Directly returning the parsed result improves code conciseness without changing behavior.
infrahub_sdk/playback.py (1)
59-59: LGTM! Good refactor eliminates unnecessary variable.Directly returning the
httpx.Responseobject improves code conciseness without changing behavior.tests/integration/test_convert_object_type.py (2)
76-81: Consider adding a case for use_default_valueYou exercise source_field and data paths. Add one mapping using use_default_value=True to cover the third mutually exclusive option.
83-97: Use the selected client consistently to reduce cross-client mixingCreation/load steps always use async client. For clarity, consider using client_sync for create/save when client_type is sync so each param set runs fully in its client mode.
tests/unit/ctl/test_render_app.py (1)
90-96: Guard against missing dulwich dependency in testsmonkeypatching dulwich.porcelain.active_branch requires dulwich to be installed. Use importorskip to avoid failures when it’s absent.
Apply this diff:
def test_render_branch_selection(monkeypatch, httpx_mock: HTTPXMock, cli_branch, env_branch, from_git, expected_branch): """Test that the render command uses the correct branch source.""" if from_git: - monkeypatch.setattr("dulwich.porcelain.active_branch", lambda _: b"git-branch") + pytest.importorskip("dulwich") + monkeypatch.setattr("dulwich.porcelain.active_branch", lambda _: b"git-branch")infrahub_sdk/spec/range_expansion.py (2)
28-56: Preserve brackets on unsupported ranges to avoid silent normalizationWhen encountering mixed/unsupported tokens (e.g., "1-a"), the function currently appends the raw token without brackets, subtly changing the original pattern. Prefer preserving the original bracketed text to make outcomes explicit.
Apply this diff:
- else: - # Mixed or unsupported range type, append as-is - expanded_values.append(value) + else: + # Mixed or unsupported range type, preserve original bracketed token + expanded_values.append(f"[{value}]")
114-118: Micro-optimization: compile regex onceConsider precompiling MATCH_PATTERN at module load to avoid recompiling per call.
Apply this diff:
-MATCH_PATTERN = r"(\[[\w,-]+\])" +MATCH_PATTERN = r"(\[[\w,-]+\])" +_RANGE_RE = re.compile(MATCH_PATTERN) ... - re_compiled = re.compile(MATCH_PATTERN) + re_compiled = _RANGE_REtests/unit/sdk/spec/test_object.py (1)
114-148: Range expansion tests are solid; add a couple of edge casesConsider adding:
- Descending ranges (e.g., AMS[5-3]) to ensure reverse order works.
- Escaped brackets (e.g., name="AMS[x] [1-2]") to verify literal bracket handling.
infrahub_sdk/client.py (2)
1658-1689: convert_object_type (async): remove deprecated raise_for_error argumentFunctionality is correct. To avoid deprecation warnings, drop raise_for_error=True; execute_graphql already raises for HTTP errors by default.
Apply this diff:
response = await self.execute_graphql( query=CONVERT_OBJECT_MUTATION, variables={ "node_id": node_id, "fields_mapping": mapping_dict, "target_kind": target_kind, }, branch_name=branch_name, - raise_for_error=True, )
3005-3035: convert_object_type (sync): remove deprecated raise_for_error argumentSame note as async version; drop raise_for_error=True to avoid warnings.
Apply this diff:
response = self.execute_graphql( query=CONVERT_OBJECT_MUTATION, variables={ "node_id": node_id, "fields_mapping": mapping_dict, "target_kind": target_kind, }, branch_name=branch_name, - raise_for_error=True, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
docs/package-lock.jsonis excluded by!**/package-lock.jsonpoetry.lockis excluded by!**/*.lock
📒 Files selected for processing (48)
.github/workflows/ci.yml(5 hunks).pre-commit-config.yaml(1 hunks)changelog/+convert-object-type.added.md(1 hunks)changelog/+escape-hfid.fixed.md(1 hunks)changelog/464.housekeeping.md(1 hunks)changelog/473.fixed.md(1 hunks)changelog/528.housekeeping.md(1 hunks)changelog/529.added.md(1 hunks)changelog/535.fixed.md(1 hunks)changelog/549.fixed.md(1 hunks)changelog/560.added.md(1 hunks)docs/docs/python-sdk/introduction.mdx(1 hunks)docs/docs/python-sdk/topics/object_file.mdx(1 hunks)docs/docusaurus.config.ts(1 hunks)docs/package.json(1 hunks)docs/src/components/VideoPlayer/index.tsx(1 hunks)infrahub_sdk/branch.py(2 hunks)infrahub_sdk/client.py(25 hunks)infrahub_sdk/convert_object_type.py(1 hunks)infrahub_sdk/ctl/cli_commands.py(1 hunks)infrahub_sdk/ctl/config.py(2 hunks)infrahub_sdk/ctl/schema.py(1 hunks)infrahub_sdk/ctl/utils.py(1 hunks)infrahub_sdk/ctl/validate.py(2 hunks)infrahub_sdk/diff.py(2 hunks)infrahub_sdk/exceptions.py(1 hunks)infrahub_sdk/graphql.py(2 hunks)infrahub_sdk/node/attribute.py(1 hunks)infrahub_sdk/node/node.py(2 hunks)infrahub_sdk/playback.py(1 hunks)infrahub_sdk/repository.py(1 hunks)infrahub_sdk/spec/object.py(6 hunks)infrahub_sdk/spec/range_expansion.py(1 hunks)infrahub_sdk/timestamp.py(1 hunks)pyproject.toml(4 hunks)tests/constants.py(1 hunks)tests/fixtures/repos/ctl_integration/.infrahub.yml(1 hunks)tests/fixtures/repos/ctl_integration/templates/tags.j2(1 hunks)tests/fixtures/repos/ctl_integration/templates/tags_query.gql(1 hunks)tests/integration/test_convert_object_type.py(1 hunks)tests/unit/ctl/test_render_app.py(1 hunks)tests/unit/sdk/conftest.py(1 hunks)tests/unit/sdk/spec/test_object.py(2 hunks)tests/unit/sdk/test_diff_summary.py(3 hunks)tests/unit/sdk/test_node.py(1 hunks)tests/unit/sdk/test_range_expansion.py(1 hunks)tests/unit/sdk/test_schema.py(1 hunks)tests/unit/sdk/test_utils.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
When implementing Infrahub checks, subclass InfrahubCheck and override validate(data); do not implement or rely on a check() method
Files:
infrahub_sdk/ctl/cli_commands.pyinfrahub_sdk/playback.pyinfrahub_sdk/node/node.pyinfrahub_sdk/diff.pytests/unit/sdk/test_utils.pyinfrahub_sdk/node/attribute.pytests/unit/sdk/conftest.pytests/unit/sdk/test_diff_summary.pyinfrahub_sdk/exceptions.pyinfrahub_sdk/ctl/schema.pytests/unit/sdk/test_node.pyinfrahub_sdk/spec/object.pytests/constants.pytests/unit/ctl/test_render_app.pyinfrahub_sdk/ctl/utils.pyinfrahub_sdk/spec/range_expansion.pyinfrahub_sdk/convert_object_type.pyinfrahub_sdk/branch.pyinfrahub_sdk/timestamp.pyinfrahub_sdk/repository.pytests/integration/test_convert_object_type.pyinfrahub_sdk/ctl/config.pytests/unit/sdk/test_schema.pyinfrahub_sdk/graphql.pytests/unit/sdk/spec/test_object.pyinfrahub_sdk/client.pytests/unit/sdk/test_range_expansion.pyinfrahub_sdk/ctl/validate.py
infrahub_sdk/ctl/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
infrahub_sdk/ctl/**/*.py: Build CLI commands with Typer
Organize and keep all CLI commands within infrahub_sdk/ctl/
Files:
infrahub_sdk/ctl/cli_commands.pyinfrahub_sdk/ctl/schema.pyinfrahub_sdk/ctl/utils.pyinfrahub_sdk/ctl/config.pyinfrahub_sdk/ctl/validate.py
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
docs/**/*.{md,mdx}: Follow the Diataxis framework and classify docs as Tutorials, How-to guides, Explanation, or Reference
Structure How-to guides with required frontmatter and sections: introduction, prerequisites, step-by-step steps, validation, related resources
Structure Topics (Explanation) docs with introduction, concepts & definitions, background & context, architecture & design, connections, further reading
Use a professional, concise, informative tone with consistent structure across documents
Use proper language tags on all code blocks
Include both async and sync examples where applicable using the Tabs component
Add validation steps to guides to confirm success and expected outputs
Use callouts for warnings, tips, and important notes
Define new terms on first use and use domain-relevant terminology consistent with Infrahub’s model/UI
Conform to markdown style rules in .markdownlint.yaml and Vale styles in .vale/styles/
Files:
docs/docs/python-sdk/topics/object_file.mdxdocs/docs/python-sdk/introduction.mdx
tests/unit/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Place and write unit tests under tests/unit/ (isolated component tests)
Files:
tests/unit/sdk/test_utils.pytests/unit/sdk/conftest.pytests/unit/sdk/test_diff_summary.pytests/unit/sdk/test_node.pytests/unit/ctl/test_render_app.pytests/unit/sdk/test_schema.pytests/unit/sdk/spec/test_object.pytests/unit/sdk/test_range_expansion.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use the custom pytest plugin (infrahub_sdk.pytest_plugin) fixtures for clients, configuration, and Infrahub-specific testing
Files:
tests/unit/sdk/test_utils.pytests/unit/sdk/conftest.pytests/unit/sdk/test_diff_summary.pytests/unit/sdk/test_node.pytests/constants.pytests/unit/ctl/test_render_app.pytests/integration/test_convert_object_type.pytests/unit/sdk/test_schema.pytests/unit/sdk/spec/test_object.pytests/unit/sdk/test_range_expansion.py
tests/integration/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Place and write integration tests under tests/integration/ (tests against real Infrahub instances)
Files:
tests/integration/test_convert_object_type.py
infrahub_sdk/client.py
📄 CodeRabbit inference engine (CLAUDE.md)
infrahub_sdk/client.py: Use HTTPX for transport with proxy support (single proxy or HTTP/HTTPS mounts)
Support authentication via API tokens or JWT with automatic refresh
Files:
infrahub_sdk/client.py
🧬 Code graph analysis (14)
infrahub_sdk/ctl/cli_commands.py (1)
infrahub_sdk/utils.py (1)
write_to_file(310-323)
infrahub_sdk/node/node.py (1)
infrahub_sdk/client.py (14)
get(388-404)get(407-423)get(426-442)get(445-461)get(464-480)get(483-499)get(501-559)get(2198-2214)get(2217-2233)get(2236-2252)get(2255-2271)get(2274-2290)get(2293-2309)get(2311-2369)
tests/unit/sdk/test_utils.py (2)
infrahub_sdk/exceptions.py (1)
JsonDecodeError(13-22)infrahub_sdk/utils.py (1)
decode_json(94-98)
tests/unit/sdk/conftest.py (1)
infrahub_sdk/schema/main.py (4)
NodeSchemaAPI(311-313)NodeSchema(306-308)convert_api(283-284)convert_api(307-308)
tests/unit/sdk/test_node.py (3)
tests/unit/sdk/conftest.py (1)
location_schema_with_dropdown(181-219)infrahub_sdk/node/attribute.py (3)
value(66-67)value(70-72)_generate_input_data(74-105)infrahub_sdk/node/related_node.py (1)
_generate_input_data(128-145)
infrahub_sdk/spec/object.py (2)
infrahub_sdk/spec/range_expansion.py (1)
range_expansion(88-118)infrahub_sdk/exceptions.py (2)
ValidationError(117-129)ObjectValidationError(132-139)
tests/unit/ctl/test_render_app.py (3)
tests/helpers/fixtures.py (1)
read_fixture(10-16)tests/helpers/utils.py (1)
temp_repo_and_cd(31-42)infrahub_sdk/async_typer.py (1)
runner(17-18)
infrahub_sdk/ctl/utils.py (2)
tests/unit/sdk/conftest.py (1)
client(32-33)infrahub_sdk/config.py (1)
default_infrahub_branch(125-130)
tests/integration/test_convert_object_type.py (3)
infrahub_sdk/client.py (22)
convert_object_type(1658-1688)convert_object_type(3005-3035)create(346-352)create(355-361)create(363-378)create(1723-1729)create(1732-1738)create(1740-1754)get(388-404)get(407-423)get(426-442)get(445-461)get(464-480)get(483-499)get(501-559)get(2198-2214)get(2217-2233)get(2236-2252)get(2255-2271)get(2274-2290)get(2293-2309)get(2311-2369)infrahub_sdk/convert_object_type.py (2)
ConversionFieldInput(43-61)ConversionFieldValue(21-40)infrahub_sdk/testing/docker.py (1)
TestInfrahubDockerClient(36-47)
tests/unit/sdk/test_schema.py (1)
infrahub_sdk/ctl/schema.py (1)
display_schema_load_errors(51-94)
tests/unit/sdk/spec/test_object.py (3)
tests/unit/sdk/conftest.py (2)
client(32-33)mock_schema_query_01(1871-1878)infrahub_sdk/spec/object.py (4)
ObjectFile(630-652)validate_format(205-221)validate_format(645-649)spec(634-637)infrahub_sdk/exceptions.py (1)
ValidationError(117-129)
infrahub_sdk/client.py (5)
infrahub_sdk/branch.py (4)
BranchData(17-25)InfrahubBranchManager(73-220)get(193-203)get(233-243)infrahub_sdk/convert_object_type.py (1)
ConversionFieldInput(43-61)infrahub_sdk/graphql.py (3)
Mutation(173-204)render(161-170)render(181-204)infrahub_sdk/diff.py (1)
get_diff_summary_query(38-74)infrahub_sdk/node/node.py (4)
InfrahubNode(458-1086)from_graphql(486-500)from_graphql(1117-1131)InfrahubNodeSync(1089-1712)
tests/unit/sdk/test_range_expansion.py (1)
infrahub_sdk/spec/range_expansion.py (1)
range_expansion(88-118)
infrahub_sdk/ctl/validate.py (2)
infrahub_sdk/utils.py (1)
write_to_file(310-323)infrahub_sdk/config.py (1)
default_infrahub_branch(125-130)
🔇 Additional comments (45)
docs/docusaurus.config.ts (1)
112-114: No concerns—formatting only.The extra whitespace after the theme config is harmless and keeps the sections visually separated.
docs/package.json (1)
26-26: LGTM! Dependency version is current.The
react-playerversion ^3.3.2 is the latest in the 3.x line and is compatible with React ^18.0.0. Based on learnings, v3 offers improved architecture with lazy-loading per-source player code.docs/docs/python-sdk/introduction.mdx (1)
4-4: LGTM! Import path is correct.The relative import path correctly resolves from
docs/docs/python-sdk/todocs/src/components/VideoPlayer/.infrahub_sdk/ctl/utils.py (1)
122-123: LGTM! Centralized branch defaulting improves consistency.Defaulting to
client.config.default_infrahub_branchwhen no branch is provided centralizes branch resolution logic and aligns with the configuration-driven approach shown in the codebase.changelog/464.housekeeping.md (1)
1-1: LGTM! Clear changelog entry.The changelog entry clearly documents the improvement to error handling during schema loading.
infrahub_sdk/exceptions.py (1)
20-21: LGTM! Enhanced error reporting improves debuggability.Including the server response content in the error message provides valuable context when debugging JSON decode failures.
tests/unit/sdk/conftest.py (1)
180-220: LGTM! Well-structured test fixture for dropdown functionality.The new
location_schema_with_dropdownfixture properly extends the Location schema with a Dropdown attribute for testing dropdown-related functionality. The implementation follows the same pattern as the existinglocation_schemafixture and correctly converts the schema using.convert_api().infrahub_sdk/ctl/config.py (2)
5-5: LGTM! Good modernization to use stdlib tomllib.The conditional import pattern correctly uses Python 3.11+'s built-in
tomllibwhile falling back totomlifor older versions. This is a best practice for libraries supporting multiple Python versions.Also applies to: 12-15
68-68: LGTM! Correct usage of tomllib.The change from
toml.loadstotomllib.loadsaligns with the new import strategy and maintains the same functionality.infrahub_sdk/branch.py (2)
191-191: LGTM! Cleaner dictionary comprehension.The refactor removes the intermediate variable while maintaining identical behavior.
231-231: LGTM! Consistent with async variant.The refactor removes the intermediate variable while maintaining identical behavior, keeping async and sync implementations consistent.
infrahub_sdk/diff.py (3)
40-41: LGTM! Query expansion supports time-based filters.The addition of optional
name,from_time, andto_timeparameters expands diff query capabilities while maintaining backward compatibility.
120-120: LGTM! Cleaner direct return.The refactor removes the intermediate variable while maintaining identical behavior.
124-124: LGTM! Field name aligns with GraphQL query.The change to use
node_dict.get("status")correctly matches the field name queried in the GraphQL (line 45), ensuring proper data extraction from the API response.infrahub_sdk/node/node.py (2)
582-582: LGTM! Cleaner direct return.The refactor removes the intermediate variable while maintaining identical behavior.
1210-1210: LGTM! Consistent with async variant.The refactor removes the intermediate variable while maintaining identical behavior, keeping async and sync implementations consistent.
infrahub_sdk/spec/object.py (5)
3-4: LGTM! Imports support the new range expansion feature.The imports are necessary for the range expansion functionality:
copyfor isolating expanded items,refor pattern matching, and range expansion utilities from the new module.Also applies to: 13-13
208-210: LGTM! Range expansion integrated before validation.The expansion occurs before validation and updates
self.datain place, ensuring that both validation and subsequent processing operate on the expanded data. This is the correct flow for the feature.
225-226: LGTM! Consistent expansion before processing.The expansion occurs before node creation, ensuring each expanded item is processed individually. This is consistent with the validation flow.
351-352: LGTM! Range expansion for relationship data.The expansion correctly applies to
MANY_OBJ_DICT_LISTrelationship data before validation, ensuring expanded items are validated individually.
566-567: LGTM! Consistent expansion for node creation.The expansion for relationship data mirrors the validation flow, ensuring each expanded item is created as a separate related node.
changelog/473.fixed.md (1)
1-1: LGTM!The changelog entry accurately documents the JsonDecodeError enhancement, providing better debugging information for non-JSON server responses.
changelog/+convert-object-type.added.md (1)
1-1: LGTM!The changelog entry clearly documents the new object type conversion feature.
tests/unit/sdk/test_diff_summary.py (1)
112-112: LGTM!The test expectations correctly reflect the updated diff summary behavior where the action field is now populated from the status field. The changes are consistent across all three node assertions.
Also applies to: 127-127, 143-143
changelog/560.added.md (1)
1-1: LGTM!The changelog entry comprehensively documents the range expansion feature, including validation semantics and usage examples.
pyproject.toml (2)
48-48: LGTM!The migration from
tomltotomli(for Python <3.11) withtomllib(for Python 3.11+) follows the standard migration pattern. The dependency is correctly marked as optional and included in the appropriate extras groups.Also applies to: 72-72, 81-81
272-272: LGTM!Adding RET504 to the test file ignores is reasonable, as unnecessary variable assignments before returns in test code don't impact clarity or correctness.
tests/unit/sdk/test_utils.py (2)
1-1: LGTM!The new imports are appropriate for testing the
decode_jsonfunction andJsonDecodeErrorbehavior.Also applies to: 5-5, 11-11, 19-19
236-283: LGTM!The test coverage for
decode_jsonis comprehensive and well-structured:
test_decode_json_success: Validates the success pathtest_decode_json_failure_with_content: Verifies error message includes server content when availabletest_decode_json_failure_without_content: Confirms server content is omitted when emptytest_json_decode_error_custom_message: Ensures custom messages are preservedThe use of
Mockfor HTTP responses is appropriate, and the assertions correctly validate the enhanced error messaging.infrahub_sdk/ctl/cli_commands.py (1)
48-48: Approve centralization of branch resolution
execute_graphql_query in utils.py (lines 122–124) correctly defaultsbranchtoclient.config.default_infrahub_branchwhenbranchisNone.changelog/529.added.md (1)
1-2: LGTM!The changelog entry is clear and concise, accurately documenting the new diff functionality.
infrahub_sdk/node/attribute.py (1)
78-81: LGTM!The explicit handling of
Nonefor mutated optional attributes improves clarity and ensures that clearing an optional attribute is properly conveyed in the input data. The dual check for bothoptionalandvalue_has_been_mutatedis appropriate..github/workflows/ci.yml (2)
240-243: LGTM!The conditional installation of
tomlifor Python 3.9 and 3.10 correctly provides TOML parsing support for versions that don't have the built-intomllibmodule (available from Python 3.11+). This aligns with the project's migration from thetomllibrary totomllib/tomli.
149-149: Manually confirm invoke has no TOML dependency. Automated PyPI metadata retrieval failed with SSL errors—runpip show invokelocally or inspect invoke’srequires_diston PyPI to ensure it doesn’t implicitly requiretoml.infrahub_sdk/graphql.py (1)
21-24: LGTM!Using
json.dumps()for string serialization ensures proper escaping of special characters (quotes, newlines, backslashes, etc.) according to JSON rules, which are compatible with GraphQL. This is more robust than the previousf'"{value}"'approach, which could produce invalid GraphQL for strings containing quotes or escape sequences.tests/constants.py (1)
1-3: LGTM!The client type constants provide a clean way to parameterize integration tests across async and sync modes. The naming convention aligns with the codebase's existing patterns.
tests/unit/sdk/test_range_expansion.py (1)
1-106: LGTM!The test suite provides comprehensive coverage of the range expansion functionality:
- Core functionality: numeric, alphabetic, and mixed ranges
- Edge cases: single values, empty brackets, no brackets, malformed ranges
- Special cases: duplicates, whitespace, descending ranges, multiple adjacent ranges
- Advanced cases: non-alphanumeric, Unicode, and literal brackets
The test organization is clear, with each test function focusing on a specific scenario.
docs/docs/python-sdk/topics/object_file.mdx (1)
199-280: LGTM!The range expansion documentation is well-structured and follows the Diataxis framework for explanation/topic documentation:
- Clear explanation of how the feature works
- Practical examples covering common use cases
- Error scenarios with expected outputs
- Proper code block formatting with language tags
- Professional and concise tone
The documentation effectively guides users through single-field expansion, multi-field expansion with matching lengths, and the error case for mismatched lengths.
infrahub_sdk/ctl/schema.py (1)
78-91: LGTM!The enhanced error handling logic improves the robustness of schema error display by:
- Detecting whether
loc_path[6]is a string (attribute name) or an index- For string attributes: searching through
error_datato find the matching entry and extract the name- For numeric indices: using direct index access as before
This change ensures that deeply nested schema validation errors are displayed with appropriate context, making debugging easier for users.
tests/unit/sdk/test_node.py (1)
1373-1399: Dropdown clear-to-None path looks correctCovers optional Dropdown clearing and verifies _generate_input_data emits {"status": {"value": None}}. Matches attribute._generate_input_data semantics for mutated optionals. Looks good.
tests/unit/sdk/test_schema.py (1)
397-454: Good coverage for nested attribute error renderingValidates mapping loc to attribute labels and prints two detailed lines. This strengthens display_schema_load_errors for attribute-level details.
infrahub_sdk/convert_object_type.py (2)
21-41: One-of validation for ConversionFieldValue is correctThe post-validator enforces exactly one of attribute_value/peer_id/peers_ids. Matches intended API.
43-61: One-of validation for ConversionFieldInput is correctEnsures exactly one of source_field/data/use_default_value. Good guardrails for server expectations.
infrahub_sdk/client.py (2)
1215-1243: create_diff: validates time window and supports async task modeParameter checks and MUTATION_QUERY_TASK usage look correct.
1245-1271: get_diff_summary: optional filters and time range check look goodVariable passing aligns with get_diff_summary_query. Returns mapped NodeDiffs.
| def expand_data_with_ranges(data: list[dict[str, Any]]) -> list[dict[str, Any]]: | ||
| """Expand any item in self.data with range pattern in any value. Supports multiple fields, requires equal expansion length.""" | ||
| range_pattern = re.compile(MATCH_PATTERN) | ||
| expanded = [] | ||
| for item in data: | ||
| # Find all fields to expand | ||
| expand_fields = {} | ||
| for key, value in item.items(): | ||
| if isinstance(value, str) and range_pattern.search(value): | ||
| try: | ||
| expand_fields[key] = range_expansion(value) | ||
| except Exception: | ||
| # If expansion fails, treat as no expansion | ||
| expand_fields[key] = [value] | ||
| if not expand_fields: | ||
| expanded.append(item) | ||
| continue | ||
| # Check all expanded lists have the same length | ||
| lengths = [len(v) for v in expand_fields.values()] | ||
| if len(set(lengths)) > 1: | ||
| raise ValidationError(f"Range expansion mismatch: fields expanded to different lengths: {lengths}") | ||
| n = lengths[0] | ||
| # Zip expanded values and produce new items | ||
| for i in range(n): | ||
| new_item = copy.deepcopy(item) | ||
| for key, values in expand_fields.items(): | ||
| new_item[key] = values[i] | ||
| expanded.append(new_item) | ||
| return expanded |
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.
Fix ValidationError instantiation.
The ValidationError at line 190 is missing the required identifier parameter. According to the exception definition (see infrahub_sdk/exceptions.py), ValidationError requires an identifier argument.
Apply this diff to fix the error instantiation:
if len(set(lengths)) > 1:
- raise ValidationError(f"Range expansion mismatch: fields expanded to different lengths: {lengths}")
+ raise ValidationError(
+ identifier="range_expansion",
+ message=f"Range expansion mismatch: fields expanded to different lengths: {lengths}"
+ )However, the overall logic is sound: the function correctly expands range patterns, validates expansion consistency, and produces isolated items using deepcopy.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def expand_data_with_ranges(data: list[dict[str, Any]]) -> list[dict[str, Any]]: | |
| """Expand any item in self.data with range pattern in any value. Supports multiple fields, requires equal expansion length.""" | |
| range_pattern = re.compile(MATCH_PATTERN) | |
| expanded = [] | |
| for item in data: | |
| # Find all fields to expand | |
| expand_fields = {} | |
| for key, value in item.items(): | |
| if isinstance(value, str) and range_pattern.search(value): | |
| try: | |
| expand_fields[key] = range_expansion(value) | |
| except Exception: | |
| # If expansion fails, treat as no expansion | |
| expand_fields[key] = [value] | |
| if not expand_fields: | |
| expanded.append(item) | |
| continue | |
| # Check all expanded lists have the same length | |
| lengths = [len(v) for v in expand_fields.values()] | |
| if len(set(lengths)) > 1: | |
| raise ValidationError(f"Range expansion mismatch: fields expanded to different lengths: {lengths}") | |
| n = lengths[0] | |
| # Zip expanded values and produce new items | |
| for i in range(n): | |
| new_item = copy.deepcopy(item) | |
| for key, values in expand_fields.items(): | |
| new_item[key] = values[i] | |
| expanded.append(new_item) | |
| return expanded | |
| def expand_data_with_ranges(data: list[dict[str, Any]]) -> list[dict[str, Any]]: | |
| """Expand any item in self.data with range pattern in any value. Supports multiple fields, requires equal expansion length.""" | |
| range_pattern = re.compile(MATCH_PATTERN) | |
| expanded = [] | |
| for item in data: | |
| # Find all fields to expand | |
| expand_fields = {} | |
| for key, value in item.items(): | |
| if isinstance(value, str) and range_pattern.search(value): | |
| try: | |
| expand_fields[key] = range_expansion(value) | |
| except Exception: | |
| # If expansion fails, treat as no expansion | |
| expand_fields[key] = [value] | |
| if not expand_fields: | |
| expanded.append(item) | |
| continue | |
| # Check all expanded lists have the same length | |
| lengths = [len(v) for v in expand_fields.values()] | |
| if len(set(lengths)) > 1: | |
| raise ValidationError( | |
| identifier="range_expansion", | |
| message=f"Range expansion mismatch: fields expanded to different lengths: {lengths}" | |
| ) | |
| n = lengths[0] | |
| # Zip expanded values and produce new items | |
| for i in range(n): | |
| new_item = copy.deepcopy(item) | |
| for key, values in expand_fields.items(): | |
| new_item[key] = values[i] | |
| expanded.append(new_item) | |
| return expanded |
🤖 Prompt for AI Agents
In infrahub_sdk/spec/object.py around lines 170 to 198, the ValidationError
raised for mismatched expansion lengths is called without the required
identifier parameter; change the instantiation to provide an identifier (e.g.
"range_expansion_mismatch") as the first argument and the existing formatted
message as the second argument (for example
ValidationError("range_expansion_mismatch", f"Range expansion mismatch: fields
expanded to different lengths: {lengths}")), so the exception matches the
ValidationError signature.
| @@ -0,0 +1 @@ | |||
| {{ data['BuiltinTag']['edges'][0]['node']['name']['value'] }} No newline at end of file | |||
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.
Guard against missing tag edges before indexing.
Indexing edges[0] raises a TemplateRuntimeError whenever the GraphQL query returns no matches. Add a small guard so the transform keeps rendering even when no tag is found.
-{{
- data['BuiltinTag']['edges'][0]['node']['name']['value']
-}}
+{% set edges = data.get('BuiltinTag', {}).get('edges', []) %}
+{% if edges %}
+{{ edges[0]['node']['name']['value'] }}
+{% endif %}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{ data['BuiltinTag']['edges'][0]['node']['name']['value'] }} | |
| {% set edges = data.get('BuiltinTag', {}).get('edges', []) %} | |
| {% if edges %} | |
| {{ edges[0]['node']['name']['value'] }} | |
| {% endif %} |
🤖 Prompt for AI Agents
In tests/fixtures/repos/ctl_integration/templates/tags.j2 around line 1, the
template directly accesses
data['BuiltinTag']['edges'][0]['node']['name']['value'] which throws a
TemplateRuntimeError when edges is empty; update the template to guard for
missing edges by checking that data['BuiltinTag']['edges'] exists and has at
least one element before accessing [0], and render a sensible fallback (empty
string or placeholder) when no tag is present so the transform continues
rendering without error.
Replaces #552
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores