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

Support HF_TOKEN environment variable in huggingface_provider.py #59

Merged
merged 3 commits into from
Dec 7, 2024

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Nov 25, 2024

This PR updates huggingface_provider.py to pull the huggingface token from HF_TOKEN environment variable instead of HUGGINGFACE_TOKEN. This is indeed the env variable supported by all libraries in the HF ecosystem (not just in the Python client, but in Rust, JS, etc. clients). Better to align now before it's too late for a change!

Disclaimer: I work at HF as maintainer of the huggingface_hub Python client 🤗


EDIT: if adding huggingface_hub as an optional dependency is not a problem, I would even advice to use huggingface_hub.get_token() instead. Returns None if the user is not connected. Otherwise it pulls the token from either an environment variable, a locally saved token (from huggingface-cli login) or from the secrets vault in a Google Colab session.

EDIT 2: using huggingface_hub.InferenceClient would also do that automatically + seamlessly work with Inference API (our serverless offer), Inference Endpoints (our dedicated servers) or a local TGI deployment. Please let me know if you are interested in a PR that uses that instead of a raw httpx call :)

@rohitprasad15
Copy link
Collaborator

@Wauplin - thanks for the suggestion. I changed it elsewhere in the documentation too.
Regarding using the client vs raw httpx call - we intended to keep the library lightweight and not use a lot of dependencies. We are still debating this between the team given the planned features ahead.
If we need to start using the client, I will ping you for sure! :) Thanks!

@rohitprasad15 rohitprasad15 merged commit 8c358d6 into andrewyng:main Dec 7, 2024
4 checks passed
@Wauplin
Copy link
Contributor Author

Wauplin commented Dec 7, 2024

Yep, as a library maintainer I understand the concern about adding dependencies! I suggested it since I saw anthropic, openai, mistral, etc. are already dependencies of the project.

@rohitprasad15
Copy link
Collaborator

You are right about your observation. If we decide to use http only, we will change all those implementations. And I did not want your work to go to waste if we decide the http route. Most likely it looks like we will continue to use the clients. I will keep you posted. Thanks again! :)

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.

2 participants