Use pathlib instead of os for Path operations #722
Conversation
WalkthroughThis pull request transitions the codebase from using the Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: |
af827c1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7b71ad07.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pog-prefer-pathlib.infrahub-sdk-python.pages.dev |
ogenstad
force-pushed
the
pog-prefer-pathlib
branch
from
35395cd to
af827c1
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## infrahub-develop #722 +/- ##
=================================================
Coverage 76.28% 76.28%
=================================================
Files 114 114
Lines 9830 9831 +1
Branches 1508 1509 +1
=================================================
+ Hits 7499 7500 +1
Misses 1837 1837
Partials 494 494
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: 0
🧹 Nitpick comments (3)
tests/unit/ctl/test_render_app.py (1)
16-16: Simplify redundant Path nesting.The expression
Path(Path(Path(__file__).resolve()).parent / ".." / "..")has unnecessary nesting. OncePath(__file__).resolve()is called, you already have a Path object.🔎 Proposed simplification
-FIXTURE_BASE_DIR = Path(Path(Path(__file__).resolve()).parent / ".." / ".." / "fixtures" / "repos") +FIXTURE_BASE_DIR = Path(__file__).resolve().parent.parent.parent / "fixtures" / "repos"This is more idiomatic and avoids redundant Path() wrapping.
tests/integration/test_infrahubctl.py (1)
25-25: Simplify redundant Path nesting.The expression
Path(Path(Path(__file__).resolve()).parent / "..")has unnecessary nesting.🔎 Proposed simplification
-FIXTURE_BASE_DIR = Path(Path(Path(__file__).resolve()).parent / ".." / "fixtures") +FIXTURE_BASE_DIR = Path(__file__).resolve().parent.parent / "fixtures"This is more idiomatic and avoids redundant Path() wrapping.
tests/unit/ctl/test_transform_app.py (1)
21-23: Simplify redundant Path nesting.The expression
Path(Path(Path(__file__).resolve()).parent / ".." / "..")has unnecessary nesting.🔎 Proposed simplification
-FIXTURE_BASE_DIR = Path( - Path(Path(__file__).resolve()).parent / ".." / ".." / "fixtures" / "integration" / "test_infrahubctl" -) +FIXTURE_BASE_DIR = ( + Path(__file__).resolve().parent.parent.parent / "fixtures" / "integration" / "test_infrahubctl" +)This is more idiomatic and avoids redundant Path() wrapping.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
infrahub_sdk/checks.pyinfrahub_sdk/operation.pypyproject.tomltests/helpers/utils.pytests/integration/test_infrahubctl.pytests/unit/ctl/test_render_app.pytests/unit/ctl/test_transform_app.py
💤 Files with no reviewable changes (1)
- pyproject.toml
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use type hints on all function signatures
Never mix async/sync inappropriately
Never bypass type checking without justification
Files:
tests/integration/test_infrahubctl.pytests/helpers/utils.pyinfrahub_sdk/checks.pytests/unit/ctl/test_transform_app.pyinfrahub_sdk/operation.pytests/unit/ctl/test_render_app.py
tests/**/*.py
📄 CodeRabbit inference engine (tests/AGENTS.md)
tests/**/*.py: Usehttpx_mockfixture for HTTP mocking in tests instead of making real HTTP requests
Do not add@pytest.mark.asynciodecorator to async test functions (async auto-mode is globally enabled)
Files:
tests/integration/test_infrahubctl.pytests/helpers/utils.pytests/unit/ctl/test_transform_app.pytests/unit/ctl/test_render_app.py
tests/integration/**/*.py
📄 CodeRabbit inference engine (tests/AGENTS.md)
tests/integration/**/*.py: Integration tests should use testcontainers to interact with real Infrahub instances
Clean up resources in integration tests
Files:
tests/integration/test_infrahubctl.py
infrahub_sdk/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Follow async/sync dual pattern for new features in the Python SDK
Files:
infrahub_sdk/checks.pyinfrahub_sdk/operation.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (tests/AGENTS.md)
Unit tests must be fast, mocked, and have no external dependencies
Files:
tests/unit/ctl/test_transform_app.pytests/unit/ctl/test_render_app.py
🧠 Learnings (3)
📚 Learning: 2025-12-10T17:13:37.990Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: tests/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:37.990Z
Learning: Applies to tests/integration/**/*.py : Integration tests should use testcontainers to interact with real Infrahub instances
Applied to files:
tests/integration/test_infrahubctl.pytests/unit/ctl/test_transform_app.py
📚 Learning: 2025-12-10T17:13:21.977Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: infrahub_sdk/ctl/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:21.977Z
Learning: Applies to infrahub_sdk/ctl/**/*.py : Do not instantiate InfrahubClient directly; always use initialize_client() or initialize_client_sync() helper functions
Applied to files:
tests/integration/test_infrahubctl.py
📚 Learning: 2025-11-25T13:23:15.190Z
Learnt from: wvandeun
Repo: opsmill/infrahub-sdk-python PR: 637
File: infrahub_sdk/operation.py:74-76
Timestamp: 2025-11-25T13:23:15.190Z
Learning: In infrahub_sdk/operation.py, the recursive=True parameter in _process_relationships is a temporary workaround to access Proposed Changes data. This will be replaced with proper object-level metadata implementation in version 1.7.
Applied to files:
infrahub_sdk/operation.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). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
infrahub_sdk/checks.py (1)
5-5: LGTM! Clean migration to pathlib.The change from
os.getcwd()tostr(pathlib.Path.cwd())is correct and maintains backward compatibility by preserving the string type forroot_directory.Also applies to: 58-58
tests/helpers/utils.py (1)
18-18: LGTM! Pathlib migration is correct.The change from
os.getcwd()toPath.cwd()is appropriate. The subsequentos.chdir()calls correctly accept Path objects as per PEP 519.Also applies to: 33-33
infrahub_sdk/operation.py (1)
3-3: LGTM! Clean migration to pathlib.The change from
os.getcwd()tostr(pathlib.Path.cwd())is correct and maintains backward compatibility by preserving the string type forroot_directory.Also applies to: 25-25
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.