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 support for primary rate limits #4

Open
gofri opened this issue Jan 27, 2023 · 6 comments
Open

Add support for primary rate limits #4

gofri opened this issue Jan 27, 2023 · 6 comments

Comments

@gofri
Copy link
Owner

gofri commented Jan 27, 2023

Unlike secondary rate limits, primary rate limits are long and categorized.
As a result, sleeping is not obviously the natural response for them, and the implementation must be aware of the categories.

Options (not mutually exclusive):

  • Add a callback for when a primary rate limit is detected (no need for category awareness).
  • Handle the different categories and implement a complete solution (maybe with more freedom regarding the user's response).
  • Provide it as a separate RoundTripper.
  • Provide wrapper functions to stack them.
@gofri
Copy link
Owner Author

gofri commented Apr 29, 2023

Following the discussion in #9 - will be implemented using x-ratelimit-resource (and perhaps a fallback for endpoint-based heuristics, because x-ratelimit-resource isn't officially documented).
Will probably go into v2.0.0 to allow a clearer API, including:

  • Renaming the options to be clearer on secondary vs primary rate limit.
  • Support for returning a dedicated error instead of sleeping (probably different error type for secondary/primary -- keeping sleep the default for secondary, and return error as the default for primary).
  • WithXXX for the options of injected (instead of the lazy approach of single options struct used now).

@NorseGaud
Copy link

bump :)

@gofri
Copy link
Owner Author

gofri commented Feb 14, 2024

@NorseGaud nice to see interest. I'd love to hear more about your use case (and hopefully, I'll get to it soon)

@gofri
Copy link
Owner Author

gofri commented Mar 2, 2024

Apparently, code search rate limits behave like primary rate limits (i.e., the response is a primary rate limit error),
but their reset time is really short (1min).
That means that when adding support for primary rate limit, it also needs to support sleep by default for short periods.

@NorseGaud @abhijit-hota - I haven't forgotten about this issue and I'm planning to do it soon. My days are pretty busy now, and I'm finishing off async-pagination support now, and then I'm gonna address this one.
https://github.com/gofri/go-github-pagination

@Sawthis
Copy link

Sawthis commented Apr 10, 2024

@gofri any update on this topic?

@gofri
Copy link
Owner Author

gofri commented Apr 10, 2024

@Sawthis
I wish.
I'm really over my head with work and my other duties. Haven't forgotten about it, and I hope that I'll be able to make the first step next week

Edit: not sure if you heard the news about Iran attacking israel, but there might be some more delay after all 😅

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

No branches or pull requests

3 participants