Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[CI/Build] Use python 3.12 in cuda image #8133

Merged
merged 11 commits into from
Sep 7, 2024
Merged

[CI/Build] Use python 3.12 in cuda image #8133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7b4f697
:arrow_up: use python 3.12 in prod image
joerunde Sep 3, 2024
c5772e0
:bug: align other python version arg
joerunde Sep 3, 2024
195b19e
:bug: fixup test installs
joerunde Sep 3, 2024
3c430c1
:bug: fixup called_with assert
joerunde Sep 4, 2024
4d7ed46
:bug: pin six
joerunde Sep 4, 2024
1f636d2
:bug: update one lora example
joerunde Sep 4, 2024
09481e8
Merge remote-tracking branch 'upstream/main' into python-312
joerunde Sep 4, 2024
731ff2e
empty bump
joerunde Sep 4, 2024
8ea8e9c
Merge remote-tracking branch 'upstream/main' into python-312
joerunde Sep 4, 2024
b2e05b4
:bug: fixup mock patch
joerunde Sep 4, 2024
a3c6f8e
Merge remote-tracking branch 'upstream/main' into python-312
joerunde Sep 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ ARG CUDA_VERSION=12.4.1
# prepare basic build environment
FROM nvidia/cuda:${CUDA_VERSION}-devel-ubuntu20.04 AS base
ARG CUDA_VERSION=12.4.1
ARG PYTHON_VERSION=3.10
ARG PYTHON_VERSION=3.12
ENV DEBIAN_FRONTEND=noninteractive

# Install Python and other dependencies
Expand Down Expand Up @@ -135,7 +135,7 @@ RUN --mount=type=cache,target=/root/.cache/pip \
# image with vLLM installed
FROM nvidia/cuda:${CUDA_VERSION}-base-ubuntu20.04 AS vllm-base
ARG CUDA_VERSION=12.4.1
ARG PYTHON_VERSION=3.10
ARG PYTHON_VERSION=3.12
WORKDIR /vllm-workspace
ENV DEBIAN_FRONTEND=noninteractive

Expand Down Expand Up @@ -181,6 +181,10 @@ FROM vllm-base AS test
ADD . /vllm-workspace/

# install development dependencies (for testing)
# A newer setuptools is required for installing some test dependencies from source that do not publish python 3.12 wheels
# This installation must complete before the test dependencies are collected and installed.
RUN --mount=type=cache,target=/root/.cache/pip \
python3 -m pip install "setuptools>=74.1.1"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build without this failed with

[2024-09-03T22:25:50Z] #37 2.608 Collecting transformers_stream_generator (from -r requirements-test.txt (line 24))
[2024-09-03T22:25:50Z] #37 2.618   Downloading transformers-stream-generator-0.0.5.tar.gz (13 kB)
[2024-09-03T22:25:50Z] #37 2.627   Preparing metadata (setup.py): started
[2024-09-03T22:25:50Z] #37 2.646   Preparing metadata (setup.py): finished with status 'error'
[2024-09-03T22:25:50Z] #37 2.648   error: subprocess-exited-with-error
[2024-09-03T22:25:50Z] #37 2.648   
[2024-09-03T22:25:50Z] #37 2.648   × python setup.py egg_info did not run successfully.
[2024-09-03T22:25:50Z] #37 2.648   │ exit code: 1
[2024-09-03T22:25:50Z] #37 2.648   ╰─> [1 lines of output]
[2024-09-03T22:25:50Z] #37 2.648       ERROR: Can not execute `setup.py` since setuptools is not available in the build environment.
[2024-09-03T22:25:50Z] #37 2.648       [end of output]
[2024-09-03T22:25:50Z] #37 2.648   
[2024-09-03T22:25:50Z] #37 2.648   note: This error originates from a subprocess, and is likely not a problem with pip.
[2024-09-03T22:25:50Z] #37 2.657 error: metadata-generation-failed

I verified this was because the vllm-base target never updates setuptools, so it still has version 45.2.0 installed, which causes this error with a very misleading message.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why setuptools is at 45.2.0 specifically and why being on an older version leads to this error message? The message doesn't mention anything about version being outdated/incompatible, only that setuptools is nto in the build env.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it took some googling around to find that others ran into the same error when setuptools is just out of date- I don't know why that's the case but I'd guess it's just some api incompatibility that's not quite correctly handled.

You can reproduce by installing setuptools==45.2.0 and then trying to install transformers-stream-generator in a fresh python 3.12 venv

I wonder why setuptools is at 45.2.0 specifically

Yeah idk, from the build logs it looks like that's just what's installed when we do apt-get install -y python3.12 python3.12-dev python3.12-venv, I see this in the log:

[2024-09-05T21:34:56Z] #10 11.07 Get:191 http://archive.ubuntu.com/ubuntu focal-updates/main amd64 python3-setuptools all 45.2.0-1ubuntu0.1 [330 kB]

RUN --mount=type=cache,target=/root/.cache/pip \
python3 -m pip install -r requirements-dev.txt

Expand Down
1 change: 1 addition & 0 deletions requirements-common.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ gguf == 0.9.1
importlib_metadata
mistral_common >= 1.3.4
pyyaml
six>=1.16.0; python_version > '3.11' # transitive dependency of pandas that needs to be the latest version for python 3.12
6 changes: 3 additions & 3 deletions tests/test_logger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def test_logger_configuring_can_be_disabled():
config behavior, however mocks are used to ensure no changes in behavior or
configuration occur."""

with patch("logging.config.dictConfig") as dict_config_mock:
with patch("vllm.logger.dictConfig") as dict_config_mock:
_configure_vllm_root_logger()
dict_config_mock.assert_not_called()

Expand Down Expand Up @@ -175,9 +175,9 @@ def test_custom_logging_config_is_parsed_and_used_when_provided():
logging_config_file.flush()
with patch("vllm.logger.VLLM_LOGGING_CONFIG_PATH",
logging_config_file.name), patch(
"logging.config.dictConfig") as dict_config_mock:
"vllm.logger.dictConfig") as dict_config_mock:
_configure_vllm_root_logger()
assert dict_config_mock.called_with(valid_logging_config)
dict_config_mock.assert_called_with(valid_logging_config)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is failing and I'm not sure what the expected behavior of this test was before, looking into it a bit to try to understand

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I think that the mock on logging.config.dictConfig was incorrect, because vllm.logger imports that eagerly, so we instead have to mock the local vllm.logger.dictConfig.

The test was passing before because dict_config_mock.called_with was returning a mock, which was then passing the assert since it's not falsey.



@patch("vllm.logger.VLLM_CONFIGURE_LOGGING", 0)
Expand Down
Loading