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

Rate limit generation of PRs #25

Open
rnorth opened this issue Apr 19, 2021 · 1 comment
Open

Rate limit generation of PRs #25

rnorth opened this issue Apr 19, 2021 · 1 comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed pre mortem

Comments

@rnorth
Copy link
Collaborator

rnorth commented Apr 19, 2021

Pushing to git and PR creation can both cause CI workflows to start (both are part of the create-prs subcommand). Particularly in an enterprise context, this could put pressure on shared infrastructure.

We should put in place reasonable measures to prevent this, e.g. one or more of:

  • opt-in rate limiting (e.g. a fixed pause between creation)
  • automatic rate limiting of PR creation when a certain number of repos is involved.
  • using check status (number of PRs with pending checks) as a signal to slow down rate of creation. This would be a fairly involved feature, which may be too much for an initial implementation.
@rnorth rnorth added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels Apr 19, 2021
@rnorth
Copy link
Collaborator Author

rnorth commented Apr 22, 2021

An implementation idea for this which might be easy to use would be:

  • Use something like https://pkg.go.dev/golang.org/x/time/rate to have a token bucket rate limiter
  • Before creating each PR, check the rate limiter to decide whether it's necessary to wait
  • If waiting, display a suitable message to the user and give them the option to proceed right away by hitting a key

This would mean that we could have a default always-on rate limiter that can be overridden per-repo by the user if they know it's safe to move quickly. This would reduce the risk of people habitually disabling the rate limiter via config/flags.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed pre mortem
Projects
None yet
Development

No branches or pull requests

2 participants