Remove unnecessary assignments before return statement
#542
Conversation
WalkthroughThis change refactors multiple modules to inline return expressions by removing intermediate local variables before returning. Updates span branch management, client version retrieval (async and sync), diff construction, node artifact fetch (async and sync), playback response creation, repository active branch retrieval, and timestamp parsing. Behavior and control flow remain the same, with no public API or signature changes. The Ruff configuration in pyproject.toml now enforces RET504 globally, with RET504 ignored only for tests via per-file ignores. Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches
🧪 Generate unit tests
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: |
8c266e7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://698c030d.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pog-returns.infrahub-sdk-python.pages.dev |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## stable #542 +/- ##
==========================================
- Coverage 75.74% 75.57% -0.18%
==========================================
Files 100 100
Lines 9421 8891 -530
Branches 1949 1750 -199
==========================================
- Hits 7136 6719 -417
+ Misses 1755 1689 -66
+ Partials 530 483 -47
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:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
infrahub_sdk/node/node.py (1)
578-583: Wrong feature message for fetch (async).
artifact_fetch()validates support using the “generate” message constant instead of the “fetch” one. This yields a misleading error.Apply:
- self._validate_artifact_support(ARTIFACT_GENERATE_FEATURE_NOT_SUPPORTED_MESSAGE) + self._validate_artifact_support(ARTIFACT_FETCH_FEATURE_NOT_SUPPORTED_MESSAGE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
infrahub_sdk/branch.py(2 hunks)infrahub_sdk/client.py(2 hunks)infrahub_sdk/diff.py(1 hunks)infrahub_sdk/node/node.py(2 hunks)infrahub_sdk/playback.py(1 hunks)infrahub_sdk/repository.py(1 hunks)infrahub_sdk/timestamp.py(1 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/node.pyinfrahub_sdk/branch.pyinfrahub_sdk/playback.pyinfrahub_sdk/timestamp.pyinfrahub_sdk/diff.pyinfrahub_sdk/repository.pyinfrahub_sdk/client.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
🧠 Learnings (1)
📚 Learning: 2025-08-24T13:35:12.998Z
Learnt from: CR
PR: opsmill/infrahub-sdk-python#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T13:35:12.998Z
Learning: Maintain a dual client pattern: InfrahubClient (async) and InfrahubClientSync (sync) must provide identical interfaces
Applied to files:
infrahub_sdk/client.py
⏰ 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). (2)
- GitHub Check: integration-tests-latest-infrahub
- GitHub Check: unit-tests (3.13)
🔇 Additional comments (10)
infrahub_sdk/playback.py (1)
59-59: Inline return is correct.Semantics unchanged; constructing Response directly here is fine.
pyproject.toml (1)
275-276: Scope RET504 to tests only — good move.This enforces the no-temp-before-return rule in prod code while keeping tests flexible.
infrahub_sdk/timestamp.py (1)
46-46: Minor simplification looks good.No behavior change; retains the same exception semantics.
infrahub_sdk/diff.py (1)
120-127: Directly returning NodeDiff is fine.No functional change; elements list is preserved.
infrahub_sdk/client.py (2)
307-311: Async get_version inlining — looks good and consistent.Matches the sync version and keeps the dual-client interface aligned.
1601-1605: Sync get_version inlining — looks good.Symmetric with async variant; interface parity maintained.
infrahub_sdk/branch.py (2)
191-191: Dict comprehension is fine.Same behavior with clearer intent.
231-231: Sync variant matches async — good.Keeps both managers consistent.
infrahub_sdk/node/node.py (2)
582-582: Inline await return — good.No semantic change; reads cleaner.
1210-1210: Sync inline return — good.Matches the async path; consistent and concise.
| def active_branch(self) -> str | None: | ||
| active_branch = porcelain.active_branch(self.root_directory).decode("utf-8") | ||
| return active_branch | ||
| return porcelain.active_branch(self.root_directory).decode("utf-8") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle detached HEAD to avoid AttributeError.
porcelain.active_branch() can return None (e.g., detached HEAD). Decoding without a guard may crash.
Apply:
- return porcelain.active_branch(self.root_directory).decode("utf-8")
+ branch = porcelain.active_branch(self.root_directory)
+ return branch.decode("utf-8") if branch else None📝 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.
| return porcelain.active_branch(self.root_directory).decode("utf-8") | |
| branch = porcelain.active_branch(self.root_directory) | |
| return branch.decode("utf-8") if branch else None |
🤖 Prompt for AI Agents
In infrahub_sdk/repository.py around line 32,
porcelain.active_branch(self.root_directory) can return None (detached HEAD) so
calling .decode("utf-8") will raise AttributeError; update the code to first
assign the result to a variable, check if it's None, and if so return a sensible
fallback (e.g., None or the current commit id), otherwise decode and return the
branch name.
2 participants
Summary by CodeRabbit
Refactor
Chores