Skip to content
This repository was archived by the owner on Nov 16, 2023. It is now read-only.

add response caching#276

Merged
eads merged 29 commits intomasterfrom
0121-response-caching
Sep 24, 2016
Merged

add response caching#276
eads merged 29 commits intomasterfrom
0121-response-caching

Conversation

@eads
Copy link
Copy Markdown
Contributor

@eads eads commented May 16, 2016

Opening this up as a work-in-progress. Nearly done though.

@eads
Copy link
Copy Markdown
Contributor Author

eads commented Sep 6, 2016

Latest update on this: I have a PR in that fixes the bug in Cachecontrol that wasn't playing nice with AP's response headers; tests were failing due to some strange side effect, but a minor change seems to have fixed it.

Next step is to reconcile this branch with master and get Cachecontrol's maintainer to roll a new release.

@eads
Copy link
Copy Markdown
Contributor Author

eads commented Sep 6, 2016

This should not be merged until CacheControl is updated despite passing tests.

@eads
Copy link
Copy Markdown
Contributor Author

eads commented Sep 6, 2016

I guess I should mention why :)

The old version of CacheControl (still specified in requirements.txt until @ionrock makes a new release) has a small bug that causes 304 and 301 responses to skip the entire caching mechanism rather than running the specified heuristic (etag, in our case, per the AP's cache headers).

Because of the nature of the bug, it won't hurt anything per se, but it also won't cache anything. So we should wait and rerun the tests when the new CacheControl comes out.

@eads eads merged commit 99c2c64 into master Sep 24, 2016
@eads eads deleted the 0121-response-caching branch September 24, 2016 04:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants