-
Notifications
You must be signed in to change notification settings - Fork 75
adds integration for tox-uv and updates workflows to use tox-uv #676
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: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes migrate build and test workflows from pip and Python venv to uv for package management and virtual environment creation. This includes updating GitHub Actions workflows, dependency constraints, and tox configuration to use uv commands and the tox-uv plugin instead of tox-current-env. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
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: 3
🧹 Nitpick comments (1)
tox.ini (1)
30-37: Consider consistent indentation style.The
passenvsection uses tab indentation (lines 34-37), while other parts of the file may use spaces. While tox.ini can handle both, consistent indentation improves readability and prevents potential parsing issues.🔎 Check indentation consistency across the file
#!/bin/bash # Check for mixed tab/space indentation in tox.ini echo "=== Lines with tabs ===" grep -n $'\t' tox.ini | head -20 echo -e "\n=== Lines with leading spaces ===" grep -n '^[[:space:]]' tox.ini | grep -v $'\t' | head -20
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/actions/run-smoke/action.yml(3 hunks).github/workflows/lint.yml(1 hunks).github/workflows/unit.yaml(1 hunks)constraints-dev.txt(1 hunks)requirements-dev.txt(1 hunks)tox.ini(2 hunks)
⏰ 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: Summary
🔇 Additional comments (15)
.github/workflows/unit.yaml (2)
74-74: LGTM: UV venv creation.The migration from
python -m venv --upgrade-depstouv venvis correct. UV manages its own dependency resolution, so the--upgrade-depsflag is no longer needed.
76-76: LGTM: UV-based package installation.The installation command correctly uses
uv pip installand includes bothtoxandtox-uvwith the constraints file..github/workflows/lint.yml (2)
80-82: LGTM: UV-based environment setup.The migration to UV for virtual environment creation and package installation is correctly implemented.
86-86: LGTM: Virtual environment activation added.Adding the explicit venv activation ensures that the lint commands run in the correct environment.
tox.ini (5)
4-6: LGTM: Tox plugin requirements added.The
requiressection correctly declares the tox-uv plugin dependency, ensuring it's available when tox runs.
21-23: LGTM: Install command migrated to UV.The
install_commandcorrectly usesuv pip installwith the constraints file, maintaining the existing line continuation format.
24-25: LGTM: PATH added to passenv.Adding
PATHtopassenvensures UV can locate system binaries and Python installations correctly.
52-58: LGTM: Smoke test environment configuration updated.The
basepythonandPATHadditions are consistent with the unit test environment configuration and support the UV-based workflow.
15-17: Both tox-uv configuration options are correct and valid.The
uv-venv-runneris the correct ID for the tox environments runner for environments not using a lock file. Theuv_seedoption controls whether pip, setuptools, and wheel are injected into the virtual environment. Both settings are properly configured for standard uv-based venv management without lock files..github/actions/run-smoke/action.yml (4)
35-37: LGTM: UV-based venv creation with Python version.The venv creation correctly uses UV with the
--pythonflag to specify the Python version from inputs, and the package installation usesuv pip install.
49-49: LGTM: Torch dependencies installation migrated to UV.The torch and build dependencies installation correctly uses
uv pip installwith the constraints file.
69-69: LGTM: Removed --current-env flag.The
--current-envflag is correctly removed since tox-uv handles environment management differently than the previous tox-current-env plugin.
56-57: The--no-build-isolationusage on line 56 is appropriate and necessary for this workflow. As an escape hatch, you can preinstall a package's build dependencies, then run uv pip install with --no-build-isolation, which is exactly what this code does. Line 49 pre-installs torch, packaging, setuptools, wheel, and other dependencies needed by flash-attn, and the code comments (lines 39-43) explain that flash-attn requires this approach due to a bug in its setup.py. The pattern is correctly implemented and follows uv's recommended approach for handling packages with undeclared build-time dependencies.requirements-dev.txt (1)
13-14: The changes align with official tox guidance. Official tox documentation indicates that tox>=4 and tox-uv>=1 are automatically provisioned together, confirming that relaxing the tox constraint to>=4is compatible withtox-uv. Consider pinningtox-uvto a minimum version (e.g.,tox-uv>=1) for better reproducibility, though this is optional.constraints-dev.txt (1)
188-189: Pinned versions are compatible and stable.tox-uv 1.25.0 is a tox 4.x plugin compatible with tox 4.29.0, and tox 4.29.0 is a stable release from August 29, 2025. No further action needed.
| - name: "Install uv" | ||
| uses: astral-sh/setup-uv@v4 | ||
| with: | ||
| enable-cache: true |
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.
🛠️ Refactor suggestion | 🟠 Major
Pin the third-party action to a commit SHA for security.
The astral-sh/setup-uv@v4 action uses a mutable tag reference. For security best practices, pin it to a specific commit SHA to ensure immutable and auditable dependencies.
🔎 Verification script to find the latest commit SHA for v4
#!/bin/bash
# Fetch the latest commit SHA for the v4 tag of astral-sh/setup-uv
gh api repos/astral-sh/setup-uv/git/refs/tags/v4 --jq '.object.sha // .object.url' | \
xargs -I {} sh -c 'if [[ {} == https://* ]]; then gh api {} --jq .object.sha; else echo {}; fi'🤖 Prompt for AI Agents
.github/actions/run-smoke/action.yml around lines 25 to 28: the workflow is
referencing the third‑party action using a mutable tag (astral-sh/setup-uv@v4);
replace the tag with the specific commit SHA for the v4 release to pin the
action immutably. Determine the correct commit SHA for the v4 tag (for example
via the provided gh api snippet), then update the uses line to use
astral-sh/setup-uv@<commit-sha> and keep the existing with: enable-cache: true
unchanged.
| - name: Install uv | ||
| uses: astral-sh/setup-uv@v4 | ||
| with: | ||
| enable-cache: true |
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.
🛠️ Refactor suggestion | 🟠 Major
Pin the third-party action to a commit SHA for security.
The astral-sh/setup-uv@v4 action uses a mutable tag reference. For consistency with other actions in this workflow (lines 63, 69) and security best practices, pin it to a specific commit SHA.
🔎 Verification script to find the latest commit SHA for v4
#!/bin/bash
# Fetch the latest commit SHA for the v4 tag of astral-sh/setup-uv
gh api repos/astral-sh/setup-uv/git/refs/tags/v4 --jq '.object.sha // .object.url' | \
xargs -I {} sh -c 'if [[ {} == https://* ]]; then gh api {} --jq .object.sha; else echo {}; fi'🤖 Prompt for AI Agents
.github/workflows/lint.yml lines 73 to 76: the workflow references the external
action using a mutable tag `astral-sh/setup-uv@v4`; replace this with the action
pinned to the specific commit SHA for the v4 tag (e.g.,
`astral-sh/setup-uv@<commit-sha>`). Obtain the latest commit SHA for the v4 tag
(using the provided verification script or `gh`/GitHub API), then update the
`uses:` line to use that SHA so the action is immutable and consistent with
other pinned actions in the file.
| - name: "Install uv" | ||
| uses: astral-sh/setup-uv@v4 | ||
| with: | ||
| enable-cache: true |
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.
🛠️ Refactor suggestion | 🟠 Major
Pin the third-party action to a commit SHA for security.
The astral-sh/setup-uv@v4 action uses a mutable tag reference. For consistency with other actions in this workflow (lines 56, 61) and security best practices, pin it to a specific commit SHA.
🔎 Verification script to find the latest commit SHA for v4
#!/bin/bash
# Fetch the latest commit SHA for the v4 tag of astral-sh/setup-uv
gh api repos/astral-sh/setup-uv/git/refs/tags/v4 --jq '.object.sha // .object.url' | \
xargs -I {} sh -c 'if [[ {} == https://* ]]; then gh api {} --jq .object.sha; else echo {}; fi'🤖 Prompt for AI Agents
.github/workflows/unit.yaml lines 65-68: the workflow pins astral-sh/setup-uv to
the mutable tag v4; replace the tag with the specific commit SHA (e.g.,
astral-sh/setup-uv@<commit-sha>) to avoid mutable references — run the provided
gh script to fetch the current v4 commit SHA, update the uses field to that SHA,
and ensure formatting matches the surrounding actions pinned earlier in the
file.
The existing codebase uses
toxto handle various scripting workflows under-the-hood, but this leads to slow CI runs and other things.But
toxuses the native python system instead ofuveven when a user is usinguv. This PR changes the behavior so tox usesuvunder the hood.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.