Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 10, 2025

Addresses Coderabbit feedback from #687: the condition for detecting hierarchy support used or with not isinstance, which always evaluated to True. This caused hierarchy support to be incorrectly enabled for ProfileSchemaAPI, GenericSchemaAPI, and TemplateSchemaAPI types.

Changes

  • Fixed isinstance check to use tuple syntax for multiple types
  • Changed from not isinstance(x, A) or not isinstance(x, B) or not isinstance(x, C) to not isinstance(x, (A, B, C))

Before/After

# Before - always True due to OR logic
if (
    not isinstance(schema, ProfileSchemaAPI)
    or not isinstance(schema, GenericSchemaAPI)
    or not isinstance(schema, TemplateSchemaAPI)
):
    self._hierarchy_support = getattr(schema, "hierarchy", None) is not None

# After - correct exclusion logic  
if not isinstance(schema, (ProfileSchemaAPI, GenericSchemaAPI, TemplateSchemaAPI)):
    self._hierarchy_support = getattr(schema, "hierarchy", None) is not None

This ensures hierarchy support is only enabled for node schemas that actually support hierarchical relationships.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@coderabbitai
Copy link

coderabbitai bot commented Dec 10, 2025

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Comment @coderabbitai help to get the list of available commands and usage tips.

Fixed incorrect boolean logic that used 'or' instead of tuple syntax in isinstance check for ProfileSchemaAPI, GenericSchemaAPI, and TemplateSchemaAPI. This ensures hierarchy support is correctly detected only for node schemas that are not Profile, Generic, or Template types.

Co-authored-by: BeArchiTek <[email protected]>
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Dec 10, 2025

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: fb57960
Status: ✅  Deploy successful!
Preview URL: https://e66aadae.infrahub-sdk-python.pages.dev
Branch Preview URL: https://copilot-sub-pr-687.infrahub-sdk-python.pages.dev

View logs

Copilot AI changed the title [WIP] Implement feedback from Coderabbit on SDK 1.17.0 merge PR Fix boolean logic error in hierarchy support detection Dec 10, 2025
Copilot AI requested a review from BeArchiTek December 10, 2025 09:46
@BeArchiTek BeArchiTek marked this pull request as ready for review December 10, 2025 10:15
@codecov
Copy link

codecov bot commented Dec 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@             Coverage Diff             @@
##           develop     #688      +/-   ##
===========================================
+ Coverage    76.01%   76.03%   +0.02%     
===========================================
  Files          113      113              
  Lines         9742     9742              
  Branches      1490     1490              
===========================================
+ Hits          7405     7407       +2     
+ Misses        1841     1840       -1     
+ Partials       496      495       -1     
Flag Coverage Δ
integration-tests 34.63% <100.00%> (+0.02%) ⬆️
python-3.10 49.94% <100.00%> (ø)
python-3.11 49.94% <100.00%> (ø)
python-3.12 49.94% <100.00%> (+0.04%) ⬆️
python-3.13 49.92% <100.00%> (+0.02%) ⬆️
python-3.14 51.58% <100.00%> (ø)
python-filler-3.12 23.94% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
infrahub_sdk/node/node.py 79.17% <100.00%> (+0.21%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@BeArchiTek BeArchiTek merged commit e9e214a into develop Dec 10, 2025
23 of 34 checks passed
@BeArchiTek BeArchiTek deleted the copilot/sub-pr-687 branch December 10, 2025 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants