Middleware+Transactions:

[Thomas Güttler]

Dear Django developers,

we currently have the following issue when upgrading from Django 1.5 to Django 1.6:
https://github.com/etianen/django-reversion/issues/268

As it seems, since Django 1.6, middlewares are not supposed to be executed within the same transaction as the view
function is.

We would like to understand why that is the default behavior now and we are supposed to re-organize our code base to
work with that changes.

What is the rationale behind middlewares not being executed within the same transaction as the view function?

Furthermore, decorating all view functions as suggested here
http://django-reversion.readthedocs.org/en/latest/api.html#revisionmiddleware is quite not feasible for large projects.
Especially given that reversion is not the only middleware that needs to be executed within the same transaction as the
view function. It would basically mean that each view function needs to be decorated potentially n times.


We understand that transactions might effect performance, thus lesser transactions being in a transaction means faster
execution. However, shouldn't there be the possibility to include distinct middlewares into the view transaction?


Regards,
Thomas Güttler

--
Thomas Güttler
http://thomas-guettler.de/

[Aymeric Augustin]

Hi Thomas,

Both ways had advantages and drawbacks. Eventually I chose to include only the view function in the transaction for the following reasons.

1) Django never made any guarantees about which process_* methods of middleware would be called. Therefore every middleware must implemented defensively anyway, that is, leave a consistent state regardless of what methods are or aren't called.

2) The goal of ATOMIC_REQUESTS is to prevent inconsistent modifications of the database in views. Since middleware run for every view, they shouldn't introduce inconsistencies no matter what happens.

3) Most middleware don't touch the database. Database queries that run at every request aren't good for overall performance.

4) For lack of a better reason, keeping the database transactions short is more efficient.

1) and 2) really boil down to the fragility of how Django executes middleware. Not including middleware in transactions reduces the risk that developers will make incorrect assumptions. 3) and 4) don't apply to your use case.

I looked at how reversion is implemented but I don't understand it sufficiently well to give advice.

This isn't a satisfying answer but I hope it helps.

-- 

Aymeric.

--
You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
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/54D08DF5.8080606%40tbz-pariv.de.
For more options, visit https://groups.google.com/d/optout.

[Thomas Güttler]

Thank you very much for your fast reply.

Our team discussed the solutions we see. At the moment we see this upgrade:

- We stay with the older django version, and don't use atomic for the next months.
- We provide a patch for django to provide settings.MIDDLEWARES_INSIDE_TRANSACTION
- I hope django will include our patches in a newer version.
- If the new version of django is released we can upgrade our code.

Explanation:

settings.MIDDLEWARES_INSIDE_TRANSACTION=['mymodule.middlewares.HandleChangesOfTransaction', ...]

HandleChangesOfTransaction is a class which does some magic at the beginning and end of each transaction.

Can you understand what I wrote?

Will a patch to provide settings.MIDDLEWARES_INSIDE_TRANSACTION be accepted?

Regards,
Thomas Güttler

Am 03.02.2015 um 11:00 schrieb Aymeric Augustin:
> Hi Thomas,
>
> Both ways had advantages and drawbacks. Eventually I chose to include only the view function in the transaction for the
> following reasons.
>
> 1) Django never made any guarantees about which process_* methods of middleware would be called. Therefore every
> middleware must implemented defensively anyway, that is, leave a consistent state regardless of what methods are or
> aren't called.
>
> 2) The goal of ATOMIC_REQUESTS is to prevent inconsistent modifications of the database in views. Since middleware run
> for every view, they shouldn't introduce inconsistencies no matter what happens.
>
> 3) Most middleware don't touch the database. Database queries that run at every request aren't good for overall performance.
>
> 4) For lack of a better reason, keeping the database transactions short is more efficient.
>
> 1) and 2) really boil down to the fragility of how Django executes middleware. Not including middleware in transactions
> reduces the risk that developers will make incorrect assumptions. 3) and 4) don't apply to your use case.
>
> I looked at how reversion is implemented but I don't understand it sufficiently well to give advice.
>
> This isn't a satisfying answer but I hope it helps.

--

Thomas Güttler
http://thomas-guettler.de/

[Florian Apolloner]

On Wednesday, February 4, 2015 at 10:53:28 AM UTC+1, guettli wrote:

Will a patch to provide settings.MIDDLEWARES_INSIDE_TRANSACTION be accepted?

Most likely not since one of the reasons why @atomic isn't applied via a middleware (which would then include the middlewares after it) is that the way middlewares currently work don't allow the usage of atomic. That said if you have a working patch we'd certainly look at it.

Cheers,
Florian

[Anssi Kääriäinen]

I'd really like to be able to define middlewares that actually work in
a well defined and easy to use way. Currently, there is no
guarantee(!) that either process_exception or process_response gets
called after process_request has been called for given middleware, and
this makes it impossible to implement a transaction middleware purely
as a middleware.

An idea... Would investigating and implementing better middlewares (or
urlpattern wrappers) make for a good GSoC project? Maybe the "wrap
URLs" approach could be part of the URL resolve refactoring GSoC
project?

- Anssi

> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to django-develop...@googlegroups.com.
> To post to this group, send email to django-d...@googlegroups.com.

> 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/49f603d3-af82-48e7-845d-50f4b8e7a530%40googlegroups.com.

[Thomas Güttler]

Am 04.02.2015 um 14:04 schrieb Anssi Kääriäinen:
> I'd really like to be able to define middlewares that actually work in
> a well defined and easy to use way. Currently, there is no
> guarantee(!) that either process_exception or process_response gets
> called after process_request has been called for given middleware, and
> this makes it impossible to implement a transaction middleware purely
> as a middleware.

It's the same with TestCase.setUp() and TestCase.tearDown() does not work
well together with decorators.

You are right. Instead of

settings.MIDDLEWARES_INSIDE_TRANSACTION

settings.CONTEXT_MANAGERS

would be better.

The atomic() could be one entry in the list of context managers.

> An idea... Would investigating and implementing better middlewares (or
> urlpattern wrappers) make for a good GSoC project? Maybe the "wrap
> URLs" approach could be part of the URL resolve refactoring GSoC
> project?

I don't know if GSoC is a good solution. It is appealing, since
it looks that someone else does the work (not me and my team mates).
But I am a little bit afraid that will this result in a more complicated
implementation. My perfect solution are only very few lines of code.
I guess this could be possible. I guess the diff for the docs
could be longer then the diff for the code.

Regards,
Thomas

[Kevin Tran]

Thomas, did you ever find a solution to your problem?  I'm having similar thoughts and am looking for an answer.

[guettli]

Am Dienstag, 10. Mai 2016 02:07:30 UTC+2 schrieb Kevin Tran:

Thomas, did you ever find a solution to your problem?  I'm having similar thoughts and am looking for an answer.

Carl Meyer has worked out an enhancement proposal, here is the pull-request: https://github.com/django/django/pull/6501

[Florian Apolloner]

Starting with Django 1.10 you can write a TransactionMiddleware again, and we will probably ship one again.

[charettes]

As MIDDLEWARE supports decorator-like objects you could simply add `django.db.transaction.atomic' to it and you'd get each request wrapped in a transaction.

Note that this will only start a transaction on the `default` database, just like the old TransactionMiddleware use to do.

Simon