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

Implement mechanism to avoid secondary rate limit on PR creation #164

Merged
merged 2 commits into from
Nov 30, 2021

Conversation

serena97
Copy link
Contributor

@serena97 serena97 commented Nov 23, 2021

Implement timeout, retryAfter and sequential request mechanism to avoid hitting the secondary rate limit, which blocks you from creating more PRs, and repeatedly hitting the rate might result in banning of your app.

Error: You have exceeded a secondary rate limit and have been temporarily blocked from content creation. Please retry your request again later.

The mechanism was built upon the best practice to avoid secondary rate limit

  1. Sequential PR creation request - Make requests for a single user or client ID serially. Do not make requests for a single user or client ID concurrently.
  2. Retry After - When you have been limited, use the Retry-After response header to slow down. This change allows for retries to handle the creation of the subsequent PRs
  3. Time out - Please create this content at a reasonable pace to avoid further limiting

Without this mechanism, I was only able to create 10 PRs and then nori will error out with the secondary rate limit. With this mechanism, I was able to successfully create 61 PRs in one go with nori. We need this change as the GitHub API has recently changed to be more defensive with its secondary rate limit, as seen in other projects

@serena97 serena97 requested a review from a team as a code owner November 23, 2021 11:19
@serena97 serena97 mentioned this pull request Nov 23, 2021
Copy link
Contributor

@ivomurrell ivomurrell left a comment

Choose a reason for hiding this comment

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

This all looks sensible and I'm glad to hear it fixed the timeout issues you were having! Just one concern about infinite retries and a small nitpick but otherwise all good.

Closes #149 and #155 (lol that doesn't work in comments apparently, I've linked the issue manually instead.)

src/commands/prs.js Outdated Show resolved Hide resolved
src/lib/octokit.js Outdated Show resolved Hide resolved
src/commands/prs.js Show resolved Hide resolved
@ivomurrell ivomurrell linked an issue Nov 23, 2021 that may be closed by this pull request
@serena97 serena97 merged commit 3553e4d into main Nov 30, 2021
@serena97 serena97 deleted the CPP-642-nori-pr-improvement branch November 30, 2021 09:33
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.

Rate limit PR creation
2 participants