Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
If you enable the debug functionality, the pages must not be cached in internal page cache and external caches like varnish, boost, ...
Comment | File | Size | Author |
---|---|---|---|
#18 | avoid_caching_debug_output-1990098-18.patch | 614 bytes | dpovshed |
avoid_caching_debug_output.patch | 591 bytes | mkalkbrenner |
Comments
Comment #1
SpleshkaHi, I've got some comments to your patch:
I like this part. We should implement it.
I don't understand this part. The first part should be enough to avoid caching of this page.
Comment #2
SpleshkaComment #3
mkalkbrennerThe second part is only relevant for some rare conditions where a very aggressive caching is done by external caches.
So I think the comment should be changed because it's a good behavior to indicate that the content of a page is not "correct":
Comment #4
SpleshkaI know about 503 header, but what users will see?
I suggest to use this headers instead:
Comment #5
mkalkbrennerThe page still works! Try it ...
Again, it's not really about caching prevention but pointing out that the content is not the "real" one.
I still think you should apply the initial patch (with a modified comment). I do the same in my ThemeKey module.
BTW great module. So far it works well on our servers :-)
Comment #6
SpleshkaOk, I'm going to take some time to test your solution.
BWT I'm glad that module works for you. In the nearest future I'll add sessions support.
Comment #7
SpleshkaI found a better way to do this. See http://drupal.org/node/914486.
Comment #8
SpleshkaThis solution seemed to be final:
Comment #9
dpovshed CreditAttribution: dpovshed commented#8 looks for me as a correct and polite way of doing things.
Comment #10
mkalkbrennerFor caching prevention I'm fine with #8.
But again, debugging a site by adding debug output to the content is "maintenance of the server". The correct HTTP response code in that case is "503 Service unavailable". Using that response code you "let the client know that it can retry the request later". (Quotes from the spec)
4xx and 5xx status codes don't prevent browsers from rendering a page!
So my final recommendation is:
Comment #11
SpleshkaI've commited this but without 503 header. I'll keep your solution in mind and see if it will have sense on some projects. For now I don't think that this is required.
Thanks for your patch :)
Comment #12
dpovshed CreditAttribution: dpovshed commented@mkalkbrenner, IMHO regarding HTTP codes the 10+ years old RFC 2616 is still the most authoritative spec, chapter 10.5.4:
This is exactly the case you proposed in #10.
And 500 is "500 Internal Server Error: The server encountered an unexpected condition which prevented it from fulfilling the request."
So I agree with Spleshka about not including this in current code.
Comment #13
andypostI think the issue should follow core's headers which 503 how!
Related #1032936-13: Maintenance mode should neither create nor retrieve cache should be discussed and fixed and have test a for headers in maintenance mode then back to the issue
Comment #14
mkalkbrennerJust some more links
http://wordpress.org/support/topic/plugin-wp-maintenance-mode-does-it-se...
http://drupal.org/node/1044786
https://pypi.python.org/pypi/django-maintenancemode
BTW adding a Retry Header for 10 Minutes might be a good solution.
Comment #15
SpleshkaThanks all for the information, was good to know. May be it is a really good idea to return a 503 if debug mode is enabled. But this is still confusing me. I need a little bit more opinions before commit this in module.
Comment #16
dpovshed CreditAttribution: dpovshed commentedThank you @mkalkbrenner for sharing these links!
"BTW adding a Retry Header for 10 Minutes might be a good solution."
Yep, looks like that! Then Drupal will both comply with modern tendencies and respectful to the standards.
Let us look how it will go with core issue quoted by @andypost.
Comment #17
SpleshkaSo does anyone wants to provide a patch for that? It seems to me that this is a low-hanging fruit :)
Comment #18
dpovshed CreditAttribution: dpovshed commentedSpleshka, here it is for you.
Not sure - shall I change issue status or we depend here on core's 1032936 so this is something up to you.
Comment #19
SpleshkaThanks, commited/pushed to 7.x-1.x.