On 29/09/11 17:13, Alex Gaynor wrote: > Hi Luke (and the rest of the list ;)), > > Just saw r16912 and the subsequent commits, I wonder if you > saw https://code.djangoproject.com/ticket/14270? I think most of the > changes in that patch and that you committed are the same, (which really > just paves the way for the caching I was going at).
No, I wasn't aware of that ticket. I was just going for general tidy up, as I'm toying with various ideas related to the prefetch_related() proposal I posted earlier, and for my own analysis (and others) it made sense to make them a bit tidier first. I think caching the related manager makes sense though. It is the usual speed/memory trade-off I guess. One nit about the tests: self.assertTrue(e1.topics.__class__ is e1.topics.__class__) self.assertTrue(e2.topics.__class__ is e2.topics.__class__) I think of these should be: self.assertTrue(e1.topics.__class__ is e2.topics.__class__) to check that the caching is done against the Entry class, not just per instance (which I presume is the desired behaviour, it certainly is the actual behaviour). BTW, in the prefetch_related() thread, did you see my request for how you managed similar functionality outside of core? I've also just realised that this change might make a non-core solution easier, as you can more easily monkey-patch the related manager - MyModel.related_set.related_manager_cls gives you a constant. Luke -- 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.
