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

RFC tokens should be persisted only with explicit request of the user #796

Closed
adrinjalali opened this issue Mar 24, 2022 · 3 comments
Closed
Labels

Comments

@adrinjalali
Copy link
Contributor

When working with HfApi (or other parts of this library), as a user I expect setting the access token (via set_access_token or deprecated login) to set the token for the instance and not the user.

An example of why this becomes an issue was touched here: #787 (comment) where we noticed running tests locally overrides the persisted token of the user.

As a user, I would expect this to work:

api1 = HfApi().set_access_token(token1)
api1.create_repo(repo_id='my_repo1')  # not passing token here

api2 = HfApi().set_access_token(token2)
api2.create_repo(repo_id='my_repo2')  # not passing token here

api1.delete_repo(repo_id='my_repo1')  # again not passing token

Right now the second HfApi.set_access_token changes the default token of the user and that means api1.delete_repo would try to use the wrong token.

I suggest we deprecate persisting tokens by default, and have other methods or an argument to the method to explicitly indicate that it should be persisted.

cc @LysandreJik @julien-c @osanseviero

@adrinjalali
Copy link
Contributor Author

An alternative API design which seems cleaner to me would be:

api1 = HfApi(token=token1)
api1.create_repo(repo_id='my_repo1')  # not passing token here

api2 = HfApi(token=token2)
api2.create_repo(repo_id='my_repo2')  # not passing token here

api1.delete_repo(repo_id='my_repo1')  # again not passing token

All methods would use the token from the instance, and if not provided, it'll try to fetch from the persisted one. If the user wants to persist a token, they can call a method which explicitly is there to persist the token (we can then talk about how we can persist the tokens not to temper with git).

@LysandreJik
Copy link
Member

I like both! Having set_access_token being called in the HfApi constructor if a token is passed sounds sensible to me. Persisting the token in the instance also sounds like an API improvement to me.

@Wauplin
Copy link
Contributor

Wauplin commented Oct 18, 2022

Addressed and closed by #1116.

@Wauplin Wauplin closed this as completed Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants