Howdy, I reported a bug earlier today (http://code.djangoproject.com/ticket/ 15144) related to a possible regression in changeset 15021 (http:// code.djangoproject.com/changeset/15021). The apparent regression is that as of this changeset, the cache middleware no longer respects the timeout value passed to the @cache_page decorator.
I found some time this evening to further research this. I was able to create a regression test that demonstrated the bug. However, in trying to create a patch which addresses the bug I'm having a hard time making sense of what changeset 15021 set out to accomplish. For whatever reason, I am unable to find any discussion related to it on this discussion list or in trac. If someone would be kind enough to direct me to discussion surrounding this changeset or the ticket(s) associated with it, I would be most appreciative. Anyhow, that said, it appears to me there are some minor issues with the code in the __init__ method of CacheMiddleware and there does not appear to be adequate test coverage around some of the code. Namely: * The cache_timeout value is being passed along to the cache, but self.cache_timeout is not being set. This is the problem I identified in the ticket I filed. The value was previously being set prior to changeset 15021. Why was this removed and why is it now being set to the default_timeout of the cache? * There appears to be an attempt to determine which alias and prefix to use in the try / except blocks. The problem is that the key is being accessed via a dict get() method, which will never raise a KeyError (http://docs.python.org/library/stdtypes.html#dict.get), it will only return None. So basically the except blocks are never going to be raised. This could probably be resolved by passing a default value of e.g. False as the second arg to get() in each instance, and then changing the try/except block to a condition. It also does seem like some tests are needed here to properly run these various code paths and make sure they are being set correctly. * It seems kind of weird to me that if a 'cache_alias' is set explicitly, then we don't fall back to CACHE_MIDDLEWARE_SECONDS if a cache_timeout is not found, but if it's not set then we do fall back to that setting? If someone can shed some light on these questions, I can hopefully set aside some time to write a few tests and rework the code a bit if it does turn out that is needed. Many thanks... -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
