ci: streamline Python installation using uv and system Python #6931
Conversation
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR replaces pyenv with uv (a modern Python package installer written in Rust) for Python environment management in the CI Docker image. The change significantly improves build performance by downloading pre-compiled Python binaries instead of compiling from source. The implementation uses UV's official Docker image as a base for the binaries, installs Python 3.9.18 via uv python install, and manually constructs the PATH to the installed interpreter. All package installations now use uv pip install --system --break-system-packages, which bypasses virtual environment creation and installs directly to the system Python. This aligns with the CI container's single-purpose design where environment isolation isn't needed, and matches the previous behavior of pip installations in the pyenv-based setup.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
| contrib/containers/ci/ci-slim.Dockerfile | 3/5 | Replaces pyenv-based Python setup with uv, using COPY from official image, manual PATH construction, and system-wide package installation flags |
Confidence score: 3/5
- This PR introduces a moderately risky change to CI infrastructure with potential cross-platform compatibility issues
- Score reflects concerns about hardcoded architecture-specific paths (line 64), runtime PATH construction that may fail silently, untested cross-architecture behavior (x86_64vs aarch64), and the implicit trust placed in external Docker image (ghcr.io/astral-sh/uv:latest) without version pinning
- Pay close attention to the PATH construction on line 64 and verify the Dockerfile works across all target architectures; test the uv installation thoroughly in CI before merging
Context used:
- Context from
dashboard- CLAUDE.md (source)
1 file reviewed, 3 comments
| && rm -rf /var/lib/apt/lists/* | ||
|
|
||
| # Install uv by copying from the official Docker image | ||
| COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/ |
There was a problem hiding this comment.
style: using :latest tag creates non-deterministic builds - consider pinning to a specific version like ghcr.io/astral-sh/uv:0.5.18
Prompt To Fix With AI
This is a comment left during a code review.
Path: contrib/containers/ci/ci-slim.Dockerfile
Line: 58:58
Comment:
**style:** using `:latest` tag creates non-deterministic builds - consider pinning to a specific version like `ghcr.io/astral-sh/uv:0.5.18`
How can I resolve this? If you propose a fix, please make it concise.| && pyenv global ${PYTHON_VERSION} \ | ||
| && pyenv rehash | ||
| RUN uv python install ${PYTHON_VERSION} | ||
| ENV PATH="/root/.local/share/uv/python/cpython-${PYTHON_VERSION}-linux-$(uname -m)-gnu/bin:${PATH}" |
There was a problem hiding this comment.
logic: $(uname -m) evaluates at build time - this will fail for multi-arch builds if the build platform differs from target platform. Will this Dockerfile be used for cross-platform builds (e.g., building arm64images on x86_64)?
Prompt To Fix With AI
This is a comment left during a code review.
Path: contrib/containers/ci/ci-slim.Dockerfile
Line: 64:64
Comment:
**logic:** `$(uname -m)` evaluates at build time - this will fail for multi-arch builds if the build platform differs from target platform. Will this Dockerfile be used for cross-platform builds (e.g., building arm64images on x86_64)?
How can I resolve this? If you propose a fix, please make it concise.| # Install Python packages | ||
| RUN set -ex; \ | ||
| pip3 install --no-cache-dir \ | ||
| RUN uv pip install --system --break-system-packages \ |
There was a problem hiding this comment.
style: --break-system-packages and --system are redundant - --system alone suffices when UV_SYSTEM_PYTHON=1 is set
Prompt To Fix With AI
This is a comment left during a code review.
Path: contrib/containers/ci/ci-slim.Dockerfile
Line: 70:70
Comment:
**style:** `--break-system-packages` and `--system` are redundant - `--system` alone suffices when UV_SYSTEM_PYTHON=1 is set
How can I resolve this? If you propose a fix, please make it concise.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe CI slim Dockerfile replaces the pyenv-based Python build with uv-managed system Python by copying the Sequence Diagram(s)sequenceDiagram
actor Dockerfile
participant UVImage as uv image
participant FS as Container FS
Dockerfile->>UVImage: COPY /usr/local/bin/uv
Dockerfile->>FS: ENV UV_SYSTEM_PYTHON=1
Dockerfile->>FS: Update PATH to include uv layout
Dockerfile->>FS: run: uv python install <version>
Note over FS: uv installs system Python and pip\nunder uv-managed layout
Dockerfile->>FS: run: uv pip install --system --break-system-packages <packages>
Dockerfile->>FS: run: uv pip install --system --break-system-packages dash_hash
Dockerfile->>FS: run: symlink uv-managed package executables -> /usr/local/bin
Note over Dockerfile,FS: Removed pyenv build steps and several apt build-deps
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
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 |
![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 (1)
contrib/containers/ci/ci-slim.Dockerfile (1)
57-58: Add uv binary source verification to ensure future maintainability.The uv binary is copied from the official Docker image. Verify that
ghcr.io/astral-sh/uv:latestis the correct and most secure source for your deployment. Consider pinning to a specific version tag (e.g.,ghcr.io/astral-sh/uv:0.x.y) to ensure reproducible builds and prevent unexpected breaking changes from newer uv versions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
contrib/containers/ci/ci-slim.Dockerfile(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
contrib/**
📄 CodeRabbit inference engine (CLAUDE.md)
Unless specifically prompted, avoid making changes to the contrib directory (contributed scripts)
Files:
contrib/containers/ci/ci-slim.Dockerfile
⏰ 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: Build container / Build container
🔇 Additional comments (3)
contrib/containers/ci/ci-slim.Dockerfile (3)
63-64: Verify Python installation is accessible to the dash user.Python is installed to
/root/.local/share/uv/python/...while running as root (line 63), and later the Dockerfile switches to thedashuser (line 117). Ensure that the dash user has sufficient permissions (read and execute) to access and run the installed Python binary. This typically works in Docker due to executable world permissions, but should be explicitly verified, especially if permission-sensitive operations occur.Additionally, note that line 64 uses
$(uname -m)for architecture detection at build time. This works for standard builds but could be problematic in cross-platform build scenarios (e.g., building ARM images on x86 hardware with QEMU emulation).
70-78: Consistent replacement of pip with uv pip install.The use of
uv pip install --system --break-system-packagesis consistent with the uv-based approach. The--break-system-packagesflag is necessary and expected in a Docker container context to override PEP 668 protections.
85-85: Consistent uv pip usage for dash_hash installation.The dash_hash installation correctly uses the same
uv pip install --system --break-system-packagespattern as the main Python package installation, ensuring consistency across the build process.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
contrib/containers/ci/ci-slim.Dockerfile (2)
54-54: Pin the uv image to a specific version.Using
:latestcreates non-deterministic builds across different CI runs and environments. This has potential reliability and reproducibility impact for CI infrastructure.Apply this diff:
-COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/ +COPY --from=ghcr.io/astral-sh/uv:0.5.18 /uv /uvx /bin/(Adjust the version to match the latest stable release of uv at the time of merge.)
75-75: Simplify redundant uv pip flags.Both
--systemand--break-system-packagesare used together, but whenUV_SYSTEM_PYTHON=1is set (line 72),--systemalone is sufficient. Using both is redundant and creates maintenance noise.Apply this diff to both lines 75 and 90:
-RUN uv pip install --system --break-system-packages \ +RUN uv pip install --system \And for line 90:
- cd dash_hash && uv pip install --system --break-system-packages -r requirements.txt .; \ + cd dash_hash && uv pip install --system -r requirements.txt .; \Also applies to: 90-90
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
contrib/containers/ci/ci-slim.Dockerfile(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
contrib/**
📄 CodeRabbit inference engine (CLAUDE.md)
Unless specifically prompted, avoid making changes to the contrib directory (contributed scripts)
Files:
contrib/containers/ci/ci-slim.Dockerfile
⏰ 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: Build container / Build container
🔇 Additional comments (1)
contrib/containers/ci/ci-slim.Dockerfile (1)
93-104: Symlink loop logic is correct, but exclusion list completeness is use-case dependent.The directory skip check (
[ -f "$exe" ]) is correctly implemented and prevents symlink creation for directories. However, the Python bin directory typically contains helper scripts like pydoc, idle, and 2to3, which are not in the current exclusion list (python, python3, pip, pip3). Whether these utilities should be symlinked depends on the intended role of this container image. The-fflag inln -sfwill silently overwrite existing entries in/usr/local/bin, which is typically acceptable in a Docker build context but worth confirming aligns with your expectations.
| ENV UV_PYTHON_INSTALL_DIR=/usr/local/python | ||
| RUN uv python install ${PYTHON_VERSION} | ||
|
|
||
| # Create symlinks to make python available system-wide | ||
| RUN set -ex; \ | ||
| curl https://pyenv.run | bash \ | ||
| && pyenv update \ | ||
| && pyenv install ${PYTHON_VERSION} \ | ||
| && pyenv global ${PYTHON_VERSION} \ | ||
| && pyenv rehash | ||
| PYTHON_PATH=$(uv python find ${PYTHON_VERSION}); \ | ||
| PYTHON_DIR=$(dirname $PYTHON_PATH); \ | ||
| ln -sf $PYTHON_DIR/python3 /usr/local/bin/python3; \ | ||
| ln -sf $PYTHON_DIR/python3 /usr/local/bin/python; \ | ||
| ln -sf $PYTHON_DIR/pip3 /usr/local/bin/pip3 2>/dev/null || true; \ | ||
| ln -sf $PYTHON_DIR/pip /usr/local/bin/pip 2>/dev/null || true | ||
|
|
||
| # Use system Python for installations | ||
| ENV UV_SYSTEM_PYTHON=1 |
There was a problem hiding this comment.
🧩 Analysis chain
Verify Python installation path and symlink creation.
The UV_PYTHON_INSTALL_DIR is set to /usr/local/python, and the symlinks are created by invoking uv python find ${PYTHON_VERSION} twice (lines 64 and 95). Ensure this is deterministic and works consistently across builds.
Run the following verification script to confirm the Python symlinks are created correctly:
🏁 Script executed:
#!/bin/bash
# Description: Verify Python symlinks are created and accessible in the final image
# Search for Python-related symlinks and their targets
fd -L python /usr/local/bin -type f -o -type lLength of output: 170
I need to examine the Dockerfile itself to verify the logic, since this is a static file review, not a runtime verification. Let me correct the approach:
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the Dockerfile and display the relevant section with context
fd -t f "ci-slim.Dockerfile" --exec cat {} \; | head -100
Length of output: 3387
🏁 Script executed:
#!/bin/bash
# Display the full Dockerfile to understand the complete context
fd -t f "ci-slim.Dockerfile"Length of output: 40
🏁 Script executed:
#!/bin/bash
# Search for all Python installation steps in the Dockerfile
fd -t f "ci-slim.Dockerfile" --exec rg -n "uv python|python3|ln -sf" {} +Length of output: 1077
---
🌐 Web query:
```web
uv Python find command deterministic reliable consistent
💡 Result:
Short answer: uv python find is generally deterministic but not always — it follows explicit discovery rules (virtualenvs first, then managed installs, then PATH/system interpreters) and provides flags to make selection deterministic (for example --system or --only-installed). However, platform/package-manager quirks (Homebrew, certain Debian pip installs, non-executable script locations) can change what gets discovered, so you should use the discovery flags and explicit constraints (e.g. 'uv python find ">=3.11"' or --system) to get consistent results across environments. [1][2][3]
Key references:
- Discovery rules, venv precedence, flags and examples. [1]
- Report: --system can still pick Homebrew Python in some setups (explains one non-deterministic case). [2]
- Report: pip install / scripts-location issues on Debian that can make uv miss an installed Python. [3]
Ensure deterministic Python discovery with explicit flags.
The code calls uv python find ${PYTHON_VERSION} twice (lines 64 and 95) expecting identical results, but according to uv's discovery behavior, this is not guaranteed deterministic without explicit flags. Platform/package-manager quirks (Homebrew, Debian pip installs) can cause different discovery results.
Additionally, ENV UV_SYSTEM_PYTHON=1 is set at line 72—after the first uv python find call at line 64—which may create inconsistency between the two invocations.
Use uv python find --only-installed ${PYTHON_VERSION} (or --system if appropriate) to ensure consistent, deterministic Python discovery across all build invocations:
- Line 64: change to
PYTHON_PATH=$(uv python find --only-installed ${PYTHON_VERSION}) - Line 95: change to
PYTHON_PATH=$(uv python find --only-installed ${PYTHON_VERSION})
🤖 Prompt for AI Agents
In contrib/containers/ci/ci-slim.Dockerfile around lines 59–72, the uv python
discovery is non-deterministic and UV_SYSTEM_PYTHON is set after the first
discovery; update both uv python find invocations to use an explicit
deterministic flag (e.g. --only-installed or --system as appropriate) — change
PYTHON_PATH=$(uv python find ${PYTHON_VERSION}) to PYTHON_PATH=$(uv python find
--only-installed ${PYTHON_VERSION}) in both places mentioned in the review, and
move the ENV UV_SYSTEM_PYTHON=1 line to before the first uv python find if you
intend to rely on the system flag instead of --only-installed so both
discoveries run with identical environment/flags.
3 participants
Issue being fixed or feature implemented
Use
uvinstead ofpyenvfor python installation and depends.Before
After
switching to UV cuts the shown portion of the CI build by ~64 seconds, ~75% overall faster, with huge wins in Python setup and package installs; the dash_hash step is slightly slower but dwarfed by the other gains.
Breaking Changes
None
Checklist:
Go over all the following points, and put an
xin all the boxes that apply.