feat: [CI] Introduce uv to accelerate pip install#9169
feat: [CI] Introduce uv to accelerate pip install#9169Zherphy wants to merge 1 commit intomodelscope:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the CI environment to use uv for faster package installations and updates the Docker CI scripts to pass relevant uv environment variables. Feedback focuses on ensuring uv can install to the system Python environment by default, using mirrors for the initial uv bootstrap, and maintaining consistency by migrating remaining pip commands and external test runner logic to uv.
| -e UV_EXTRA_INDEX_URL=$UV_EXTRA_INDEX_URL \ | ||
| -e UV_INDEX_STRATEGY=$UV_INDEX_STRATEGY \ | ||
| -e UV_NO_CACHE=$UV_NO_CACHE \ | ||
| -e UV_SYSTEM_PYTHON=$UV_SYSTEM_PYTHON \ |
There was a problem hiding this comment.
By default, uv prevents installation into the system Python environment unless the UV_SYSTEM_PYTHON environment variable is set to 1 (or the --system flag is used). If this variable is not set in the host environment, the uv pip install commands inside the container will fail. It is safer to provide a default value of 1 here to ensure the CI works correctly in containerized environments.
| -e UV_SYSTEM_PYTHON=$UV_SYSTEM_PYTHON \ | |
| -e UV_SYSTEM_PYTHON=${UV_SYSTEM_PYTHON:-1} \ |
| -e UV_EXTRA_INDEX_URL=$UV_EXTRA_INDEX_URL \ | ||
| -e UV_INDEX_STRATEGY=$UV_INDEX_STRATEGY \ | ||
| -e UV_NO_CACHE=$UV_NO_CACHE \ | ||
| -e UV_SYSTEM_PYTHON=$UV_SYSTEM_PYTHON \ |
There was a problem hiding this comment.
Similar to the change above, providing a default value for UV_SYSTEM_PYTHON ensures that uv can install packages into the system Python environment within the container without requiring explicit configuration in the host environment.
| -e UV_SYSTEM_PYTHON=$UV_SYSTEM_PYTHON \ | |
| -e UV_SYSTEM_PYTHON=${UV_SYSTEM_PYTHON:-1} \ |
| if [ "$MODELSCOPE_SDK_DEBUG" == "True" ]; then | ||
| # pip config set global.index-url https://pypi.tuna.tsinghua.edu.cn/simple | ||
| pip install -r requirements/tests.txt -i https://mirrors.aliyun.com/pypi/simple/ | ||
| pip install uv |
There was a problem hiding this comment.
The pip install uv command does not use the mirror index that was previously configured for package installations. In environments where the default PyPI index is slow or unreachable (as suggested by the previous use of the Aliyun mirror), this bootstrap step may fail or be significantly slower. Consider using the mirror for this step as well.
| pip install uv | |
| pip install uv ${UV_INDEX_URL:+-i $UV_INDEX_URL} |
| # pip config set global.index-url https://pypi.tuna.tsinghua.edu.cn/simple | ||
| pip install -r requirements/tests.txt -i https://mirrors.aliyun.com/pypi/simple/ | ||
| pip install uv | ||
| uv pip install -r requirements/tests.txt |
There was a problem hiding this comment.
The migration to uv appears incomplete. While this shell script now uses uv pip install, the test runner tests/run.py (called on line 45) still contains functions like install_packages and install_requirements that use pip. Additionally, the pip uninstall command on line 25 was not migrated. For consistent performance gains and to avoid mixing package managers, all package operations should be moved to uv.
Zherphy
force-pushed
the
feature/zy_buildfaster_0421
branch
2 times, most recently
from
93c182c to
4e83fe3
Compare
- Add UV environment variables for PyPI mirror configuration - Replace pip install with uv pip install for faster package installation - Pass UV env vars to docker container for ms-swift CI Signed-off-by: Zherphy <1123678689@qq.com>
Zherphy
force-pushed
the
feature/zy_buildfaster_0421
branch
from
4e83fe3 to
e724c0c
Compare
1 participant
PR type
PR information
Add UV environment variables for PyPI mirror configuration
Replace pip install with uv pip install for faster package installation
Pass UV env vars to docker container for ms-swift CI
Experiment results
Paste your experiment result here(if needed).