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)
Change History (15)
by , 17 years ago
| Attachment: | patch_cache_control.patch added |
|---|
comment:1 by , 17 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:2 by , 17 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:3 by , 17 years ago
| Triage Stage: | Accepted → Unreviewed |
|---|
comment:4 by , 17 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:5 by , 17 years ago
| Keywords: | sprintsept14 added |
|---|
comment:6 by , 17 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
follow-up: 8 comment:7 by , 17 years ago
| Triage Stage: | Ready for checkin → Accepted |
|---|
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)?
comment:8 by , 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 , 17 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
follow-up: 11 comment:10 by , 17 years ago
| Owner: | changed from to |
|---|---|
| Status: | assigned → new |
Yeah, that's looking more like what I had in mind -- thanks!
comment:11 by , 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 , 17 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
previous patch in patch format