good-first-issue
good-first-issue copied to clipboard
Handle rate limiting gracefully
When testing to make sure nothing breaks, I semi-often get rate-limited by GitHub. This is because there are (IIRC) 50 queries accepted before 1-hour rate limiting kicks in.
Obviously this shouldn't be happening for a normal user (though I'd applaud a user that was working on dozens of good-first-issues), but it may happen for contributors.
Checklist:
- [ ] Handle errors in the CLI gracefully
- [ ] Allow consumers of the module to pass GitHub authentication, which we would then pass to @octokit/rest
- [ ] Documentation for both of the above
Found this in the docs about rate limits:
The Search API has a custom rate limit. For requests using Basic Authentication, OAuth, or client ID and secret, you can make up to 30 requests per minute. For unauthenticated requests, the rate limit allows you to make up to 10 requests per minute.
https://developer.github.com/v3/search/#rate-limit
Also, not sure how much users will give GitHub credentials to avoid these limits. So not sure if it is worth adding an authentication functionality just for this? Otherwise I would like to give it a shot, found this article about a CLI application getting and storing a OAuth token etc.
I believe it's 5k/hour or so – I have experience with this from building out NodeKitten.
For implementation of this, I believe we can just accept an object and pass it directly to @octokit/rest.js, which is the lib we're using for GitHub. See the Authentication examples in their README.md for an example – in NodeKitten I used token authentication but we could just let the user decide how they want to authenticate by passing an object as @octokit/rest.js would expect.
Oops didn't mean to close 😁
I believe it's 5k/hour or so
if the rate is 5K :shamrock: I don't think we/any user at any point exceed this rate (except in case of bruting :wink: )
I think the search that we use is limited at a lower rate than normal requests. Have tried it out, and can hit the limit if i make a dozen requests in a minute.
For implementation of this, I believe we can just accept an object and pass it directly to @octokit/rest.js
If we don't store the authentication in a way, the user will have to login/authenticate each time we run? Don't think that is a fun UX if we ask that every time.
@kennethvandenberghe so this is more for when the user require()
s the module to use the output programmatically and less when the user is using it from the CLI.
IMO it's less needed on the CLI since I can't really think of a use case in which a user (not a contributor) is genuinely using the CLI to find a Good First Issue.
That said, we may want to add the ability for contributors to add a file locally like github-auth.json
that includes personal auth tokens so we have less of an issue when working on contributing? Perhaps that could be split out into a different issue.
- As for a module consumption scenario - we can add
auth token
as an option to it - and for the cli, I don't think a user might end up making hundreds to request/hour, hence no need in the CLI
So to conclude stuff here so i can start fixing this stuff finally, what is the goal specifically @bnb?
@kennethvandenberghe enable the user to pass an object to the module when calling it that we will pass directly to @octokit/rest.js
.
We don't need to be specific about what that object looks like, since @octokit/rest.js
already has mechanisms for that – we just need to pass that object to @octokit/rest.js
if it is provided.
Here's the examples from the @octokit/rest.js
README: https://github.com/octokit/rest.js#authentication