feat: add lockfiles for reproducible sandbox builds#1231
feat: add lockfiles for reproducible sandbox builds#1231gwarmstrong wants to merge 1 commit intoNVIDIA-NeMo:mainfrom
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| PATH="/root/.local/bin:${PATH}" | ||
|
|
||
| # Install uv | ||
| RUN curl -LsSf https://astral.sh/uv/install.sh | sh |
There was a problem hiding this comment.
Piping curl to shell (curl | sh) downloads and executes remote code without verification. Consider downloading the script first, verifying its checksum, then executing it for better security.
| RUN curl -LsSf https://astral.sh/uv/install.sh | sh | |
| RUN curl -LsSf https://astral.sh/uv/install.sh -o /tmp/uv-install.sh && \ | |
| sh /tmp/uv-install.sh && rm /tmp/uv-install.sh |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
… builds Add sandbox-core.lock and sandbox.lock for pinned dependency installation. Switch Dockerfile.sandbox from pip install to uv install from lockfiles. Add flask and gunicorn to code_execution.txt requirements. Signed-off-by: George Armstrong <georgea@nvidia.com>
gwarmstrong
force-pushed
the
georgea/sandbox-lockfiles
branch
from
4d02820 to
a5997c4
Compare
📝 WalkthroughWalkthroughThe Dockerfile.sandbox is refactored to replace per-file Python dependency setup with a lock-file-driven installation flow using two lock files (sandbox-core.lock and sandbox.lock), introduces uv package manager, and applies conditional logic for non-CI and non-arm64 installations. The code_execution.txt gains flask and gunicorn dependencies. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 1
🤖 Fix all issues with AI agents
In `@dockerfiles/Dockerfile.sandbox`:
- Around line 82-83: Add a pinned UV version ARG and use it in the existing uv
install RUN command: introduce an ARG named UV_VERSION (set to the uv version
used to generate the lock files) near the top of Dockerfile.sandbox and change
the existing RUN curl -LsSf https://astral.sh/uv/install.sh | sh invocation to
use https://astral.sh/uv/${UV_VERSION}/install.sh so the RUN line that installs
uv is reproducible; reference the ARG name UV_VERSION and the RUN command that
currently installs uv to locate the change.
🧹 Nitpick comments (3)
requirements/code_execution.txt (1)
18-19: Infrastructure dependencies mixed with LLM-accessible packages.The file header (line 16) describes this as "requirements for code execution — put all packages that LLM should have access to in here."
flaskandgunicornare server infrastructure dependencies, not packages an LLM would typically use during code execution. Consider adding a brief inline comment (e.g.,# server infrastructure) to distinguish them from the data-science packages, or split them into a separate requirements file that the lockfile compilation also consumes.dockerfiles/Dockerfile.sandbox (2)
101-104: Inconsistent package manager:pip install gdownvsuv pip installelsewhere.The rest of the Dockerfile now uses
uv pip install, but this line still uses barepip. Consider switching touv pip install gdownfor consistency.Proposed fix
-RUN mkdir -p /data && pip install gdown && \ +RUN mkdir -p /data && uv pip install --system gdown && \
152-153: DuplicateENV LISTEN_PORT=6000.
LISTEN_PORTis set to6000on both line 153 and line 165. One of these can be removed.Also applies to: 165-165
| # Install uv | ||
| RUN curl -LsSf https://astral.sh/uv/install.sh | sh |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
uv install script version pinning astral.sh
💡 Result:
To pin a specific uv version when using Astral’s install scripts, put the version in the URL path:
- macOS/Linux (sh):
curl -LsSf https://astral.sh/uv/0.10.0/install.sh | sh - Windows (PowerShell):
powershell -ExecutionPolicy ByPass -c "irm https://astral.sh/uv/0.10.0/install.ps1 | iex"
[1]
For Docker, pin the installer URL the same way (or pin the Docker image tag / digest):
ADD https://astral.sh/uv/0.10.0/install.sh /uv-installer.sh[2]
If you also need reproducible install behavior (path, no PATH modification, etc.), the installer supports environment variables like UV_INSTALL_DIR, UV_NO_MODIFY_PATH, and UV_UNMANAGED_INSTALL. [3]
🏁 Script executed:
# Look for any version hints in lock files or documentation
fd -type f \( -name "uv.lock" -o -name "uv.toml" -o -name "pyproject.toml" -o -name "requirements*.txt" \) | head -20Repository: NVIDIA-NeMo/Skills
Length of output: 232
🏁 Script executed:
# Check the Dockerfile context for other version pins or version references
cat -n dockerfiles/Dockerfile.sandbox | head -100Repository: NVIDIA-NeMo/Skills
Length of output: 5242
🏁 Script executed:
# Look for CI/CD files and setup configurations that may specify uv version
fd -name "*.yml" -o -name "*.yaml" -o -name ".github" | head -20Repository: NVIDIA-NeMo/Skills
Length of output: 292
🏁 Script executed:
# Search for uv version references in the entire repo
rg "uv.*[0-9]+\.[0-9]+" --max-count 10Repository: NVIDIA-NeMo/Skills
Length of output: 523
🏁 Script executed:
# Check the lock files themselves for version info
fd -name "*.lock" -type fRepository: NVIDIA-NeMo/Skills
Length of output: 292
🏁 Script executed:
# Find lock files and CI files
git ls-files | grep -E "(\.lock|\.yml|\.yaml|pyproject\.toml)" | head -30Repository: NVIDIA-NeMo/Skills
Length of output: 1318
🏁 Script executed:
# Search for uv version references more carefully
rg "uv" --type py --type txt --type toml | grep -i "version\|0\.[0-9]" | head -20Repository: NVIDIA-NeMo/Skills
Length of output: 374
🏁 Script executed:
# Check if there's a pyproject.toml with uv config
fd "pyproject.toml" -type fRepository: NVIDIA-NeMo/Skills
Length of output: 232
🏁 Script executed:
# Find lock files more simply
find . -name "*.lock" -type f 2>/dev/null | head -20Repository: NVIDIA-NeMo/Skills
Length of output: 122
🏁 Script executed:
# Check the GitHub workflow files for uv version pins
cat .github/workflows/tests.yml | head -100Repository: NVIDIA-NeMo/Skills
Length of output: 3631
🏁 Script executed:
# Look for any mentions of uv version in the codebase
rg "astral.*uv" --max-count 10Repository: NVIDIA-NeMo/Skills
Length of output: 464
🏁 Script executed:
# Check Dockerfile.nemo-rl to see what UV_VERSION is set to
cat -n dockerfiles/Dockerfile.nemo-rl | grep -A 5 -B 5 "UV_VERSION"Repository: NVIDIA-NeMo/Skills
Length of output: 576
🏁 Script executed:
# Look for ARG UV_VERSION or ENV UV_VERSION definitions
rg "UV_VERSION" -B 2 -A 2Repository: NVIDIA-NeMo/Skills
Length of output: 525
Pin the uv version for reproducible builds.
The whole point of this PR is reproducible sandbox builds, but uv itself is installed without a version pin. Future uv releases could change behavior or introduce incompatibilities. Pin the version in the install URL using an ARG (following the pattern already established in Dockerfile.nemo-rl).
Dockerfile.nemo-rl correctly pins this with:
ARG UV_VERSION=0.9.7
RUN curl -LsSf https://astral.sh/uv/${UV_VERSION}/install.sh | shApply the same pattern to Dockerfile.sandbox, using the uv version that was used to generate the lock files.
🤖 Prompt for AI Agents
In `@dockerfiles/Dockerfile.sandbox` around lines 82 - 83, Add a pinned UV version
ARG and use it in the existing uv install RUN command: introduce an ARG named
UV_VERSION (set to the uv version used to generate the lock files) near the top
of Dockerfile.sandbox and change the existing RUN curl -LsSf
https://astral.sh/uv/install.sh | sh invocation to use
https://astral.sh/uv/${UV_VERSION}/install.sh so the RUN line that installs uv
is reproducible; reference the ARG name UV_VERSION and the RUN command that
currently installs uv to locate the change.
|
Recreating from origin branch to fix CI auth |
1 participant
Summary
requirements/sandbox-core.lockandrequirements/sandbox.lockfor pinned sandbox dependency installationDockerfile.sandboxfrompip installtouv pip installfrom lockfiles for reproducible buildsflaskandgunicorntorequirements/code_execution.txtSplit from #1226 — this PR handles lockfile infrastructure only; gunicorn migration follows in a separate PR.
Summary by CodeRabbit