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

feat: use final redirect URL for chunk download #39

Merged
merged 5 commits into from
Jul 22, 2024

Conversation

McPatate
Copy link
Member

@McPatate McPatate commented Jul 20, 2024

Only call the final redirect URL to avoid overloading the Hub with requests and also altering the download count

cc @osanseviero & @Pierrci regarding download count.

@Wauplin we should check that we do the same in huggingface_hub for download but also when using the multipart upload, we want to make sure parts_urls: Vec<String> only contains s3 urls and not hugginface.co (though this won't affect download count, more a thing about reducing load on hf.co).

Also included in this PR:

  • fix clippy warnings
  • bump dependencies, audit was failing

@McPatate McPatate requested review from Narsil and XciD July 20, 2024 08:28
@julien-c
Copy link
Member

pretty sure it's always already been the case in huggingface_hub

@julien-c
Copy link
Member

@McPatate for reference, can you link to internal discussion of this subject? (e.g. a Slack link)

@McPatate
Copy link
Member Author

McPatate commented Jul 22, 2024

This conversation for example: https://huggingface.slack.com/archives/C02EMARJ65P/p1717775944145709.

We discussed this mostly offline w/ @XciD

@McPatate McPatate force-pushed the feat/use_final_redirect_url branch from 9092db2 to b7e5ea7 Compare July 22, 2024 08:57
@McPatate McPatate merged commit 1e3b846 into main Jul 22, 2024
11 checks passed
@McPatate McPatate deleted the feat/use_final_redirect_url branch July 22, 2024 09:52
@McPatate McPatate restored the feat/use_final_redirect_url branch July 22, 2024 20:40
@McPatate McPatate deleted the feat/use_final_redirect_url branch July 22, 2024 20:41
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