Skip to content
This repository has been archived by the owner on Aug 26, 2022. It is now read-only.

bug 1431259: Don't cache the "create page" redirect #4731

Merged
merged 1 commit into from Apr 6, 2018

Conversation

jwhitlock
Copy link
Contributor

Use a "no-cache" cache control header when redirecting to create a page, to avoid a loop where you have to wait for the cache to expire to see the new page. This also updates the shared_cache_control decorator to take no action if it detects the cache control header is set to no-cache.

@jwhitlock jwhitlock requested a review from escattone April 6, 2018 18:41
@jwhitlock jwhitlock changed the title bug 1431259: Don't cache the "create page" redirectr bug 1431259: Don't cache the "create page" redirect Apr 6, 2018
Copy link
Contributor

@escattone escattone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than one requested change, this looks great, and I think it's a great idea! The first time the non-existent doc endpoint is hit, the redirect will not be cached. Only on the second and subsequent hits will we tell the CDN to cache the response. Thanks @jwhitlock!

def _cache_controlled(request, *args, **kw):
response = viewfunc(request, *args, **kw)
nocache = (response.has_header('Cache-Control') and
'no-cache' in response['Cache-Control'])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add

 and 'no-store' in response['Cache-Control']

as well, since either could be used independent of the other.

@escattone escattone self-assigned this Apr 6, 2018
Use a "no-cache" cache control header when redirecting to create a page,
to avoid a loop where you have to wait for the cache to expire to see
the new page. This also updates the shared_cache_control decorator to
take no action if it detects the cache control header is set to
no-cache.
@jwhitlock
Copy link
Contributor Author

Force-update to check for no-store as well

@escattone escattone merged commit 1251407 into mdn:master Apr 6, 2018
@jwhitlock jwhitlock deleted the keep-no-cache-1431259 branch April 6, 2018 20:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants