refactor: ci using pixi and ruff/flake8#472
Conversation
|
Warning Review limit reached
More reviews will be available in 55 minutes and 50 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughReplaces Black and flake8 with Ruff for formatting and linting. Introduces ChangesRuff + pixi toolchain migration
Build system and release workflow modernization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
setup.py.bck (1)
36-36: ⚡ Quick winAvoid clobbering externally provided
CFLAGS.Line 36 overwrites existing build flags from CI/users. Append
-O3instead of replacing to keep upstream build controls intact.Proposed fix
-os.environ["CFLAGS"] = "-O3" +os.environ["CFLAGS"] = f'{os.environ.get("CFLAGS", "").strip()} -O3'.strip()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@setup.py.bck` at line 36, The assignment at line 36 where os.environ["CFLAGS"] is set to "-O3" overwrites any existing CFLAGS value that may have been set externally by CI or users. Instead of using direct assignment with the equals operator, append the "-O3" flag to the existing CFLAGS environment variable. Retrieve the current CFLAGS value (defaulting to an empty string if not set) and concatenate it with "-O3" before reassigning it back to os.environ["CFLAGS"].
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 31-32: The "Check code" step in the workflow is using `poetry run
lint` which is incompatible with the Pixi-based job configuration. Replace the
`poetry run lint` command in the run field of the "Check code" step with the
equivalent Pixi command to run the lint task directly, using the format `pixi
run lint` to maintain consistency with Pixi's task runner.
In `@pyproject.toml`:
- Around line 42-48: The ruff configuration has redundant settings for
tests/test_scontrol_parsing.py where the file is listed in extend-exclude (which
completely removes it from linting) and also has a per-file-ignores entry for
E501 that never gets applied because excluded files skip linting entirely.
Choose one strategy: either remove tests/test_scontrol_parsing.py from the
extend-exclude list and keep the per-file-ignores entry to lint the file with
E501 ignored, OR remove the per-file-ignores entry for that file from the
tool.ruff.lint.per-file-ignores section and keep it fully excluded.
In `@setup.py.bck`:
- Around line 22-33: The current approach of combining all Python files from the
base_dir into a single Extension named snakemake_executor_plugin_slurm._core
will make submodules like utils, accounts, and partitions non-importable,
breaking imports throughout the codebase and tests. Instead of iterating through
all .py files and combining them into one extension with py_files, create a
separate Extension object for each submodule in the list (e.g., one Extension
for snakemake_executor_plugin_slurm.utils, one for
snakemake_executor_plugin_slurm.accounts, etc.) with each submodule's
corresponding Python file. This preserves the submodule import structure that
the codebase depends on.
- Line 47: The install_requires parameter in setup.py is using
list(dependencies.keys()) which extracts only the package names and drops all
version constraints from the dependencies dictionary. This allows unconstrained
versions to be installed, breaking compatibility. Replace the
list(dependencies.keys()) call to preserve the full dependency specifications
including their version ranges, either by using the appropriate dictionary
values or by combining package names with their version constraints in the
proper format expected by install_requires.
---
Nitpick comments:
In `@setup.py.bck`:
- Line 36: The assignment at line 36 where os.environ["CFLAGS"] is set to "-O3"
overwrites any existing CFLAGS value that may have been set externally by CI or
users. Instead of using direct assignment with the equals operator, append the
"-O3" flag to the existing CFLAGS environment variable. Retrieve the current
CFLAGS value (defaulting to an empty string if not set) and concatenate it with
"-O3" before reassigning it back to os.environ["CFLAGS"].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9f5bbc66-a549-4082-bd88-0d2f7803dda6
📒 Files selected for processing (5)
.github/workflows/ci.ymlpixi.tomlpyproject.tomlsetup.cfgsetup.py.bck
💤 Files with no reviewable changes (1)
- setup.cfg
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
10-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDeclare explicit least-privilege
permissionsfor the workflow token.No
permissions:block is defined, so default token scopes apply. Add explicit minimal permissions to reduce blast radius and satisfy policy checks.Suggested fix
name: CI +permissions: + contents: read on: push:If any job later needs extra scopes, elevate only in that specific job.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 10 - 57, The workflow lacks an explicit permissions block, which means the default token scopes are being used. Add a top-level permissions block at the workflow level (before the jobs section) that declares minimal required permissions. Set contents to read since the workflow only needs checkout access. If any individual job later requires elevated permissions, those can be specified at the job level to override the workflow-level defaults while keeping other jobs restricted.Source: Linters/SAST tools
🧹 Nitpick comments (2)
.github/workflows/release-please.yml (2)
34-39: ⚖️ Poor tradeoffConsider using PyPI Trusted Publishing for enhanced security.
The current token-based authentication works, but PyPI's Trusted Publishing (OIDC) provides stronger security by eliminating long-lived secrets. This requires one-time configuration on PyPI to trust this GitHub repository/workflow.
To enable trusted publishing, you would:
- Configure the publisher on PyPI for this package
- Simplify the publish step:
- name: Publish to PyPI if: ${{ steps.release.outputs.release_created }} uses: pypa/gh-action-pypi-publish@release/v1 # No credentials needed - uses OIDCThis is optional but recommended when feasible.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release-please.yml around lines 34 - 39, In the "Publish to PyPI" step using the pypa/gh-action-pypi-publish@release/v1 action, remove the `user` and `password` fields (which currently use `__token__` and `secrets.PYPI_TOKEN`). After removing these authentication parameters, configure PyPI Trusted Publishing by creating a trusted publisher in your PyPI project settings for this GitHub repository and workflow to enable OIDC-based authentication. The action will automatically use OIDC once the publisher is configured on PyPI.Source: Linters/SAST tools
12-12: Consider pinning actions to commit SHAs for enhanced supply-chain security.While the action versions used (
actions/checkout@v6,actions/setup-python@v6,googleapis/release-please-action@v5, andpypa/gh-action-pypi-publish@release/v1) are valid and current, pinning to full commit SHAs instead of version tags provides stronger security guarantees against tag tampering or unexpected updates.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release-please.yml at line 12, Replace the version tags in all GitHub Actions with their full commit SHAs for enhanced security. Specifically, update googleapis/release-please-action from `@v5` to its full commit SHA, and also pin actions/checkout, actions/setup-python, and pypa/gh-action-pypi-publish to their respective full commit SHAs instead of using version tags. This prevents potential tag tampering and ensures reproducible, secure builds by explicitly specifying the exact commit being used.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Line 17: Replace all three occurrences of the mutable tag
`prefix-dev/setup-pixi@v0` with an immutable commit SHA reference in the GitHub
Actions workflow file. Specifically, at the three locations where `uses:
prefix-dev/setup-pixi@v0` appears (lines 17, 29, and 50), change the tag from
`@v0` to `@<commit-sha>` where commit-sha is the full commit hash of a specific
release from the setup-pixi repository. This prevents action supply-chain drift
by pinning to an immutable commit digest rather than a mutable version tag.
In @.github/workflows/release-please.yml:
- Around line 18-21: Add the `persist-credentials: false` parameter to the
`actions/checkout@v6` step to disable credential persistence and prevent
potential credential leakage. This should be added as a new line in the `with`
section alongside the existing `fetch-depth: 0` parameter.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 10-57: The workflow lacks an explicit permissions block, which
means the default token scopes are being used. Add a top-level permissions block
at the workflow level (before the jobs section) that declares minimal required
permissions. Set contents to read since the workflow only needs checkout access.
If any individual job later requires elevated permissions, those can be
specified at the job level to override the workflow-level defaults while keeping
other jobs restricted.
---
Nitpick comments:
In @.github/workflows/release-please.yml:
- Around line 34-39: In the "Publish to PyPI" step using the
pypa/gh-action-pypi-publish@release/v1 action, remove the `user` and `password`
fields (which currently use `__token__` and `secrets.PYPI_TOKEN`). After
removing these authentication parameters, configure PyPI Trusted Publishing by
creating a trusted publisher in your PyPI project settings for this GitHub
repository and workflow to enable OIDC-based authentication. The action will
automatically use OIDC once the publisher is configured on PyPI.
- Line 12: Replace the version tags in all GitHub Actions with their full commit
SHAs for enhanced security. Specifically, update
googleapis/release-please-action from `@v5` to its full commit SHA, and also pin
actions/checkout, actions/setup-python, and pypa/gh-action-pypi-publish to their
respective full commit SHAs instead of using version tags. This prevents
potential tag tampering and ensures reproducible, secure builds by explicitly
specifying the exact commit being used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f49d0fd9-2dcc-455a-85ee-64b0169c82da
📒 Files selected for processing (5)
.github/workflows/ci.yml.github/workflows/release-please.ymlsnakemake_executor_plugin_slurm/__init__.pytests/tests.pyupgrade_dependencies.py
✅ Files skipped from review due to trivial changes (3)
- upgrade_dependencies.py
- tests/tests.py
- snakemake_executor_plugin_slurm/init.py
| - uses: actions/checkout@v6 | ||
| if: ${{ steps.release.outputs.release_created }} | ||
| with: | ||
| python-version: "3.11" | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
Add persist-credentials: false to prevent credential persistence.
The checkout step should disable credential persistence to prevent potential credential leakage through artifacts or subsequent steps.
Proposed fix
- uses: actions/checkout@v6
if: ${{ steps.release.outputs.release_created }}
with:
fetch-depth: 0
+ persist-credentials: false🧰 Tools
🪛 zizmor (1.25.2)
[warning] 18-21: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 18-18: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release-please.yml around lines 18 - 21, Add the
`persist-credentials: false` parameter to the `actions/checkout@v6` step to
disable credential persistence and prevent potential credential leakage. This
should be added as a new line in the `with` section alongside the existing
`fetch-depth: 0` parameter.
Source: Linters/SAST tools
| release-type: python | ||
| token: ${{ secrets.RELEASE_PLEASE_PR_CI_TOKEN }} | ||
|
|
||
| publish: |
There was a problem hiding this comment.
The separation into a release-please job and a publish job was actually a good thing. I would suggest to keep that, while still switching to pixi for the publish job.
| @@ -0,0 +1,30 @@ | |||
| [project] | |||
There was a problem hiding this comment.
There is no need for a spearate pixi.toml. Pixi works fine with a pyproject.toml (see the docs). Pixi specific sections are just added as [tool.pixi.*].
2 participants
the situation with black and python version mismatches on local and CI environments is not bearable any longer. Hence, the switch to a CI supporting pixi, ruff and flake8
Summary by CodeRabbit
Release Notes
Chores
pixiworkflow.pixi.tomlto standardize environments and task commands.python -m buildand publish via PyPI.Bug Fixes
array_limit.Style / Tooling
throttlerdependency lower bound.