On Tuesday, October 15, 2013 8:23:38 AM UTC+3, [email protected] wrote: > > This topic was also discussed during the deprecation of > TransactionMiddleware and the introduction of ATOMIC_REQUESTS. The existing > middleware semantics can't guarantee that __exit__ (during > process_response) will get called no matter what, necessitating the setting > that invokes BaseHandler.make_view_atomic. make_view_atomic implements > basically what you're suggesting, for one specific case (DB/ORM code in the > front controller, ick!). > > I can't find the previous discussion, does anyone have a link? >
Some time ago I needed to fix a TransactionMiddleware bug. A connection's transaction state wasn't reset properly between requests. I remember being very frustrated about how current middleware implementation works. The basic problem was that if a middleware's process_* method throws an exception, there is no guarantee that process_exception() methods are called. Thus TransactionMiddleware didn't get to reset the transaction state in all cases, and this led to leaked transaction state. For more details see https://code.djangoproject.com/ticket/19707. A call-through middleware will guarantee that the semantics are easy to understand. Something like: def process(self, next, request): # The next parameter is an object that can be called. So, transaction middleware # would be basically this: with transaction.atomic(): return next(request) The next() call could end up calling the next middleware's process(), or if the current middleware is last in stack, then it would result in calling the resolved view method. What happens is abstracted away, so the middleware doesn't need to care. All it needs to know is that it can return next(request), any HttpResponse or raise an exception. I think the semantics of such a method would be better than what we currently offer. For example, this is how you could implement what process_request(), process_response() and process_exception() do, but in a more robust way: class CallProcessAll(object): def process(self, next, request): try: response = self.process_request(request) if response is None: response = next(request) except Exception as e: response = self.process_exception(request, e) if response is None: raise return self.process_response(request, response) I am not suggesting using the above process() example in Django. The semantics for process_exception() in particular are backwards incompatible. It is just an example that you can achieve what is currently available with process_request(), process_response() and process_exception() methods, but with obvious semantics. As said, the basic idea is to process the request by calling through the stack. It seems you can achieve the same with the suggested decorate_view() method. Wrap the view in decorate_view(), and to the same things you would do inside process() in the wrapper. But to me it seems that adds one step of indirection where it isn't needed. Also, a call-through middleware is conceptually easier to understand than a decorator (try-except-finally is basics, while decorators are a bit more advanced). Also, it isn't entirely clear what should happen in case decorate_view() itself raises an exception. - Anssi -- You received this message because you are subscribed to the Google Groups "Django developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/e44ae98e-255e-4cb7-b696-c3f89b679116%40googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
