-
-
Notifications
You must be signed in to change notification settings - Fork 9k
[Docs] Update docker.md with HF_TOKEN, new model, and podman fix #21856
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
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 Docker documentation by fixing an issue with the podman
command, making the Hugging Face token handling consistent, and changing the example model. While the podman
fix and token handling are good improvements, I've identified a critical issue: the new example model name appears to be invalid, which would cause the documented commands to fail. My review includes suggestions to use a valid model to ensure the examples are runnable for users.
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
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 thanks!
There is also #19236 which attempts to solve the registry issue, do you prefer modifying the podman command instead of modifying the Dockerfile?
I think we have to do both since the change here is modifying the command to fetch the prebuilt image from dockerhub, and the change you linked is just to fix podman building the image from source |
Previously if you ran the podman command, it would fail because it doesn't default to dockerhub as a registry
But there are other issues like gpu passthrough.