-
Notifications
You must be signed in to change notification settings - Fork 7
Upgrade ruff #636
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
Upgrade ruff #636
Conversation
WalkthroughThis pull request updates the ruff linter from version 0.11.0 to 0.14.5 across workflow configuration, documentation, and dependencies. Multiple file operations are updated to explicitly specify UTF-8 encoding when reading and writing text files. Default arguments are simplified in several dictionary.get() calls by removing explicit None defaults. The linting configuration is augmented to include ASYNC240 in the per-file-ignores ruleset. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: |
2470982
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://dee4d948.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pog-upgrade-ruff.infrahub-sdk-python.pages.dev |
a9b2040 to
2470982
Compare
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## infrahub-develop #636 +/- ##
=================================================
Coverage 75.56% 75.56%
=================================================
Files 113 113
Lines 9514 9514
Branches 1448 1448
=================================================
Hits 7189 7189
Misses 1838 1838
Partials 487 487
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pyproject.toml (1)
249-251: ASYNC240 added to ignores—tracked for future fix.Adding ASYNC240 (async pathlib.Path usage) to per-file-ignores is reasonable for now, as noted in the PR description. This rule flags async functions using
pathlib.Pathmethods instead of async-friendly alternatives likeanyio.path.Would you like me to search the codebase for async functions using pathlib.Path methods to assess the scope of work needed for the future PR that addresses this rule?
#!/bin/bash # Find async functions that may be using pathlib.Path methods ast-grep --pattern $'async def $FUNC($$$) { $$$ $_.read_text($$$) $$$ }' ast-grep --pattern $'async def $FUNC($$$) { $$$ $_.write_text($$$) $$$ }'infrahub_sdk/node/related_node.py (1)
74-74: Consider applying the same cleanup for consistency.Line 74 has the same nested
.get()pattern with an explicitNonedefault that could be removed for consistency with the change on line 67.Apply this diff to remove the redundant
None:- prop_data = properties_data.get(prop, properties_data.get(f"_relation__{prop}", None)) + prop_data = properties_data.get(prop, properties_data.get(f"_relation__{prop}"))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.github/workflows/ci.yml(1 hunks)CLAUDE.md(1 hunks)infrahub_sdk/ctl/graphql.py(1 hunks)infrahub_sdk/node/attribute.py(2 hunks)infrahub_sdk/node/related_node.py(1 hunks)infrahub_sdk/pytest_plugin/items/jinja2_transform.py(1 hunks)infrahub_sdk/transfer/importer/json.py(1 hunks)infrahub_sdk/utils.py(1 hunks)pyproject.toml(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
infrahub_sdk/utils.py (1)
infrahub_sdk/testing/repository.py (1)
path(57-58)
⏰ 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 (9)
infrahub_sdk/ctl/graphql.py (1)
111-111: LGTM! Explicit UTF-8 encoding is best practice.Adding explicit
encoding="utf-8"towrite_text()ensures consistent behavior across platforms and aligns with PEP 597 recommendations.infrahub_sdk/pytest_plugin/items/jinja2_transform.py (1)
86-86: LGTM! Explicit UTF-8 encoding for template files.Adding
encoding="utf-8"toread_text()ensures consistent decoding of Jinja2 templates across platforms.infrahub_sdk/transfer/importer/json.py (1)
147-147: LGTM! Explicit UTF-8 encoding for JSON files.Adding
encoding="utf-8"toread_text()ensures consistent decoding of JSON relationship data.pyproject.toml (1)
84-84: Ruff version upgrade is consistent.The upgrade to 0.14.5 matches the CI workflow update and enables the new lint rules that prompted the encoding fixes in this PR.
CLAUDE.md (1)
167-167: Documentation updated to reflect ruff 0.14.5.Keeping the documentation in sync with the actual version upgrade is good practice.
infrahub_sdk/utils.py (1)
321-321: LGTM! Explicit UTF-8 encoding for file writes.Adding
encoding="utf-8"towrite_text()ensures consistent behavior across platforms and completes the UTF-8 encoding consistency improvements in this PR.infrahub_sdk/node/attribute.py (1)
38-57: LGTM! Simplified dict.get() calls are functionally equivalent.Removing explicit
Nonedefaults fromdata.get()calls is functionally equivalent sincedict.get()returnsNoneby default when the key is not found. This appears to be an autofixable ruff simplification that improves code clarity without changing behavior..github/workflows/ci.yml (1)
81-81: Verify Ruff upgrade impact on configuration and linting behavior.The upgrade from 0.11.0 to 0.14.5 includes notable changes that require verification:
- Default Python target updated to 3.14 in 0.14.x series, which can affect rule applicability and diagnostics
- New/stabilized rules and preview features promoted across 0.12–0.14, which can change which diagnostics are reported by default
- Ruff uses minor-version bumps for potentially breaking changes in config and stable-rule behavior
Before merging, confirm:
- Your project's
requires-pythonandtarget-versionsettings are correct for Python 3.14 (or explicitly pin if needed)- Run linting with 0.14.5 and verify the rule output hasn't unexpectedly changed or broken any existing lint configurations
- Review the Ruff CHANGELOG for any rules relevant to your codebase
infrahub_sdk/node/related_node.py (1)
67-67: LGTM! RedundantNonedefault removed.The explicit
Nonedefault in the inner.get()call is unnecessary since.get()returnsNoneby default when the key is not found. This cleanup aligns with the ruff upgrade objectives.
Upgrade ruff, and add some autofixable errors due to violations in the new version. Also added a new rule to the list of exceptions (
Async functions should not use pathlib.Path methods, use trio.Path or anyio.path, this one would be good to fix in an upcoming PR though).Summary by CodeRabbit