Skip to content
This repository has been archived by the owner on Jun 24, 2024. It is now read-only.

[Idea] Sleep until rate limit updates and try again #1644

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

johndbritton
Copy link
Contributor

In working on updating ES indexes, I'm slightly worried about exhausting some rate limits.

This is just an idea for how we could rescue from a hitting our rate limits, hold off for a cool down period, and then resume work.

If something like this would work, I think it might be worth defaulting to not behaving this way and implementing a way to choose the behavior, similar to how we can choose to skip the API cache.

cc/ @d12 @tarebyte for your opinions on the approach.

rescue GitHub::TooManyRequests
retries ||= 0
if retries < 1
retries +=1

Choose a reason for hiding this comment

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

Layout/SpaceAroundOperators: Surrounding space missing for operator +=.

rescue GitHub::TooManyRequests
retries ||= 0
if retries < 1
retries +=1

Choose a reason for hiding this comment

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

Layout/SpaceAroundOperators: Surrounding space missing for operator +=.

@johndbritton
Copy link
Contributor Author

Maybe we could move the sleep / retry logic into the with_error_handling definition and enable it by passing an argument?

@tarebyte
Copy link
Member

Maybe we could move the sleep / retry logic into the with_error_handling definition and enable it by passing an argument?

Yeah that seems reasonable to me 👍

Copy link
Contributor

@BenEmdon BenEmdon left a comment

Choose a reason for hiding this comment

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

Maybe we could move the sleep / retry logic into the with_error_handling definition and enable it by passing an argument?

This would cover all our handled API requests so I'd be more in favor of this

@stale
Copy link

stale bot commented Dec 10, 2018

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale label Dec 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants