-
Notifications
You must be signed in to change notification settings - Fork 6
Add support for clearing optional attributes #548
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
Conversation
…ts for dropdown handling
WalkthroughAttribute input generation now preserves explicit None for optional attributes that have been mutated: when value is None and schema.optional and value_has_been_mutated, Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⏰ 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). (3)
🔇 Additional comments (1)
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: |
4993c13
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://111a9994.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://atg-20250915-ifc-1853.infrahub-sdk-python.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## stable #548 +/- ##
==========================================
+ Coverage 75.59% 75.65% +0.06%
==========================================
Files 100 100
Lines 8892 8894 +2
Branches 1750 1751 +1
==========================================
+ Hits 6722 6729 +7
+ Misses 1688 1685 -3
+ Partials 482 480 -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:
|
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 (4)
changelog/7218.fixed.md (1)
1-1: Changelog entry reads well; tiny wording/style nitConsider tightening and matching the schema kind casing.
-Allow the ability to clear optional dropdown attributes by setting them to None. +Allow clearing optional Dropdown attributes by setting them to None.infrahub_sdk/node/attribute.py (1)
78-81: Include properties on clear; avoid early return so flags propagateWhen clearing a Dropdown to None, we likely still want to carry attribute properties (e.g., is_protected). Let the function continue so the flags/objects sections run, and keep the return shape consistent.
- if self.value is None: - if self._schema.kind == "Dropdown" and self._schema.optional: - data["value"] = None - return data + if self.value is None: + if self._schema.kind == "Dropdown" and self._schema.optional: + # Explicit clear for optional Dropdowns; continue so properties are included. + data["value"] = None + else: + return datatests/unit/sdk/conftest.py (1)
180-220: Optional: reduce duplication with base schemaYou could build from
location_schemadata and append thestatusattribute to minimize drift between fixtures.tests/unit/sdk/test_node.py (1)
1373-1399: Optional: assert behavior with exclude_unmodified=TrueClearing should count as a modification; asserting this guards regressions in strip-unmodified logic.
assert node._generate_input_data()["data"] == { "data": { "name": {"value": "JFK1"}, "description": {"value": "JFK Airport"}, "type": {"value": "SITE"}, "status": {"value": None}, "primary_tag": None, } } + + # Clearing should still be emitted when excluding unmodified fields + input_data = node._generate_input_data(exclude_unmodified=True) + assert input_data["data"]["data"]["status"] == {"value": None}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
changelog/7218.fixed.md(1 hunks)infrahub_sdk/node/attribute.py(1 hunks)tests/unit/sdk/conftest.py(1 hunks)tests/unit/sdk/test_node.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/node/attribute.pytests/unit/sdk/test_node.pytests/unit/sdk/conftest.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Place and write unit tests under tests/unit/ (isolated component tests)
Files:
tests/unit/sdk/test_node.pytests/unit/sdk/conftest.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_node.pytests/unit/sdk/conftest.py
🧬 Code graph analysis (2)
tests/unit/sdk/test_node.py (5)
tests/unit/sdk/conftest.py (2)
client(32-33)location_schema_with_dropdown(181-219)infrahub_sdk/node/node.py (1)
_generate_input_data(195-281)infrahub_sdk/node/attribute.py (3)
value(66-67)value(70-72)_generate_input_data(74-105)infrahub_sdk/node/relationship.py (1)
_generate_input_data(60-61)infrahub_sdk/node/related_node.py (1)
_generate_input_data(128-145)
tests/unit/sdk/conftest.py (2)
infrahub_sdk/schema/main.py (4)
NodeSchemaAPI(311-313)NodeSchema(306-308)convert_api(283-284)convert_api(307-308)infrahub_sdk/yaml.py (1)
data(147-150)
⏰ 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). (4)
- GitHub Check: unit-tests (3.12)
- GitHub Check: unit-tests (3.13)
- GitHub Check: integration-tests-latest-infrahub
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
infrahub_sdk/node/attribute.py (1)
78-81: Correctly surfaces null for optional DropdownsThis fixes the inability to clear optional Dropdown attributes by emitting
{"value": None}. Works with existing_generate_input_dataplumbing innode.py.tests/unit/sdk/conftest.py (1)
180-220: Good, focused fixture for Dropdown coverageThe fixture mirrors
location_schemaand adds a Dropdown cleanly; fits existing test patterns.tests/unit/sdk/test_node.py (1)
1373-1399: Solid test validating clear-to-null behavior across async/sync nodesCovers initial value, clear to None, and expected payload shape including
primary_tag: None.
…onal 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 <[email protected]>
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores