-
Notifications
You must be signed in to change notification settings - Fork 7k
[ci] install uv python in forge without project #58663
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
Conversation
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.
Code Review
This pull request updates the forge.Dockerfile to install Python using uv without creating project configuration files. This is achieved by adding the --no-project flag to uv python find and removing the uv python pin command. The changes also introduce a UV_PYTHON_VERSION variable for better maintainability and improve shell command robustness by quoting variables.
My review includes a suggestion to make the symlink creation more robust by using the -f flag, which will prevent errors if the script is run in an environment where the symlinks already exist.
| ln -s "$UV_PYTHON_BIN" "/usr/local/bin/python${UV_PYTHON_VERSION}" | ||
| ln -s "$UV_PYTHON_BIN" /usr/local/bin/python3 | ||
| ln -s "$UV_PYTHON_BIN" /usr/local/bin/python |
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.
To make the script more robust and idempotent, consider using the -f (force) flag with ln. This will ensure that if the symlinks already exist, they are overwritten without causing an error. This is particularly useful if this part of the script is re-executed.
ln -sf "$UV_PYTHON_BIN" "/usr/local/bin/python${UV_PYTHON_VERSION}"
ln -sf "$UV_PYTHON_BIN" /usr/local/bin/python3
ln -sf "$UV_PYTHON_BIN" /usr/local/bin/python
so that it does not need / create project config files we just need the self-contained binary Signed-off-by: Lonnie Liu <lonnie@anyscale.com>
9c8127c to
06f5e87
Compare
so that it does not need / create project config files we just need the self-contained binary Signed-off-by: Lonnie Liu <lonnie@anyscale.com> Signed-off-by: Justin Miller <justinrmiller@gmail.com>
so that it does not need / create project config files we just need the self-contained binary Signed-off-by: Lonnie Liu <lonnie@anyscale.com> Signed-off-by: ryankert01 <ryan980053@gmail.com>
so that it does not need / create project config files we just need the self-contained binary Signed-off-by: Lonnie Liu <lonnie@anyscale.com> Signed-off-by: Aydin Abiar <aydin@anyscale.com>
so that it does not need / create project config files we just need the self-contained binary Signed-off-by: Lonnie Liu <lonnie@anyscale.com> Signed-off-by: YK <1811651+ykdojo@users.noreply.github.com>
so that it does not need / create project config files we just need the self-contained binary Signed-off-by: Lonnie Liu <lonnie@anyscale.com>
so that it does not need / create project config files
we just need the self-contained binary