Opened 17 years ago

Closed 17 years ago

#5047 closed (fixed)

max-age set by cache_page is overwritten by cache_middleware

Reported by: Tomáš Kopeček Owned by: Jacob
Component: Core (Cache system) Version: dev
Severity: Keywords: cache_control, cache, middleware, sprintsept14
Cc: permonik@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When setting 'max-age' by @cache_page (more generally by patch_response_headers) is later overwritten by cache_middleware. In the documentation (http://www.djangoproject.com/documentation/cache/#controlling-cache-using-other-headers) is said that it should work. The problem is in order of calling function. Decorator is called before middleware's process_response. Simple patch against r5783 is attached, but it probably needs some design decision and folllowing rewrite.

Attachments (3)

patch_cache_control.patch (672 bytes ) - added by Tomáš Kopeček 17 years ago.
previous patch in patch format
patch_cache_controll.diff (1.5 KB ) - added by anonymous 17 years ago.
svn patch
max_age.diff (524 bytes ) - added by Tomáš Kopeček 17 years ago.
update

Download all attachments as: .zip

Change History (15)

by Tomáš Kopeček, 17 years ago

Attachment: patch_cache_control.patch added

previous patch in patch format

by anonymous, 17 years ago

Attachment: patch_cache_controll.diff added

svn patch

comment:1 by Tomáš Kopeček, 17 years ago

Owner: changed from nobody to Tomáš Kopeček
Status: newassigned

comment:2 by Tomáš Kopeček, 17 years ago

Triage Stage: UnreviewedAccepted

comment:3 by Tomáš Kopeček, 17 years ago

Triage Stage: AcceptedUnreviewed

comment:4 by Tomáš Kopeček, 17 years ago

Triage Stage: UnreviewedAccepted

comment:5 by Tomáš Kopeček, 17 years ago

Keywords: sprintsept14 added

comment:6 by Tomáš Kopeček, 17 years ago

Triage Stage: AcceptedReady for checkin

comment:7 by Jacob, 17 years ago

Triage Stage: Ready for checkinAccepted

I'm not sure this "rewrite" argument is needed -- shouldn't patch_cache_control() simply respect an existing max-age argument and not overrite it? Or perhaps set it to max(old, new)?

by Tomáš Kopeček, 17 years ago

Attachment: max_age.diff added

update

in reply to:  7 comment:8 by Tomáš Kopeček, 17 years ago

Replying to jacob:

I'm not sure this "rewrite" argument is needed -- shouldn't patch_cache_control() simply respect an existing max-age argument and not overrite it? Or perhaps set it to max(old, new)?

Yes, I look at the patch and it was really ineffective :-( So my change is to use minimum of these two values. It makes me more sense than maximum of them. I think that nearest cache invalidation time should be the effective one.

comment:9 by Tomáš Kopeček, 17 years ago

Triage Stage: AcceptedReady for checkin

comment:10 by Jacob, 17 years ago

Owner: changed from Tomáš Kopeček to Jacob
Status: assignednew

Yeah, that's looking more like what I had in mind -- thanks!

in reply to:  10 comment:11 by Tomáš Kopeček, 17 years ago

Replying to jacob:

Yeah, that's looking more like what I had in mind -- thanks!

I've thinked little bit more about it and here are (i think) typical usecases:

  • I don't have cache middleware. Everything is ok
  • I have cache middleware with some caching time e.g. 300 seconds
  • * I have some special view, which could be cached for one hour, I'll use @cache_page decorator with 60*60
  • * I have another special view which should be cached only for ten seconds. Again simplest way is to use decorator with parameter 10

In last two cases is important, that I need to _rewrite_ default behaviour. So, the first patch solves the problem in most expected way. Otherwise (with min value) will be always one of these cases unreachable.

comment:12 by Jacob, 17 years ago

Resolution: fixed
Status: newclosed

(In [6434]) Fixed #5047: patch_cache_control now respects existing max-age settings. Thanks, permon.

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