-
Notifications
You must be signed in to change notification settings - Fork 67
Drop Entrypoint and Improve Containerfile #250
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
94bef5f
to
36de5fa
Compare
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.
Pull Request Overview
This PR simplifies the container deployment by removing the custom entrypoint script and updating the Containerfile to use native GuideLLM features. The changes focus on improving container rebuild efficiency and reducing user confusion.
- Remove the custom
entrypoint.sh
script that duplicated functionality now built into GuideLLM - Update Containerfile to use more rebuild-friendly patterns and standardized paths
- Add documentation for container usage with examples
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
deploy/entrypoint.sh | Complete removal of custom entrypoint script |
deploy/Containerfile | Updated to use native GuideLLM entrypoint, improved build patterns, and standardized paths |
README.md | Added container usage documentation with practical examples |
Comments suppressed due to low confidence (1)
deploy/Containerfile:40
- The default target 'http://localhost:8000' points to localhost inside the container, which likely won't work in most deployment scenarios. This could confuse users who expect the container to work out-of-the-box without understanding they need to override this value.
ENV GUIDELLM_TARGET="http://localhost:8000" \
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
36de5fa
to
e879267
Compare
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!
* Drop the container entrypoint script as GuideLLM has had native support for its features since vllm-project#99 * Make containerfile more rebuild friendly based on vllm-project#213 * Drop the ENV default scenario as it is confusing to users setting CLI args Closes: vllm-project#213 --------- Signed-off-by: Samuel Monson <[email protected]>
Closes: #213