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.

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

Add Dockerfile #278

Merged
merged 4 commits into from
Jan 13, 2025
Merged

Add Dockerfile #278

merged 4 commits into from
Jan 13, 2025

Conversation

nvonpentz
Copy link
Contributor

Resolves #276

Dockerfile Outdated
@@ -0,0 +1,21 @@
# Use vLLM's vllm-openai server image as the base
FROM vllm/vllm-openai:latest
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a way that we can set the version of this base image to the one pinned in pyproject.toml here? It will be nice if the version here changes automatically when we migrate the version of vLLM in the pyproject.toml.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find a way to do this that wasn't kind of hacky. But I did update this to match the pyproject.toml.

Dockerfile Outdated

# Install additional Python dependencies
RUN --mount=type=cache,target=/root/.cache/pip \
python3 -m pip install -r requirements.txt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you change the dependency installation to follow the instructions in this branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like that branch has not been merged, so there's no pyproject.toml in this branch

Copy link
Collaborator

@jeffreymeetkai jeffreymeetkai Nov 6, 2024

Choose a reason for hiding this comment

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

I will merge that branch before this so you can install following the instructions from that branch first.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That branch has just been merged to main.

Dockerfile Outdated
python3 -m pip install -r requirements.txt

# Override the VLLM entrypoint with the functionary server
ENTRYPOINT ["python3", "server_vllm.py", "--model", "meetkai/functionary-small-v3.2", "--host", "0.0.0.0", "--max-model-len", "8192"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we allow user-defined options at container runtime? Maybe something like CMD []

Dockerfile Outdated
FROM vllm/vllm-openai:latest

# Set working directory
WORKDIR /workspace
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we allow user-defined workdir? Maybe we can default to /workspace if none is provided.

@nvonpentz nvonpentz force-pushed the dockerfile branch 2 times, most recently from 06745e9 to 0f54ac2 Compare November 12, 2024 14:54
@nachthammer
Copy link

Any progress here? Would be nice to have a docker deployment option for vllm @jeffreymeetkai
I mean if the dockerfiles need some user defined attributes, the user can change it themselves when cloning this repository. And otherwise just having a Dockerfile here even if it is per default not super customizable would be very wild.

@nvonpentz
Copy link
Contributor Author

It's been ready for another review from @jeffreymeetkai, though it's possible it's gone stale since it's been a little while

nvonpentz and others added 4 commits January 13, 2025 13:48
* Allow overriding of entrypoint arguments with CMD []
* Make WORKDIR an argument
* Pin to specific vllm-openai image
Copy link
Collaborator

@jeffreymeetkai jeffreymeetkai left a comment

Choose a reason for hiding this comment

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

Sorry for getting back really late. LGTM, just that I've moved the file into the dockerfiles directory to consolidate all the dockerfiles together. Will merge after the tests are passed. Thanks for this PR!

@jeffreymeetkai jeffreymeetkai merged commit 7cbe866 into MeetKai:main Jan 13, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dockerfile and official docker image?
4 participants