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

Add auth to snapshot download #340

Merged
merged 12 commits into from
Sep 13, 2021

Conversation

lewtun
Copy link
Member

@lewtun lewtun commented Sep 10, 2021

This PR adds a use_auth_token argument to snapshot_download so private repos can be downloaded from the Hub.

Usage

from huggingface_hub import snapshot_download

# Download with explicit token
filepath = snapshot_download("some-private-repo", use_auth_token="api_XXX")
# Download with token from cache
filepath = snapshot_download("some-private-repo", use_auth_token=True)

Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

Thanks for working on the implementation @lewtun, this was an oversight!

src/huggingface_hub/snapshot_download.py Outdated Show resolved Hide resolved
@lewtun
Copy link
Member Author

lewtun commented Sep 10, 2021

I'm not sure why the unit tests are failing since they seem to be unrelated to the changes in this PR 🤔

Copy link
Member

@osanseviero osanseviero left a comment

Choose a reason for hiding this comment

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

Re: failing tests, some are unfortunately flaky at the moment :( You can try to re-run the workflows.

Code looks good to me, it would be great if you add some unit tests. You can use private in Repository to make a private repo. Then you could have a test without token that shows that the command fails, and another one with token that shows it works.

@lewtun lewtun force-pushed the add-auth-to-snapshot-download branch from 1f52587 to 42d3ed1 Compare September 12, 2021 16:57
@lewtun
Copy link
Member Author

lewtun commented Sep 12, 2021

Code looks good to me, it would be great if you add some unit tests. You can use private in Repository to make a private repo. Then you could have a test without token that shows that the command fails, and another one with token that shows it works.

Thanks for the tips and suggestions - the new unit test should cover the three possible values of use_auth_token in snapshot_download

tests/test_hf_api.py Outdated Show resolved Hide resolved
tests/test_hf_api.py Outdated Show resolved Hide resolved
Copy link
Member

@osanseviero osanseviero left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks a lot

@lewtun
Copy link
Member Author

lewtun commented Sep 13, 2021

@LysandreJik this PR is ready for a final pass / approval on your side :)

Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

Looks great, thank you for iterating @lewtun!

src/huggingface_hub/snapshot_download.py Show resolved Hide resolved
@LysandreJik LysandreJik merged commit 10e7445 into huggingface:main Sep 13, 2021
@LysandreJik
Copy link
Member

Will do a release containing this commit later today.

@lewtun lewtun deleted the add-auth-to-snapshot-download branch September 13, 2021 11:34
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.

None yet

3 participants