Use RelationshipCardinality enum in favor of a string#913
Use RelationshipCardinality enum in favor of a string#913ogenstad merged 1 commit intoinfrahub-developfrom
Conversation
Deploying infrahub-sdk-python with
|
| Latest commit: |
35cf000
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6633528d.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pog-use-relationship-cardina.infrahub-sdk-python.pages.dev |
WalkthroughThis pull request refactors cardinality comparison logic across multiple modules in the infrahub SDK. String-based cardinality checks using literals like 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## infrahub-develop #913 +/- ##
=================================================
Coverage 81.03% 81.03%
=================================================
Files 121 121
Lines 10642 10643 +1
Branches 1583 1583
=================================================
+ Hits 8624 8625 +1
Misses 1501 1501
Partials 517 517
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
infrahub_sdk/transfer/importer/json.py (1)
134-142: Update exporter to use enum-based cardinality checks for consistency.The importer correctly migrated cardinality comparisons to use
RelationshipCardinality.MANYenum (lines 134, 139), but the exporter (infrahub_sdk/transfer/exporter/json.py, lines 46, 53) still uses string literals ("many"). Align the exporter with the importer's approach by replacing string comparisons with enum comparisons for consistency and type safety across the transfer module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sdk/transfer/importer/json.py` around lines 134 - 142, The exporter still compares cardinality using string literals ("many"); change those comparisons to use the enum value RelationshipCardinality.MANY (e.g., replace occurrences like card == "many" or card != "many" with card == RelationshipCardinality.MANY / card != RelationshipCardinality.MANY) in the JSON exporter logic that mirrors the importer (check locations around the previous string checks in infrahub_sdk/transfer/exporter/json.py), and ensure RelationshipCardinality is imported/accessible in that module so the enum-based comparisons compile and mirror the importer behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@infrahub_sdk/transfer/importer/json.py`:
- Around line 134-142: The exporter still compares cardinality using string
literals ("many"); change those comparisons to use the enum value
RelationshipCardinality.MANY (e.g., replace occurrences like card == "many" or
card != "many" with card == RelationshipCardinality.MANY / card !=
RelationshipCardinality.MANY) in the JSON exporter logic that mirrors the
importer (check locations around the previous string checks in
infrahub_sdk/transfer/exporter/json.py), and ensure RelationshipCardinality is
imported/accessible in that module so the enum-based comparisons compile and
mirror the importer behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4deab71c-2f62-4d12-9c05-94cc9c84158d
📒 Files selected for processing (5)
infrahub_sdk/node/node.pyinfrahub_sdk/protocols_generator/generator.pyinfrahub_sdk/schema/__init__.pyinfrahub_sdk/spec/object.pyinfrahub_sdk/transfer/importer/json.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
infrahub_sdk/transfer/importer/json.py (1)
134-142: Update exporter to use enum-based cardinality checks for consistency.The importer correctly migrated cardinality comparisons to use
RelationshipCardinality.MANYenum (lines 134, 139), but the exporter (infrahub_sdk/transfer/exporter/json.py, lines 46, 53) still uses string literals ("many"). Align the exporter with the importer's approach by replacing string comparisons with enum comparisons for consistency and type safety across the transfer module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sdk/transfer/importer/json.py` around lines 134 - 142, The exporter still compares cardinality using string literals ("many"); change those comparisons to use the enum value RelationshipCardinality.MANY (e.g., replace occurrences like card == "many" or card != "many" with card == RelationshipCardinality.MANY / card != RelationshipCardinality.MANY) in the JSON exporter logic that mirrors the importer (check locations around the previous string checks in infrahub_sdk/transfer/exporter/json.py), and ensure RelationshipCardinality is imported/accessible in that module so the enum-based comparisons compile and mirror the importer behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@infrahub_sdk/transfer/importer/json.py`:
- Around line 134-142: The exporter still compares cardinality using string
literals ("many"); change those comparisons to use the enum value
RelationshipCardinality.MANY (e.g., replace occurrences like card == "many" or
card != "many" with card == RelationshipCardinality.MANY / card !=
RelationshipCardinality.MANY) in the JSON exporter logic that mirrors the
importer (check locations around the previous string checks in
infrahub_sdk/transfer/exporter/json.py), and ensure RelationshipCardinality is
imported/accessible in that module so the enum-based comparisons compile and
mirror the importer behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4deab71c-2f62-4d12-9c05-94cc9c84158d
📒 Files selected for processing (5)
infrahub_sdk/node/node.pyinfrahub_sdk/protocols_generator/generator.pyinfrahub_sdk/schema/__init__.pyinfrahub_sdk/spec/object.pyinfrahub_sdk/transfer/importer/json.py
Why
We have an enum for
RelationshipCardinalitybut in a number of places we were instead using a hard coded string, this is a slight cleanup to ensure that we have the correct types throughout the code.What changed
RelationshipCardinalityenumSummary by CodeRabbit