Re: [Django] #15201: CACHE_MIDDLEWARE_ANONYMOUS_ONLY is ugly, misleading, and unnecessary, and should be deprecated
Django
should be deprecated
--------------------------------------+------------------------------------
Reporter: carljm | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Cache system) | Version: SVN
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by carljm):
* ui_ux: => 0
* easy: => 0
Comment:
The use case I described above that would justify keeping
CACHE_MIDDLEWARE_ANONYMOUS_ONLY is bogus - by sending out anonymous
responses without Vary: Cookie you'd allow upstream caches to serve that
anonymous response to anyone, including logged-in staff users. Really
there is no circumstance where you can justify checking the user's logged-
in status without adding Vary: Cookie to the response.
--
Ticket URL: <https://code.djangoproject.com/ticket/15201#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
should be deprecated
--------------------------------------+------------------------------------
Reporter: carljm | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by aaugustin):
I hit this bug too. I only noticed because I have some randomized content
and it changed on every refresh instead of staying cached for some time.
In my case, the problem wasn't reproducible outside of the browser (eg.
with unit tests or with curl). The culprit was Google Analytics. It sets
`__utcma`, `__utcmb`, `__utcmc` and `__utcmz` cookies. Since Django's
cache engine honors `Vary: Cookie`, and my templates hide some parts for
anonymous users, this results in per-user cache instead of global cache.
This is a variant of point 2) in the report above.
I'm not familiar with tracking technology, but I believe cookies is the
most straightforward and compatible implementation. I expect this problem
to occur on any website that uses Google Analytics or a similar service.
In other words, `CACHE_MIDDLEWARE_ANONYMOUS_ONLY` most certainly
malfunctions silently for every non-trivial website, ''even if its
behavior is checked by unit tests''!
----
The feature we want here is "cache content for anonymous users, regardless
of Vary: Cookie". Obviously, it clashes with the current APIs, because it
means triggering HTTP-level caching based on application-level
information. And it works only under the assumption that, when a user is
anonymous, cookies aren't used for anything that affects the output
(besides determining that the user is anonymous).
This assumption cannot be checked automatically. It's something the
developer declares by turning `CACHE_MIDDLEWARE_ANONYMOUS_ONLY` on. For
instance, `{% if user.is_authenticated %}Welcome, {{ user }}!{% endif %}`
is enough to turn "Vary: Cookie" on, but the developer knows that it
results in the same output for all anonymous users. Often this holds true
for an entire site, and that's why `CACHE_MIDDLEWARE_ANONYMOUS_ONLY`
exists.
----
To sum up, the expected behavior is "ignore Vary: Cookie when caching
pages for anonymous users" (both for update and fetch).
I think that's a useful behavior, more useful that "cache pages that don't
have Vary: Cookie", because it's almost impossible not to have Vary:
Cookie.
If it cannot be implemented satisfactorily, then we should deprecate
`CACHE_MIDDLEWARE_ANONYMOUS_ONLY`.
----
Naive idea that I haven't tested:
- if the setting is on, the user is anonymous, and the Vary header
contains "Cookie"
- then remove "Cookie" from Vary, do the caching operations, and restore
"Cookie" in Vary.
--
Ticket URL: <https://code.djangoproject.com/ticket/15201#comment:5>
Django
should be deprecated
--------------------------------------+------------------------------------
Reporter: carljm | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by aaugustin):
My idea isn't going to play well with CSRF. `{% csrf_token %}` really
depends on the cookie.
Under the Don't Give Users Guns Aimed At Feet policy, I'm retracting it.
----
This leaves us with Carl's idea of "cache anything that doesn't have Vary:
Cookie" — but is it even useful?
Caching is hard!
--
Ticket URL: <https://code.djangoproject.com/ticket/15201#comment:6>
Django
should be deprecated
--------------------------------------+------------------------------------
Reporter: carljm | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by aaugustin):
#18863 was closed as duplicate.
--
Ticket URL: <https://code.djangoproject.com/ticket/15201#comment:7>
Django
should be deprecated
--------------------------------------+------------------------------------
Reporter: carljm | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* cc: raymond.penners@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15201#comment:8>
Django
should be deprecated
--------------------------------------+------------------------------------
Reporter: carljm | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by aaugustin):
Another argument for nuking `CACHE_MIDDLEWARE_ANONYMOUS_ONLY` is that it
creates a dependency of core on contrib (namely, django.contrib.auth). See
#8713.
--
Ticket URL: <https://code.djangoproject.com/ticket/15201#comment:9>
Django
should be deprecated
Reporter: carljm | Owner: anonymous
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Core (Cache system) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by anonymous):
* status: new => assigned
* owner: nobody => anonymous
--
Ticket URL: <https://code.djangoproject.com/ticket/15201#comment:10>
Django
should be deprecated
Reporter: carljm | Owner: ambv
Type: Cleanup/optimization | Status: assigned
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* owner: anonymous => ambv
--
Ticket URL: <https://code.djangoproject.com/ticket/15201#comment:11>
Django
should be deprecated
--------------------------------------+------------------------------------
Reporter: carljm | Owner: ambv
Type: Cleanup/optimization | Status: assigned
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by aaugustin):
After a quick discussion with ambv at the DjangoCon sprints, I'm also
leaning towards deprecating this setting and not providing a replacement.
--
Ticket URL: <https://code.djangoproject.com/ticket/15201#comment:12>
Django
should be deprecated
--------------------------------------+------------------------------------
Reporter: carljm | Owner: ambv
Type: Cleanup/optimization | Status: assigned
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/1114
This patch is on the right track. Two tests for
CACHE_MIDDLEWARE_ANONYMOUS_ONLY trigger warnings (`tests % PYTHONPATH=..
python2.6 -Wd runtests.py --settings=test_sqlite cache`). These warnings
should be silenced.
--
Ticket URL: <https://code.djangoproject.com/ticket/15201#comment:13>
Django
should be deprecated
--------------------------------------+------------------------------------
Reporter: carljm | Owner: ambv
Type: Cleanup/optimization | Status: assigned
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by ambv):
FTR, the PR is fixed, doesn't generate warnings anymore.
--
Ticket URL: <https://code.djangoproject.com/ticket/15201#comment:14>
Django
should be deprecated
Reporter: carljm | Owner: aaugustin
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Core (Cache system) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* owner: ambv => aaugustin
* needs_better_patch: 1 => 0
Comment:
I'm putting this on my review list. I may commit it only after the
sprints.
--
Ticket URL: <https://code.djangoproject.com/ticket/15201#comment:15>
Django
should be deprecated
-------------------------------------+-------------------------------------
Reporter: carljm | Owner: aaugustin
Cleanup/optimization | Version: master
Component: Core (Cache system) | Resolution: fixed
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
Merged in
https://github.com/django/django/commit/bd97f7d0cb72191744552142817184e88ce8841d
--
Ticket URL: <https://code.djangoproject.com/ticket/15201#comment:16>
Django
should be deprecated
-------------------------------------+-------------------------------------
Reporter: carljm | Owner: aaugustin
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Core (Cache system) | Resolution: fixed
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"f567d04b249913db4a37adab8ba521cdc974d423"]:
{{{
#!CommitTicketReference repository=""
revision="f567d04b249913db4a37adab8ba521cdc974d423"
Removed settings.CACHE_MIDDLEWARE_ANONYMOUS_ONLY per deprecation timeline.
refs #15201.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/15201#comment:17>