Improved OkHttpConnector caching behavior#542
Merged
Conversation
Member
Author
|
I found this while doing some exploration of the OkHttpConnector with WireMock Snapshotting. Still need tests to show how this change effects query behavior. |
Contributor
|
For mocking better just disable cache |
040109c to
4b05953
Compare
Member
Author
|
@KostyaSha Tests aside from this one will not have caching enabled. |
4b05953 to
5e87d58
Compare
Member
Author
|
Turned off the tests for now. Caching is painful. |
Member
Author
|
I've removed the tests from CI, but left them in place for running locally. The data provided with the tests is accurate, but behavior changes when running from WireMock files. 😱 |
res0nance
approved these changes
Sep 24, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the Jenkins project we've seen a number of less than ideal behaviors by the OKHttp cache.
Some instances it seems bad data gets in and then gets stuck in the cache. In other cases it appears the cache obey the
max-age=60returned by GitHub and thus doesn't even look for new data from some requests.The cache behavior is fine for client-side uses such as Android where optimizing for network usage is important. Users of this library are are generally less concerned with network usage - they turn on caching to avoid using up their itHub rate-limit budget.
This change modifies the default OkHttpConnect behavior to make OkHttp always check the cache against the GitHub site. OkHttp still sends the response
ETagin the request which lets GitHub return a304 Not Modifiedif nothing has changed and does not count that request against the users rate-limit. OkHttp handles this response automatically and returns the cached data.This means that by using a bit more network bandwidth, this change guarantees up-to-date data while keeping the same rate-limit savings.
This change has no effect outside the scenario where OkHttpConnector is used with caching.
This is basically the same behavior added to Jenkins here.