Skip to content

[Bugfix] add hf_token to EngineArgs #16093

New issue

Have a question about this project? No Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “No 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? No Sign in to your account

Merged
merged 13 commits into from
Apr 6, 2025

Conversation

paolovic
Copy link
Contributor

@paolovic paolovic commented Apr 5, 2025

Added hf_token to EngineArgs. It stores True if just the flag and no value is given, it stores the value if the flag and value are given, and evaluates to None otherwise.
Also, it can be passed to LLM as an argument where it's forwarded in the __init__ to EngineArgs where it's stored as an attribute and forwarded to ModelConfig in create_model_config.
Within ModelConfig it's available to be passed to get_hf_image_processor_config which forwards it to transformers' get_image_processor_config.

FIX 14854

paolovic and others added 11 commits March 27, 2025 10:19
Signed-off-by: paolovic <paul-philipp.luley@uzh.ch>
Signed-off-by: paolovic <paul-philipp.luley@uzh.ch>
Signed-off-by: paolovic <paul-philipp.luley@uzh.ch>
Signed-off-by: paolovic <paul-philipp.luley@uzh.ch>
Signed-off-by: paolovic <paul-philipp.luley@uzh.ch>
Signed-off-by: paolovic <paul-philipp.luley@uzh.ch>
Signed-off-by: paolovic <paul-philipp.luley@uzh.ch>
Copy link

github-actions bot commented Apr 5, 2025

👋 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 fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

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 ready label to the PR or enable auto-merge.

🚀

@mergify mergify bot added the frontend label Apr 5, 2025
@paolovic
Copy link
Contributor Author

paolovic commented Apr 5, 2025

branched out a new source branch such that I am not blocked until @void-mckenzie gives his feedback

@WoosukKwon
Copy link
Collaborator

QQ: Why not just use env variables like HF_TOKEN or HF_HOME?

@paolovic
Copy link
Contributor Author

paolovic commented Apr 5, 2025

Hi @WoosukKwon ,
void-mckenzie requested this feature like the following:
"It works fine if you just have one HF token. But if you have multiple, you need to reset the environment variable every time. Wouldn't it make more sense to use the inbuilt auth token functionality in transformers? It exists to support multiple token access. I have an issue right now with my unsloth build, since vllm cuts off the auth token pathway from unsloth to transformers."

In the following DarkLight1337 and him agreed on the design that is implemented with this PR.
#14854

Copy link
Member

@DarkLight1337 DarkLight1337 left a comment

Choose a reason for hiding this comment

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

Since the OP of the original issue hasn't responded, I'll just merge this first

@DarkLight1337 DarkLight1337 enabled auto-merge (squash) April 6, 2025 12:54
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Apr 6, 2025
@DarkLight1337 DarkLight1337 merged commit da224da into vllm-project:main Apr 6, 2025
62 checks passed
@paolovic paolovic deleted the feature/hf_token branch April 6, 2025 15:38
lengrongfu pushed a commit to lengrongfu/vllm that referenced this pull request Apr 7, 2025
Signed-off-by: paolovic <paul-philipp.luley@uzh.ch>
Co-authored-by: paolovic <paul-philipp.luley@uzh.ch>
@void-mckenzie
Copy link

@paolovic Sorry! been busy with a few things. I see that this has been merged, let me try to see if I can pass the tokens. I'll keep you updated. once again, sorry for the delay!

nishith-fujitsu pushed a commit to nishith-fujitsu/vllm that referenced this pull request Apr 9, 2025
Signed-off-by: paolovic <paul-philipp.luley@uzh.ch>
Co-authored-by: paolovic <paul-philipp.luley@uzh.ch>
yangw-dev pushed a commit to yangw-dev/vllm that referenced this pull request Apr 21, 2025
Signed-off-by: paolovic <paul-philipp.luley@uzh.ch>
Co-authored-by: paolovic <paul-philipp.luley@uzh.ch>
Signed-off-by: Yang Wang <elainewy@meta.com>
No Sign up for free to join this conversation on GitHub. Already have an account? No Sign in to comment
Labels
frontend ready ONLY add when PR is ready to merge/full CI is needed
Projects
None yet
4 participants