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
Don't update the ratelimits if we got a response from a cache #2273
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2273 +/- ##
=======================================
Coverage 97.83% 97.83%
=======================================
Files 115 115
Lines 10360 10363 +3
=======================================
+ Hits 10136 10139 +3
Misses 156 156
Partials 68 68
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @Jille !
Can you please add a unit test that demonstrates this change in functionality?
Also, once you sign the CLA, we can move forward with this PR.
The recommended cache, https://github.com/gregjones/httpcache, returns an X-From-Cache: 1 header whenever it serves a response from the cache. Cached responses might include (possibly stale) ratelimit headers, which we would've already processed in the past.
155f27f
to
2b58fd7
Compare
Thanks for the quick review :) Added tests, rebased to master, signed the CLA. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @Jille !
LGTM.
Awaiting second LGTM from any other contributor to this repo before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thank you, @raynigon ! |
The recommended cache, https://github.com/gregjones/httpcache, returns
an X-From-Cache: 1 header whenever it serves a response from the cache.
Cached responses might include (possibly stale) ratelimit headers, which
we would've already processed in the past.