Cache 404 pages per-version #6797
Conversation
readthedocs/proxito/views/serve.py
Outdated
| @@ -225,7 +279,9 @@ def get(self, request, proxito_path, template_name='404.html'): | |||
|
|
|||
| # TODO: decide if we need to check for infinite redirect here | |||
| # (from URL == to URL) | |||
| return HttpResponseRedirect(redirect_url) | |||
| response = HttpResponseRedirect(redirect_url) | |||
| cache.set('dirhtml_404+{proxito_path}', response, 60) | |||
There was a problem hiding this comment.
Pretty sure we can't cache the object itself. We need to cache some string representation of it, then recreate the Python object.
There was a problem hiding this comment.
I tested this locally and it worked properly. I didn't find anything about HttpResponse not being serializable, but I'll take a deeper look for potential issues.
There was a problem hiding this comment.
Django's internals do the same: https://github.com/django/django/blob/4ed534758cb6a11df9f49baddecca5a6cdda9311/django/middleware/cache.py#L116
Although, that method check for some security concerns before caching the response, like Cache-Control: private, Cookie and caches only 200 and 304.
I don't think we need anyone of those in community at least. For commercial, I'm not sure yet, but I will think about.
Caches are our most expensive requests since at this point we check for multiple dirhtml files in the storage, for redirects, for a custom 404 page and we finally render the default Read the Docs' 404 page (Maze). This new code checks for these scenarios (dirhtml, redirect, custom, default) and return the cached response if it exists before performing any other operation. The default 404 page response is cached globally since it's the same for all the projects. Although, before returning it, we check that was the response we served the first time and not the custom 404 one.
humitos
force-pushed
the
humitos/cache-404-pages
branch
from
bf74e76 to
9a55553
Compare
| @@ -34,6 +35,7 @@ def proxito_404_page_handler(request, exception=None, template_name='404.html'): | |||
|
|
|||
| resp = render(request, template_name) | |||
| resp.status_code = 404 | |||
| cache.set('default_404_page', resp, 60) | |||
There was a problem hiding this comment.
This time should probably be bigger than 60 seconds.
|
@ericholscher do we want to explore a little more on this or should we close it for now? I think it's a nice to have, but we haven't hit this issue of 404 pages in a while now. |
|
Yea, I think we can close this for now since we're behind the CDN with autoscaling, it's much less of an issue 👍 |
Caches are our most expensive requests since at this point we check
for multiple dirhtml files in the storage, for redirects, for a custom
404 page and we finally render the default Read the Docs' 404
page (Maze).
This new code checks for these scenarios (dirhtml, redirect, custom,
default) and return the cached response if it exists before performing
any other operation.
The default 404 page response is cached globally since it's the same
for all the projects. Although, before returning it, we check that was
the response we served the first time and not the custom 404 one.