-
Notifications
You must be signed in to change notification settings - Fork 6
Refresh python packages #415
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: stable
Are you sure you want to change the base?
Conversation
Deploying infrahub-sdk-python with
|
Latest commit: |
f44c8e7
|
Status: | ✅ Deploy successful! |
Preview URL: | https://213ba63c.infrahub-sdk-python.pages.dev |
Branch Preview URL: | https://bkr-fix-python.infrahub-sdk-python.pages.dev |
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## stable #415 +/- ##
==========================================
- Coverage 75.76% 75.61% -0.15%
==========================================
Files 100 100
Lines 8838 8806 -32
Branches 1733 1324 -409
==========================================
- Hits 6696 6659 -37
- Misses 1666 1677 +11
+ Partials 476 470 -6
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 5 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.
I made a small comment regarding the change in pyproject.toml.
Also this PR has a merge conflict that needs to be resolved first. It looks like we're doing different things within this PR. Could we perhaps close this one and open two new ones, one for the package update and another one for the code changes and explain the changes with an issue or news fragment if needed?
""" WalkthroughThe changes introduce stricter checks for the presence and validity of IDs before proceeding with operations involving nodes, credentials, or relationships. File reading and writing operations are updated to explicitly use UTF-8 encoding. Dependency management for numpy in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Node
participant Repository
participant Importer
User->>Node: save()
alt Node.id is set
Node->>Node: Add related groups/nodes
else Node.id is not set
Node->>User: Raise UninitializedError
end
User->>Repository: Save credential
Repository->>Repository: Check credential.id
alt credential.id is not set
Repository->>User: Raise ValueError
end
Importer->>Importer: import_data()
alt node.id is set
Importer->>Importer: Add node to all_nodes
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (6)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
infrahub_sdk/node/node.py
(7 hunks)infrahub_sdk/node/related_node.py
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- infrahub_sdk/node/related_node.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
infrahub_sdk/node/node.py (4)
infrahub_sdk/exceptions.py (1)
UninitializedError
(156-157)infrahub_sdk/node/related_node.py (5)
id
(83-86)fetch
(204-210)fetch
(251-257)peer
(213-214)peer
(260-261)infrahub_sdk/node/relationship.py (2)
fetch
(148-182)fetch
(271-306)infrahub_sdk/query_groups.py (4)
add_related_groups
(127-138)add_related_groups
(221-232)add_related_nodes
(114-125)add_related_nodes
(208-219)
⏰ 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). (6)
- GitHub Check: unit-tests (3.12)
- GitHub Check: unit-tests (3.10)
- GitHub Check: unit-tests (3.11)
- GitHub Check: unit-tests (3.13)
- GitHub Check: integration-tests-latest-infrahub
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
infrahub_sdk/node/node.py (6)
8-14
: LGTM! Import addition is correctly placed.The
UninitializedError
import is properly added and follows the existing alphabetical ordering pattern.
583-585
: Good defensive programming with ID validation.The check ensures the artifact has a valid ID before attempting to fetch its definition relationship and generate the artifact, preventing potential issues with uninitialized objects.
1176-1178
: Consistent ID validation in sync version.The ID validation logic matches the async implementation, maintaining consistency across both interfaces.
629-643
: Excellent ID validation before group context operations.The validation ensures nodes have valid IDs before adding them to group contexts, preventing operations on uninitialized objects. The error messages are clear and descriptive.
828-828
: Good explicit type conversion for ID consistency.The explicit
str()
conversion ensures IDs are consistently handled as strings throughout the codebase, preventing potential type-related issues.
1417-1417
: Consistent ID type handling in sync version.The explicit string conversion matches the async implementation, maintaining type consistency across both interfaces.
I do not remember changing this one
Summary by CodeRabbit
Bug Fixes
Chores