-
Notifications
You must be signed in to change notification settings - Fork 129
Fix vllm ci #265
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
Fix vllm ci #265
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,7 +29,7 @@ help: ## Print help | |
| TOKENIZER_LIB = lib/libtokenizers.a | ||
|
|
||
| # Extract RELEASE_VERSION from Dockerfile | ||
| TOKENIZER_VERSION := $(shell grep '^ARG RELEASE_VERSION=' Dockerfile | cut -d'=' -f2) | ||
| TOKENIZER_VERSION ?= $(shell grep '^ARG RELEASE_VERSION=' Dockerfile | cut -d'=' -f2) | ||
|
|
||
| .PHONY: download-tokenizer | ||
| download-tokenizer: $(TOKENIZER_LIB) | ||
|
Comment on lines
31
to
35
|
||
|
|
@@ -116,7 +116,7 @@ detect-python: ## Detects Python and prints the configuration. | |
| .PHONY: setup-venv | ||
| setup-venv: detect-python ## Sets up the Python virtual environment. | ||
| @printf "\033[33;1m==== Setting up Python virtual environment in $(VENV_DIR) ====\033[0m\n" | ||
| @if [ ! -f "$(VENV_BIN)/pip" ]; then \ | ||
| @if [ ! -f "$(VENV_BIN)/python" ]; then \ | ||
| echo "Creating virtual environment..."; \ | ||
| $(PYTHON_EXE) -m venv $(VENV_DIR) || { \ | ||
| echo "ERROR: Failed to create virtual environment."; \ | ||
|
|
@@ -132,17 +132,15 @@ setup-venv: detect-python ## Sets up the Python virtual environment. | |
| .PHONY: install-python-deps | ||
| install-python-deps: setup-venv ## installs dependencies. | ||
| @printf "\033[33;1m==== Setting up Python virtual environment in $(VENV_DIR) ====\033[0m\n" | ||
| @if [ ! -f "$(VENV_BIN)/pip" ]; then \ | ||
| echo "Creating virtual environment..."; \ | ||
| $(PYTHON_EXE) -m venv $(VENV_DIR) || { \ | ||
| echo "ERROR: Failed to create virtual environment."; \ | ||
| echo "Your Python installation may be missing the 'venv' module."; \ | ||
| echo "Try: 'sudo apt install python$(PYTHON_VERSION)-venv' or 'sudo dnf install python$(PYTHON_VERSION)-devel'"; \ | ||
| exit 1; \ | ||
| }; \ | ||
| @if [ ! -f "$(VENV_BIN)/python" ]; then \ | ||
|
hyeongyun0916 marked this conversation as resolved.
|
||
| echo "ERROR: Virtual environment not found. Run 'make setup-venv' first."; \ | ||
| exit 1; \ | ||
| fi | ||
| @if $(VENV_BIN)/python -c "import vllm" 2>/dev/null; then \ | ||
| echo "vllm is already installed, skipping..."; \ | ||
| exit 0; \ | ||
| fi | ||
| @echo "Upgrading pip and installing dependencies..." | ||
|
|
||
| @echo "Installing vllm..." | ||
| @if [ "$(TARGETOS)" = "linux" ]; then \ | ||
| if [ "$(TARGETARCH)" = "amd64" ]; then \ | ||
| echo "Installing vLLM pre-built wheel for x86_64..."; \ | ||
|
|
||
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.
The file paths are inconsistent in the final image stage. Line 76 copies Python source code to '/app/pkg/preprocessing/chat_completions', while line 77 copies site-packages to '/workspace/build/venv/lib/python3.12/site-packages'. The PYTHONPATH on line 81 reflects this mixed structure with both '/app/' and '/workspace/' prefixes. While this will likely function correctly, it's inconsistent and harder to maintain. Consider organizing all Python-related files under a single directory prefix (either '/app/' or '/workspace/') for better clarity and maintainability.