RHAIENG-2189: configure PIP_INDEX_URL
#2760
Open
+545
−204
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Skipping CI for Draft Pull Request. |
github-actions
bot
added
the
review-requested
Contributor
WalkthroughAdds RHAIENG-2189 BEGIN/END ENV blocks to many Dockerfiles introducing PIP_INDEX_URL, UV_INDEX_URL (deprecated comment) and UV_DEFAULT_INDEX; adds an ntb package (constants + string utilities), refactors scripts/dockerfile_fragments.py to a data-driven workflow with sanity_check, updates CI/workflows to setup-uv@v7 and uv invocations, and adjusts tests to assert the new env vars. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
jiridanek
added
the
tide/merge-method-rebase
openshift-ci
bot
added
size/xl
and removed
do-not-merge/work-in-progress
size/xl
labels
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build-notebooks-TEMPLATE.yaml(2 hunks)jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda(2 hunks)jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm
⏰ 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). (47)
- GitHub Check: Red Hat Konflux / odh-pipeline-runtime-datascience-cpu-py312-ubi9-on-pull-request
- GitHub Check: Red Hat Konflux / odh-workbench-jupyter-pytorch-llmcompressor-cuda-py312-ubi9-on-pull-request
- GitHub Check: Red Hat Konflux / odh-workbench-jupyter-trustyai-cpu-py312-ubi9-on-pull-request
- GitHub Check: Red Hat Konflux / odh-workbench-rstudio-minimal-cpu-py312-rhel9-on-pull-request
- GitHub Check: Red Hat Konflux / odh-workbench-jupyter-tensorflow-rocm-py312-ubi9-on-pull-request
- GitHub Check: Red Hat Konflux / odh-workbench-jupyter-minimal-cpu-py312-ubi9-on-pull-request
- GitHub Check: Red Hat Konflux / odh-pipeline-runtime-pytorch-cuda-py312-ubi9-on-pull-request
- GitHub Check: Red Hat Konflux / odh-workbench-codeserver-datascience-cpu-py312-ubi9-on-pull-request
- GitHub Check: Red Hat Konflux / odh-workbench-rstudio-minimal-cuda-py312-c9s-on-pull-request
- GitHub Check: Red Hat Konflux / odh-pipeline-runtime-tensorflow-rocm-py312-ubi9-on-pull-request
- GitHub Check: Red Hat Konflux / odh-workbench-jupyter-pytorch-cuda-py312-ubi9-on-pull-request
- GitHub Check: Red Hat Konflux / odh-workbench-jupyter-datascience-cpu-py312-ubi9-on-pull-request
- GitHub Check: Red Hat Konflux / odh-workbench-jupyter-tensorflow-cuda-py312-ubi9-on-pull-request
- GitHub Check: Red Hat Konflux / odh-pipeline-runtime-minimal-cpu-py312-ubi9-on-pull-request
- GitHub Check: Red Hat Konflux / odh-workbench-rstudio-minimal-cpu-py312-c9s-on-pull-request
- GitHub Check: Red Hat Konflux / odh-pipeline-runtime-pytorch-llmcompressor-cuda-py312-ubi9-on-pull-request
- GitHub Check: Red Hat Konflux / odh-pipeline-runtime-tensorflow-cuda-py312-ubi9-on-pull-request
- GitHub Check: Red Hat Konflux / odh-workbench-rstudio-minimal-cuda-py312-rhel9-on-pull-request
- GitHub Check: build (runtime-datascience-ubi9-python-3.12, 3.12, linux/s390x, false) / build
- GitHub Check: build (jupyter-datascience-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (rocm-jupyter-minimal-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (cuda-rstudio-c9s-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (rocm-jupyter-pytorch-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (cuda-jupyter-tensorflow-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (runtime-cuda-tensorflow-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (runtime-cuda-pytorch-llmcompressor-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (cuda-jupyter-pytorch-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (cuda-jupyter-minimal-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (jupyter-minimal-ubi9-python-3.12, 3.12, linux/s390x, false) / build
- GitHub Check: build (rocm-runtime-pytorch-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (runtime-cuda-tensorflow-ubi9-python-3.12, 3.12, linux/arm64, false) / build
- GitHub Check: build (jupyter-minimal-ubi9-python-3.12, 3.12, linux/ppc64le, false) / build
- GitHub Check: build (rstudio-c9s-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (rocm-runtime-tensorflow-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (jupyter-minimal-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (rocm-jupyter-tensorflow-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (runtime-datascience-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (runtime-cuda-pytorch-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (cuda-jupyter-tensorflow-ubi9-python-3.12, 3.12, linux/arm64, false) / build
- GitHub Check: build (runtime-minimal-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (runtime-minimal-ubi9-python-3.12, 3.12, linux/s390x, false) / build
- GitHub Check: build (codeserver-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (cuda-jupyter-minimal-ubi9-python-3.12, 3.12, linux/arm64, false) / build
- GitHub Check: build (cuda-jupyter-pytorch-llmcompressor-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (codeserver-ubi9-python-3.12, 3.12, linux/arm64, false) / build
- GitHub Check: build (jupyter-trustyai-ubi9-python-3.12, 3.12, linux/amd64, false) / build
- GitHub Check: build (jupyter-datascience-ubi9-python-3.12, 3.12, linux/ppc64le, false) / build
🔇 Additional comments (3)
.github/workflows/build-notebooks-TEMPLATE.yaml (2)
284-287: Clarify the intent of setting and then unsetting CFLAGS/CXXFLAGS.Lines 285 set
--env=CFLAGS=-Dundefined=64 --env=CXXFLAGS=-Dundefined=64but then immediately unset both variables with--unsetenv=CFLAGS --unsetenv=CXXFLAGS. This appears contradictory—the variables are set only to be removed.Can you clarify whether this is intentional (e.g., setting them for a specific build phase and then clearing them)? If so, consider adding a comment explaining the intended behavior. If unintentional, the unsetenv flags should likely be removed.
329-334: Verify astral-sh/setup-uv@v7 self-hosted runner compatibility.The v7 upgrade includes a runtime change from Node 20 to Node 24. If this workflow runs on self-hosted runners, ensure they support Node 24. The workflow's cache configuration using
enable-cache: trueandcache-dependency-glob: "uv.lock"is compatible with v7 and requires no changes.Other v7 breaking changes (removal of deprecated
server-urlinput) do not affect this configuration.jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda (1)
62-63: Consider alignment:--extra-index-url(secondary) vs.PIP_INDEX_URL(primary override).The pip install here uses
--extra-index-url https://pypi.org/simpleto add PyPI as a fallback index. In contrast, lines 144–150 setPIP_INDEX_URLto override the primary index. These are two different strategies. Confirm this is intentional and that the early pip install doesn't need the primary-index override to align with the PR's global migration objective.
Comment on lines
+144
to
+150
| ### BEGIN RHAIENG-2189: this is AIPCC migration phase 1.5 | ||
| ENV PIP_INDEX_URL=https://pypi.org/simple | ||
| # UV_INDEX_URL is deprecated in favor of UV_DEFAULT_INDEX | ||
| ENV UV_INDEX_URL=https://pypi.org/simple | ||
| # https://docs.astral.sh/uv/reference/environment/#uv_default_index | ||
| ENV UV_DEFAULT_INDEX=https://pypi.org/simple | ||
| ### END RHAIENG-2189: this is AIPCC migration phase 1.5 |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Get file size first
wc -l jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda
echo "=== Build stages (FROM ... AS statements) ==="
rg -n "^FROM.*AS" jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda
echo -e "\n=== All pip/uv invocations with context ==="
rg -n -B 2 "(pip install|uv pip)" jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda | head -100
echo -e "\n=== ENV var definitions (PIP_INDEX_URL, UV_INDEX_URL, UV_DEFAULT_INDEX) ==="
rg -n "ENV (PIP_INDEX_URL|UV_INDEX_URL|UV_DEFAULT_INDEX)" jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda
echo -e "\n=== Context around lines 62-63 (first pip install) ==="
sed -n '55,75p' jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda | cat -n
echo -e "\n=== Context around lines 144-150 (env vars) ==="
sed -n '135,155p' jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda | cat -nRepository: opendatahub-io/notebooks
Length of output: 154
🏁 Script executed:
#!/bin/bash
# First, verify the file exists and check size
echo "=== Checking file ==="
if [ -f "jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda" ]; then
wc -l "jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda"
ls -lh "jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda"
else
echo "File not found. Searching for similar Dockerfiles..."
find . -name "*pytorch*" -type f 2>/dev/null | head -20
fi
echo -e "\n=== Looking for FROM statements (stages) ==="
grep -n "^FROM" "jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda" 2>/dev/null || echo "File not accessible"
echo -e "\n=== Looking for ENV statements with PIP_INDEX_URL or UV ==="
grep -n "ENV.*PIP_INDEX_URL\|ENV.*UV_INDEX_URL\|ENV.*UV_DEFAULT_INDEX" "jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda" 2>/dev/null || echo "No matches found"
echo -e "\n=== Lines 55-75 (around line 62-63) ==="
sed -n '55,75p' "jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda" 2>/dev/null | cat -n
echo -e "\n=== Lines 135-155 (around line 144-150) ==="
sed -n '135,155p' "jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda" 2>/dev/null | cat -nRepository: opendatahub-io/notebooks
Length of output: 2979
🏁 Script executed:
#!/bin/bash
echo "=== All pip and uv invocations by line number ==="
grep -n "pip\|uv" jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda
echo -e "\n=== Check which stage each pip/uv invocation is in ==="
# Show stages and track which one we're in
awk '
/^FROM.*AS/ {
current_stage = $NF
stage_line = NR
print "Line " NR ": Stage " current_stage " begins"
}
/pip|uv/ && !/^#/ {
print "Line " NR ": " $0 " [in stage: " current_stage "]"
}
' jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cudaRepository: opendatahub-io/notebooks
Length of output: 1880
🏁 Script executed:
#!/bin/bash
echo "=== Context around line 63 (pip install in cuda-base) ==="
sed -n '60,70p' jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda | cat -n
echo -e "\n=== Context around line 170 (uv pip install in cuda-jupyter-pytorch) ==="
sed -n '165,180p' jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda | cat -n
echo -e "\n=== Check ENV var order in cuda-jupyter-pytorch ==="
sed -n '137,175p' jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda | cat -nRepository: opendatahub-io/notebooks
Length of output: 4696
ENV var placement is correct for the main package installation, but confirm if line 63's explicit flag should be aligned.
The ENV variables are appropriately positioned before the actual package installation at line 170 (uv pip install), which will inherit UV_DEFAULT_INDEX. However, there is an inconsistency in approach: line 63 uses an explicit --extra-index-url flag to install the uv tool itself in the cuda-base stage, while lines 145–149 define ENV variables for the cuda-jupyter-pytorch stage.
Since line 63 installs only the tools (micropipenv and uv), not the application packages, the --extra-index-url fallback may be intentional. The actual package installation at line 170 will correctly use the UV_DEFAULT_INDEX environment variable. Confirm whether this two-method approach is acceptable or if consistency is preferred (either use ENV vars everywhere or explicit flags everywhere).
🤖 Prompt for AI Agents
In jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda around lines
63 and 144-150, the review notes an inconsistency: line 63 installs uv with an
explicit --extra-index-url flag while lines 144-150 set
UV_DEFAULT_INDEX/UV_INDEX_URL ENV vars for later installs; decide and apply one
consistent approach across stages — either replace the explicit flag at line 63
with usage of the same ENV (set earlier or exported into the cuda-base stage) so
all installs inherit UV_DEFAULT_INDEX, or keep the explicit flag and add a
clarifying comment explaining why tooling installation must use the flag; update
the Dockerfile accordingly (move/export ENV into the base stage if using ENV, or
add comment and keep flag if intentional).
Contributor
|
@jiridanek: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: atheo89 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
… execution in pytest (opendatahub-io#2760)
…s to be run with uv run (opendatahub-io#2760)
There is a Jira from the past about this * https://issues.redhat.com/browse/RHOAIENG-28824 * https://issues.redhat.com/browse/RHAIENG-1284 ``` SUBFAILED[dlib='/opt/app-root/lib64/python3.12/site-packages/mysql/vendor/private/sasl2/libplain.so.3.0.0'] tests/containers/base_image_test.py::TestBaseImage::test_elf_files_can_link_runtime_libs[ghcr.io/opendatahub-io/notebooks/workbench-images:rocm-jupyter-tensorflow-ubi9-python-3.12-_8ac1b04d1f22ec3c5bc4a50a95176a522bee132d] - Failed: dlib='/opt/app-root/lib64/python3.12/site-packages/mysql/vendor/private/sasl2/libplain.so.3.0.0' has unsatisfied dependencies deps='libcrypt.so.1 => not found' ```
…ilation failure
```
/dev/mapper/buildvg-buildlv 68G 14G 52G 21% /home/runner/.local/share/containers
× Failed to build `pyzmq==27.1.0`
├─▶ The build backend returned an error
╰─▶ Call to `scikit_build_core.build.build_wheel` failed (exit status: 1)
...
/tmp/tmpy8t962b7/build/_deps/bundled_libzmq-build/platform.hpp:22:28:
error: ‘undefined’ was not declared in this scope
22 | #define ZMQ_CACHELINE_SIZE undefined
| ^~~~~~~~~
```
Contributor
|
New changes are detected. LGTM label has been removed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://issues.redhat.com/browse/RHAIENG-2189
Description
This PR configures
PIP_INDEX_URL,UV_INDEX_URL, andUV_DEFAULT_INDEXenvironment variables across notebook images to use PyPI (https://pypi.org/simple) as the primary package index, overriding the AIPCC RedHat Python index. This change represents AIPCC migration phase 1.5, ensuring that both pip and uv package managers use the public PyPI repository directly as their primary source when resolving dependencies.The configuration is applied consistently across all Python 3.12 UBI9-based images:
The environment variables are set using managed Dockerfile fragments with
### BEGIN/ENDmarkers for maintainability and consistency.Side note: This PR includes preparatory infrastructure work in the first three commits:
ntb/) for common utilitiesThese commits establish the
scripts/dockerfile_fragments.pyinfrastructure (inspired by AIPCC'sreplace-markers.sh) that manages reusable Dockerfile fragments across the repository, making the PIP_INDEX_URL configuration changes more maintainable and consistent.How Has This Been Tested?
PIP_INDEX_URL,UV_INDEX_URL,UV_DEFAULT_INDEX) are properly set in the built imagesmake testto ensure all tests passHow Has This Been Tested?
rerun after adding few more fixes to the PR
looks reasonable; except some PRs need to go first to fix ci around this
Self checklist (all need to be checked):
make test(gmakeon macOS) before asking for reviewDockerfile.konfluxfiles should be done inodh/notebooksand automatically synced torhds/notebooks. For Konflux-specific changes, modifyDockerfile.konfluxfiles directly inrhds/notebooksas these require special attention in the downstream repository and flow to the upcoming RHOAI release.Merge criteria:
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.