[Django] #25782: Discourage usage of cache_page decorator with UpdateCacheMiddleware (or make middleware ignore decorated views)

Django

unread,

Nov 19, 2015, 6:37:04 PM11/19/15

to django-...@googlegroups.com

#25782: Discourage usage of cache_page decorator with UpdateCacheMiddleware (or
make middleware ignore decorated views)
-------------------------------+--------------------
Reporter: int-ua | Owner: nobody
Type: Uncategorized | Status: new
Component: Documentation | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
If django.middleware.cache.UpdateCacheMiddleware is active and some view
is decorated with the cache_page that overrides cache alias, this view
will be cached in both caches, first time by cache_page and then by the
middleware. Well, at least it will try. And fail if the default cache is
memcached and the view response is bigger than its' maximum entry size.
While filebased cache will work and even create the cache entry. This way
first user to visit when there are no entry in cache will get error
instead of data.
So, I guess there should be some warning about this in the docs.
Ideally, middleware should ignore views with overridden cache alias, maybe
by means of checking headers like in case of the cache_control decorator.

--
Ticket URL: <https://code.djangoproject.com/ticket/25782>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Nov 19, 2015, 6:40:06 PM11/19/15

to django-...@googlegroups.com

#25782: Discourage usage of cache_page decorator with UpdateCacheMiddleware (or
make middleware ignore decorated views)

-------------------------------+--------------------------------------

Reporter: int-ua | Owner: nobody
Type: Uncategorized | Status: new
Component: Documentation | Version: master

Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------+--------------------------------------
Changes (by carljm):

* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0

Comment:

Any attempt to fix this should also take into account #15855 -- the
`cache_page` decorator is already basically broken in terms of Vary
headers. It shouldn't cache at all, it should mark the response for later
caching by the middleware, or something.

--
Ticket URL: <https://code.djangoproject.com/ticket/25782#comment:1>

Django

unread,

Nov 20, 2015, 7:24:30 AM11/20/15

to django-...@googlegroups.com

#25782: Discourage usage of cache_page decorator with UpdateCacheMiddleware (or
make middleware ignore decorated views)

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by timgraham):

* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/25782#comment:2>

Django

unread,

Mar 18, 2024, 3:22:47 AM3/18/24

to django-...@googlegroups.com

#25782: Discourage usage of cache_page decorator with UpdateCacheMiddleware (or
make middleware ignore decorated views)

--------------------------------------+------------------------------------
Reporter: Serhiy | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: dev

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 Ülgen Sarıkavak):

* cc: Ülgen Sarıkavak (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/25782#comment:3>

Django

unread,

Jun 27, 2024, 3:22:22 PM6/27/24

to django-...@googlegroups.com

#25782: Discourage usage of cache_page decorator with UpdateCacheMiddleware (or
make middleware ignore decorated views)

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by Wassef Ben Ahmed):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/25782#comment:4>

Django

unread,

Jun 27, 2024, 3:22:37 PM6/27/24

to django-...@googlegroups.com

#25782: Discourage usage of cache_page decorator with UpdateCacheMiddleware (or
make middleware ignore decorated views)

-------------------------------------+-------------------------------------
Reporter: Serhiy | Owner: Wassef
Type: | Ben Ahmed
Cleanup/optimization | Status: assigned

Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------

Changes (by Wassef Ben Ahmed):

* owner: nobody => Wassef Ben Ahmed
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/25782#comment:5>

Django

unread,

Jun 28, 2024, 3:15:44 AM6/28/24

to django-...@googlegroups.com

#25782: Discourage usage of cache_page decorator with UpdateCacheMiddleware (or
make middleware ignore decorated views)

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/25782#comment:6>

Django

unread,

Jul 9, 2024, 2:31:35 AM7/9/24

to django-...@googlegroups.com

#25782: Discourage usage of cache_page decorator with UpdateCacheMiddleware (or
make middleware ignore decorated views)

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Comment (by Sarah Boyce):

Although I am now happy with the patch, this might rely on #15855 to be
fixed and so further investigation is required in that ticket before
proceeding further here
--
Ticket URL: <https://code.djangoproject.com/ticket/25782#comment:7>

Django

unread,

Aug 1, 2024, 4:06:02 PM8/1/24

to django-...@googlegroups.com

#25782: Discourage usage of cache_page decorator with UpdateCacheMiddleware (or
make middleware ignore decorated views)

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Wassef Ben Ahmed):

* needs_better_patch: 1 => 0

Comment:

@sarahboyce, update is that I backed out from pushing what's mentioned
above.

I guess ticket-15855 will have to be worked on in a separate PR.

for now, I updated the release notes for the approved change.
and, Yes I confirm, UpdateCacheMiddleware still needs to be at the top for
it to handle undecorated views etc.
--
Ticket URL: <https://code.djangoproject.com/ticket/25782#comment:8>

Django

unread,

Aug 5, 2024, 7:50:43 AM8/5/24

to django-...@googlegroups.com

#25782: Discourage usage of cache_page decorator with UpdateCacheMiddleware (or
make middleware ignore decorated views)

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* needs_better_patch: 0 => 1

Comment:

Added minor comments on the PR
Before this can be marked as "Patch needs improvement" False, we need to
check that this change doesn't introduce issues due to #15855
--
Ticket URL: <https://code.djangoproject.com/ticket/25782#comment:9>

Django

unread,

Aug 24, 2024, 8:32:32 AM8/24/24

to django-...@googlegroups.com

#25782: Discourage usage of cache_page decorator with UpdateCacheMiddleware (or
make middleware ignore decorated views)

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Wassef Ben Ahmed):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/25782#comment:10>

Django

unread,

Aug 24, 2024, 8:32:55 AM8/24/24

to django-...@googlegroups.com

#25782: Discourage usage of cache_page decorator with UpdateCacheMiddleware (or
make middleware ignore decorated views)

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Comment (by Wassef Ben Ahmed):

Its not necessary to have UpdateCacheMiddleware on top, it's only a
workaround suggested by ticket-15855, but let's question it?
as without FetchFromCacheMiddleware, the UpdateCacheMiddleware will be
just setting the value to some cache with the correct headers but not
actually serving them as the value by this decorator will be the one
served... so the workaround just causes this problem and fixes nothing.

with this PR, will avoid that unnecessary extra write in the middleware,
and continue having “Vary headers“ missing.

on the other hand I think the "vary header" problem is smaller to have
compared to doubling cache size so we should proceed with this until
someone gathers the courage to deprecate the decorator?
--
Ticket URL: <https://code.djangoproject.com/ticket/25782#comment:11>

Django

unread,

Aug 30, 2024, 3:11:52 AM8/30/24

to django-...@googlegroups.com

#25782: Discourage usage of cache_page decorator with UpdateCacheMiddleware (or
make middleware ignore decorated views)

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/25782#comment:12>

Reply all

Reply to author

Forward