Cache etags #7107
Cache etags #7107
Conversation
Codecov Report
|
|
I'll add that I've gone through a bit and not seen that calling |
|
As @rtibbles pointed out - the original solution renders the view anyway. Ultimately Richard's suggestion is to remove the etag and to address optimization along these lines separately. So this PR has changed to be removing the etag altogether. |
nucleogenesis
changed the title
|
What is the effect of
Just noting that even if etags are regenerated on every request and render the full page, they still save network bandwidth by returning a |
nucleogenesis
force-pushed
the
etag-account-for-plugin-data
branch
from
33fb6b8
to
69cb1f8
Compare
kolibri/core/decorators.py
Outdated
| setattr(_response, "response", None) | ||
|
|
||
| request = args[0] | ||
| etag = cache.get(request.path) |
There was a problem hiding this comment.
Thought of one more thing in review:
I don't think it is huge likely, but might be good to prefix the request path with something to prevent accidental cache key collisions if something else is using the path:
So we can add this in the local scope:
CACHE_KEY_TEMPLATE = "SPA_ETAG_CACHE_{}"
Then to generate the cache key:
cache_key = CACHE_KEY_TEMPLATE.format(request.path)
There was a problem hiding this comment.
Missed using the cache key template here :)
Summary
After further discussion with @rtibbles this PR has changed to be removing the etag decorator we were looking at altogether.
-- below this is the original comment --
Updated the etag generation decorator function so that it also uses the rendered HTML which includes everything in
plugin_data.Is there a better or easier way to get
plugin_dataonly?I hate that I call the view_func twice and feel like that could be a problem - I'll come back to it.
Reviewer guidance
kolibri_plugin.pyfile for the plugin associated with the URL you're testing. Add a key to theplugin_dataReferences
#6785
Contributor Checklist
PR process:
Testing:
Reviewer Checklist
yarnandpip)