Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#14103 closed (fixed)

Have django.utils.cache.patch_response_headers honor settings.USE_ETAGS setting

Reported by: trbs Owned by: nobody
Component: Core (Cache system) Version: master
Severity: Keywords: use_etags, patch_response_headers
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

To be able to exactly tune when your site is setting ETags on request response objects it needs to be configurable for the Django caching system.
Since we already have the the settings.USE_ETAGS configuration setting this seems the logical choice :)

Documentation states "USE_ETAGS: Whether to use the "Etag" header." and does not specify that this is only valid for CommonMiddleware.

The 'offending' function is patch_response_headers which adds some useful headers to the response object; one of these is the ETag.

However is does not honor the settings.USE_ETAGS setting from the site configuration. This means you can see ETag headers in the responses while having ETag explicitly disabled.

Patch django_utils_cache_honor_use_etags.diff attached simple adds the check for settings.USE_ETAGS to patch_response_headers.

Second patch cache_honor_use_etags.diff does basically the same thing but introduces a new setting CACHE_MIDDLEWARE_USE_ETAGS
which defaults to settings.USE_ETAGS so it can be enabled/disabled separately to USE_ETAGS.

Attachments (4)

django_utils_cache_honor_use_etags.diff (631 bytes) - added by trbs 6 years ago.
cache_honor_use_etags.diff (1.1 KB) - added by trbs 6 years ago.
django_utils_cache_honor_use_etags_plus_test.diff (1.9 KB) - added by trbs 6 years ago.
14103_with_docs.diff (2.5 KB) - added by Eric Holscher 6 years ago.

Download all attachments as: .zip

Change History (10)

Changed 6 years ago by trbs

Changed 6 years ago by trbs

Attachment: cache_honor_use_etags.diff added

comment:1 Changed 6 years ago by Paul McMillan

Needs tests: set
Triage Stage: UnreviewedAccepted

Either of these patches look good to me, though I prefer the first slightly. This needs tests, then I think it's ready to go.

comment:2 Changed 6 years ago by trbs

added patch (based on the first 'simple' version) with tests

comment:3 Changed 6 years ago by trbs

Needs tests: unset

Changed 6 years ago by Eric Holscher

Attachment: 14103_with_docs.diff added

comment:4 Changed 6 years ago by Eric Holscher

Triage Stage: AcceptedReady for checkin

Added a patch that mentions this other usage in the documentation. I didn't see the etag header specifically mentioned in the caching middleware docs, but it might be worth adding a note there about the USE_ETAG setting as well. Otherwise I think this is RFC.

comment:5 Changed 6 years ago by Jannis Leidel

Resolution: fixed
Status: newclosed

(In [14885]) Fixed #14103 -- Take USE_ETAGS setting into account when patching the response headers. Thanks, trbs and Eric Holscher.

comment:6 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

Note: See TracTickets for help on using tickets.
Back to Top