vllm icon indicating copy to clipboard operation
vllm copied to clipboard

[WIP] Use uv python for docker

Open mgoin opened this issue 9 months ago • 7 comments

I picked this back up since some of the Python apt commands seemed to be timing out on my PRs. The goal of this PR is to use uv instead of the deadsnakes ppa to install Python

mgoin avatar Feb 19 '25 22:02 mgoin

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

github-actions[bot] avatar Feb 19 '25 22:02 github-actions[bot]

The uv docs sugggest using UV_UNMANAGED_INSTALL for CI, would that allow us to avoid the path changes?

hmellor avatar Feb 20 '25 11:02 hmellor

This pull request has merge conflicts that must be resolved before it can be merged. Please rebase the PR, @mgoin.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

mergify[bot] avatar Feb 20 '25 12:02 mergify[bot]

superseded by #13566

hmellor avatar Feb 20 '25 14:02 hmellor

@hmellor this PR is not superseded, it is trying to enable an additional change to how we are installing python

mgoin avatar Feb 20 '25 15:02 mgoin

This pull request has merge conflicts that must be resolved before it can be merged. Please rebase the PR, @mgoin.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

mergify[bot] avatar Feb 20 '25 15:02 mergify[bot]

Oh, sorry about that!

hmellor avatar Feb 20 '25 16:02 hmellor

OOO why dose ENTRYPOINT ["python3", "-m", "vllm.entrypoints.openai.api_server"] still work when we are using uv globally?

simon-mo avatar Mar 06 '25 16:03 simon-mo

OOO why dose ENTRYPOINT ["python3", "-m", "vllm.entrypoints.openai.api_server"] still work when we are using uv globally?

It should because we placed uv's python at the front of the path

# Create venv with specified Python and activate by placing at the front of path
ENV VIRTUAL_ENV="/opt/venv"
RUN uv venv --python ${PYTHON_VERSION} --seed ${VIRTUAL_ENV}
ENV PATH="$VIRTUAL_ENV/bin:$PATH"

mgoin avatar Mar 06 '25 17:03 mgoin

It should not depend on the host python environment to run the docker. (/root/.local/bin) This PR might be modifying the container environment in a way that removes or breaks the Python 3 executable.

The official vLLM Docker image for v0.8.0 is not functional. I got an error : FATAL: "python3": executable file not found in $PATH

longqu avatar Mar 19 '25 17:03 longqu