Skip to content

Handle Rate Limits in Pagination with Automatic Retries #2970

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

Merged
merged 4 commits into from
Apr 16, 2025

Conversation

Weyaxi
Copy link
Contributor

@Weyaxi Weyaxi commented Apr 1, 2025

This PR enhances the paginate function in src/huggingface_hub/utils/_pagination.py/_pagination.py by implementing automatic retries using the http_backoff function. Previously, when attempting to list all models via api.list_models(), the process would fail due to rate limiting (HTTP 429) after approximately 500K models (this number depends on many factors). This led to data loss and required restarting the process from the beginning. Additionally, after restarting, the process would hit the rate limit again after another 500K models, making it impossible to scrape all models in a single run.

Changes & Improvements:

  • Replaced session.get with http_backoff to enable automatic retries.
  • Configured max_retries=20 and retry_on_status_codes=429 to handle API rate limits gracefully.
  • Generalized solution so that all API calls using paginate benefit from improved stability (though this rate limiting is most likely to occur only when listing >1.5M models).

Why This Change?

  • Prevents failures & data loss when paginating through large queries.
  • Ensures uninterrupted API calls by retrying when rate limits are hit.
  • Enhances pagination reliability for all functions using paginate, not just model listing.

This PR was initially planned to implement retry logic directly within _pagination.py instead of using the http_backoff function. However, thanks to @Wauplin's great guidance in pointing out that http_backoff already exists and is highly stable, this approach is now much better! 🙂

Thanks, @Wauplin! 🚀


Below screenshot shows the behavior before this PR. As seen in the screenshot, an exception was thrown when the rate limit was hit, and restarting the process would only result in the same issue occurring again. As stated earlier, this made it impossible to access the remaining models in a simple way. 🙂

image

Copy link
Contributor

@Wauplin Wauplin 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 opening the PR @Weyaxi ! As discussed in DMs, I think it's a reasonable solution 👍

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@@ -42,7 +42,7 @@ def paginate(path: str, params: Dict, headers: Dict) -> Iterable:
next_page = _get_next_page(r)
while next_page is not None:
logger.debug(f"Pagination detected. Requesting next page: {next_page}")
r = session.get(next_page, headers=headers)
r = http_backoff("GET", next_page, max_retries=20, retry_on_status_codes=429, headers=headers)
Copy link
Contributor

Choose a reason for hiding this comment

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

shoud add from . import get_session, hf_raise_for_status, http_backoff, logging in import section

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, I forgot, sorry. Should be done with this:

5d455cd

Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

Let's wait for green CI and we're good

Copy link
Contributor

@hanouticelina hanouticelina 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, thank you!

Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

It seems this test is broken: https://github.com/huggingface/huggingface_hub/blob/main/tests/test_utils_pagination.py#L9 Would you like to check what's wrong with it? Most likely the http_backoff method that has to be mocked as well (same as currently done with get_session). Let me know if you need help.

@Wauplin
Copy link
Contributor

Wauplin commented Apr 16, 2025

(pushed 03f8ff9 to fix the mocked tests)

@Wauplin Wauplin merged commit 37ab5ba into huggingface:main Apr 16, 2025
21 checks passed
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.

4 participants