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 limiting issue #51

Closed
MalloZup opened this issue Sep 29, 2017 · 8 comments
Closed

rate limiting issue #51

MalloZup opened this issue Sep 29, 2017 · 8 comments

Comments

@MalloZup
Copy link
Contributor

MalloZup commented Sep 29, 2017

when we are fetching to much for new changes on PRs

We need to find a longterm solution for rate-limiting problem.

@MalloZup MalloZup changed the title rate limiting issue when we are fetching to much for new changes on PRs rate limiting issue Sep 29, 2017
@MalloZup
Copy link
Contributor Author

@MalloZup
Copy link
Contributor Author

@MalloZup
Copy link
Contributor Author

pulls = Octokit.pulls 'rails/rails';
etag = Octokit.last_response.headers['etag']
# => "\"deda972870491aaa85feb17b6987a56d\""

pulls = Octokit.pulls 'rails/rails', :headers => {'If-None-Match' => etag}
# => [#<Sawyer::Resource:0x007fa51fa50660 ... ]

Octokit.last_response.headers['status']
# => "200 OK"

@MalloZup
Copy link
Contributor Author

@MalloZup
Copy link
Contributor Author

@juliogonzalez i think in longterm this will be the solution:

http://kgrz.io/http-request-response-caching-using-faraday-part-1.html
(since it even supported and suggested by the octokit.rb library).

I will try to research and read about it, in short term your pr can be a solution

@juliogonzalez
Copy link
Member

juliogonzalez commented Sep 29, 2017

Well, I saw faraday, but what I don't like about the solution is that, as far as I understand, it requires a new service to be configured or at least persistent storage, or keeping gitarro running all the time (in the end you need to save the cache information somewhere).

If that is the case, it means it will be harder to integrate gitarro with other tools, since it won't be standalone anymore (in the worst case you will need it to keep it running all the time).

Shouldn't we go just for the conditional requests?

For example, we can use conditional search for commits and comments, so if a commit was changed we don't need to scan the comments unless there are new as well (just to remove the magic sentence if present, but in my opinion we should NOT remove the comments automatically as we do).

Just by doing this, we'll stop making a lot of requests.

In the end my PR is not the complete solution, but we should keep it in the future.

We do not want to run anything on PRs that do not have new comments (in this case we need to analyze to see if the magic sentence is present) or new commits.

And of course, next step should be having gitbot being able to run more than one test in one pass if needed, so we would have one job with a trigger calling one job running all the tests in a single call and updating all those needed.

This way you can analyze the comments in one call, the commits in one call, etc...

@MalloZup
Copy link
Contributor Author

Yop the same thing i was thinking is that we need a faraday storage, and i agree with your considerations.

I think that conditional request are the best solution atm.

@MalloZup MalloZup mentioned this issue Oct 5, 2017
3 tasks
@MalloZup
Copy link
Contributor Author

MalloZup commented Oct 9, 2017

closing, fixed by 2 prs merged

@MalloZup MalloZup closed this as completed Oct 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants