-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[CI/Build] Replace vllm.entrypoints.openai.api_server
entrypoint with vllm serve
command
#25967
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
Conversation
Signed-off-by: DarkLight1337 <[email protected]>
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.
LGTM
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.
Code Review
This pull request updates the Dockerfiles to use the vllm serve
command as the entrypoint, replacing the direct call to the python module vllm.entrypoints.openai.api_server
. This change aligns the Docker images with the recommended command-line interface, improving consistency and user experience. The changes are applied correctly and consistently across the CUDA, CPU, and XPU Dockerfiles. The new entrypoint is backward compatible with existing argument passing conventions. The changes look good and I don't see any issues.
Signed-off-by: DarkLight1337 <[email protected]>
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.
Thanks @DarkLight1337!
For the Docker to be drop in replacement, we need the old flow
to work as well |
Oh, I forgot about |
Signed-off-by: DarkLight1337 <[email protected]>
I have updated the CLI parsing logic to deprecate instead of erroring out when |
vllm serve
commandvllm.entrypoints.openai.api_server
entrypoint with vllm serve
command
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
@simon-mo can you check again? Mamba kernels test failure is happening on main as well so it can be ignored. |
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.
Thanks @DarkLight1337!
"With `vllm serve`, you should provide the model as a " | ||
"positional argument or in a config file instead of via " | ||
"the `--model` option.") | ||
"the `--model` option. " | ||
"The `--model` option will be removed in v0.13.") |
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.
Is there any harm in allowing both indefinitely?
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.
I am not sure who removed the ability to use --model
in the first place. Maybe @mgoin ?
Signed-off-by: DarkLight1337 <[email protected]>
…th `vllm serve` command (vllm-project#25967) Signed-off-by: DarkLight1337 <[email protected]>
…th `vllm serve` command (#25967) Signed-off-by: DarkLight1337 <[email protected]> Signed-off-by: yewentao256 <[email protected]>
Purpose
vllm serve <model>
works now, without having to add--model
. So I think it's safe to just change the command directly.Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.md
andexamples
for a new model.