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

Standardize HF_ACCESS_TOKEN -> HF_TOKEN #610

Merged
merged 3 commits into from
Dec 6, 2023
Merged

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Dec 5, 2023

Same as huggingface/transformers.js#431 and huggingface/huggingface.js#391.
Related to slack thread (internal).

This PR standardizes the name of the environment variable used to make requests with a HF API token. The goal is to harmonize it across the HF ecosystem. For what I've seen, both HF_TOKEN and HF_ACCESS_TOKEN are defined in this repo. What this PR does is:

  • rename HF_ACCESS_TOKEN to HF_TOKEN => the token used for api request
  • rename HF_TOKEN to HF_DEPLOYMENT_TOKEN => the token used to deploy chat-ui to production. This looks like an internal secret.

Given this switch, there is a chance that we mismatch an old HF_TOKEN (for deployment) with a new HF_TOKEN (for api requests). Do we only need to change github secrets or is there a possibility that users conifgured chat-ui in their own environment with the old HF_TOKEN as environment variable? In that case, do you have an idea how to solve this? In any case, please let me know if there's anytime to change.

TODO before merging update HF_TOKEN => HF_DEPLOYMENT_TOKEN and HF_ACCESS_TOKEN => HF_TOKEN in Github repo secrets.

cc @xenova @julien-c

@nsarrazin
Copy link
Collaborator

Hi @Wauplin thanks for the PR, I'll handle the change to the secrets 👍

@nsarrazin
Copy link
Collaborator

I think for legacy support, we should check HF_ACCESS_TOKEN and HF_TOKEN (with HF_TOKEN taking precedence) before pushing those changes. Would this be ok or is this intended to be a breaking change ? @Wauplin

The old HF_TOKEN (new HF_DEPLOYMENT_TOKEN) is only used for HuggingChat CI and will not be impacting users.

@Wauplin
Copy link
Contributor Author

Wauplin commented Dec 5, 2023

Thanks @nsarrazin. I'm looking at the failed CI and I'm getting Error: Module '"$env/static/private"' has no exported member 'HF_TOKEN'. . Is this because github secrets are not updated or should I update $env/static/private? If I need to update static/private, where should it be done?

I think for legacy support, we should check HF_ACCESS_TOKEN and HF_TOKEN (with HF_TOKEN taking precedence) before pushing those changes. Would this be ok or is this intended to be a breaking change ?

Yes it was my intention to keep backward compatibility. I've done it in scripts/updateProdEnv.ts but maybe it's not enough?

const HF_TOKEN = process.env.HF_TOKEN ?? process.env.HF_ACCESS_TOKEN;

Feel free to directly push on my branch if it's easier.

@nsarrazin
Copy link
Collaborator

No problem, the CI failed because the .env was still using HF_ACCESS_TOKEN.

I added legacy support for HF_ACCESS_TOKEN in the code itself, see 89f2d96 @Wauplin

@Wauplin
Copy link
Contributor Author

Wauplin commented Dec 5, 2023

Thanks @nsarrazin! I did not see the .env file 🙈
And thanks for adding legacy support as well, it'll definitely be important for some already-deployed spaces. Feel free to merge once reviewed/approved!

Copy link
Collaborator

@nsarrazin nsarrazin left a comment

Choose a reason for hiding this comment

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

LGTM in the current state!

Copy link
Collaborator

@gary149 gary149 left a comment

Choose a reason for hiding this comment

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

ok

@nsarrazin
Copy link
Collaborator

Updated the secrets, merging this!

@nsarrazin nsarrazin merged commit 3cbea34 into main Dec 6, 2023
@nsarrazin nsarrazin deleted the harmonize-hf-token branch December 6, 2023 10:46
ice91 pushed a commit to ice91/chat-ui that referenced this pull request Oct 30, 2024
* Standardize HF_ACCESS_TOKEN -> HF_TOKEN

* Replace HF_ACCESS_TOKEN by HF_TOKEN in .env

* Add legacy support for HF_ACCESS_TOKEN

---------

Co-authored-by: Nathan Sarrazin <sarrazin.nathan@gmail.com>
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.

3 participants