[rhds] RHAIENG-304, RHAIENG-784: switch from Pipfile.lock to pylock.toml for package management across all relevant Dockerfiles
#1508
Conversation
…ackage management across all relevant Dockerfiles This propagates to rhds the changes done in * opendatahub-io#2145
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
jiridanek
changed the title
Pipfile.lock to pylock.toml for package management across all relevant DockerfilesPipfile.lock to pylock.toml for package management across all relevant Dockerfiles
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Review Summary
This pull request consistently updates 16 Dockerfiles to use uv and pylock.toml for Python package management, replacing the previous micropipenv and Pipfile.lock setup. The changes are straightforward and align with the goal of modernizing the dependency management process.
🔍 General Feedback
- The transition to
uvis well-implemented across all relevant files. - I've noticed that in several files, comments still refer to
requirements.txtorPipfile.lockeven though the code now usespylock.toml. I've left a couple of inline comments as examples. Please apply the same fix to all other occurrences for consistency.
Overall, this is a good improvement, and with the minor comment fixes, it will be ready for merging.
| # Install useful packages from Pipfile.lock | ||
| COPY ${CODESERVER_SOURCE_CODE}/Pipfile.lock ./ | ||
| # Install useful packages from requirements.txt | ||
| COPY ${CODESERVER_SOURCE_CODE}/pylock.toml ./ | ||
|
|
There was a problem hiding this comment.
🟢 For consistency, the comment should refer to pylock.toml instead of requirements.txt, as that is the file being used for installation.
| # Install useful packages from pylock.toml |
There was a problem hiding this comment.
Already tracked for subsequent fixing in
| # This may have to download and compile some dependencies, and as we don't lock requirements from `build-system.requires`, | ||
| # we often don't know the correct hashes and `--require-hashes` would therefore fail on non amd64, where building is common. |
There was a problem hiding this comment.
🟢 This comment refers to requirements.txt, but the file has been changed to pylock.toml. Please update the comment for consistency.
| # This may have to download and compile some dependencies, and as we don't lock requirements from `build-system.requires`, | |
| # we often don't know the correct hashes and `--require-hashes` would therefore fail on non amd64, where building is common. | |
| # Not using --build-constraints=./pylock.toml because error: Unnamed requirements are not allowed as constraints (found: `https://repo.radeon.com/rocm/manylinux/rocm-rel-6.4/ |
There was a problem hiding this comment.
Already tracked for subsequent fixing in
|
@jiridanek: The following test 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. |
|
/lgtm |
https://issues.redhat.com/browse/RHAIENG-784
This propagates to rhds the changes done in