Disable preemptive rate limiting#2710
Disable preemptive rate limiting#2710kishaningithub wants to merge 1 commit intointegrations:mainfrom
Conversation
b65970a to
d87aee5
Compare
|
@kishaningithub upgrading the SDK version is going to be complicated as there are going to be a significant number of related changes required. |
|
I understand, would be great if we can simplify this and get it done Let me know your ideas |
|
@kishaningithub I've added a comment to your issue, we should probably have any discussions there. |
|
Waiting on this PR to be merged #2898 |
|
@kishaningithub We've now upgraded to the latest go-github version. If I'm understanding the change you are proposing, you are disabling the rate limit handling from go-github, without adding any handling to the provider. This doesn't sound feasible. As suggested in the issue, could you implement https://github.com/gofri/go-github-ratelimit? |
d87aee5 to
f87afe9
Compare
|
@deiga the provider is doing some rate limiting in the transport, but it's only ever been tested in coordination with the SDK rate limiting. @kishaningithub if we were to make this change there will be a lot of testing required, are you in a position to add the tests and run them? My preference is to disable both the SDK rate limiter and the current provider implementation and to replace them with gofri/go-github-ratelimit integrated with the resource timeouts (that we've been adding as part of the switch to context based functions). |
Resolves #2709
Before the change?
After the change?
Pull request checklist
Does this introduce a breaking change?
Please see our docs on breaking changes to help!