Opened 8 years ago

Closed 8 years ago

#26043 closed New feature (wontfix)

Add a hook to run queries between the start of a transaction and a view

Reported by: Sven R. Kunze Owned by: nobody
Component: HTTP handling Version: 1.8
Severity: Normal Keywords:
Cc: Florian Apolloner, tzanke@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Sven R. Kunze)

We are upgrading from Django 1.7 to Django 1.8. We noticed, we cannot perform things like:

connection.cursor().execute('SET LOCAL my_variable TO %d' % var)

in some of our Middlewares anymore due to the missing TransactionMiddleware.

We have more than 2,000 views (some of them created dynamically) , so decorating them manually is not a practical nor secure solution.

Currently, we patch BaseHandler.make_view_atomic to handle this but its weird that there is no default way to insert custom code in between starting a transaction and the actual execution of a view.

Change History (16)

comment:1 by Sven R. Kunze, 8 years ago

Description: modified (diff)

comment:2 by Simon Charette, 8 years ago

Resolution: invalid
Status: newclosed

Hi srkunze, TransactionMiddleware has been deprecated since Django 1.6 in favor of the ATOMIC_REQUESTS settings which should do exactly what you are expecting.

Please reopen if adding ATOMIC_REQUESTS: True to your DATABASES setting aliases doesn't work for your.

comment:3 by Aymeric Augustin, 8 years ago

Resolution: invalid
Status: closednew

I believe the transaction created by ATOMIC_REQUESTS only applies to the view, not to middleware.

As a consequence it may not solve the reporter's problem (but I'm not sure I understood it entirely).

comment:4 by Sven R. Kunze, 8 years ago

I believe the transaction created by ATOMIC_REQUESTS only applies to the view, not to middleware.

Exactly. That's why we monkey patched BaseHandler.make_view_atomic.

Basically like this:

def make_view_atomic(self, view):
    from django.db import transaction, connections
    non_atomic_requests = getattr(view, '_non_atomic_requests', set())
    for db in connections.all():
        if (db.settings_dict['ATOMIC_REQUESTS']
                and db.alias not in non_atomic_requests):
            view = transaction.atomic(using=db.alias)(yet_another_wrapper(db, view))
    return view

def yet_another_wrapper(db, view):
    def wrapped_view(request, *args, **kwargs):
        # do what legacy systems usually do
        # weird stuff
        # user-and-transaction-related code that always needs to run
        # custom things

        # actual view
        return view(request, *args, **kwargs)
    return wrapped_view

from django.core.handlers.base import BaseHandler
BaseHandler.make_view_atomic = make_view_atomic
Last edited 8 years ago by Sven R. Kunze (previous) (diff)

comment:5 by Tim Graham, 8 years ago

Summary: Missing Middlewares for Changing Transaction ParametersAdd a hook to run queries between the start of a transaction and a view
Triage Stage: UnreviewedAccepted
Type: BugNew feature

comment:6 by Sven R. Kunze, 8 years ago

@timgraham I changed the code example to pass db into the wrapper function.

Please, note we would need to execute arbitrary code (not just queries) between start of a transaction and all views.

Last edited 8 years ago by Sven R. Kunze (previous) (diff)

comment:7 by Thomas Güttler, 8 years ago

Just for the records. The same topic was discussed in February 2015: https://groups.google.com/forum/#!topic/django-developers/uW5Ogio8QBc

comment:8 by Aymeric Augustin, 8 years ago

At that point I would suggest to write your own transaction middleware that does:

self.atomic = transaction.atomic()
self.atomic.__enter__()

in the request phase and

self.atomic.__exit__(...)

in the response phase.

Django has gotten away with roughly the equivalent of this until version 1.6.

Sorry, I'm not proud of that...

comment:9 by Sven R. Kunze, 8 years ago

@aaugustin We'd like to use the ATOMIC_REQUESTS feature unbroken as is.

A simple hook, as timgraham suggested, would fit the bill.

comment:10 by Aymeric Augustin, 8 years ago

But that would increase complexity to an area that is already poorly designed (by my fault), thus making it even harder to refactor in the future :-/

comment:11 by Tim Graham, 8 years ago

If I understand correctly, the new middleware proposal would allow implementing a TransactionMiddleware with the same functionality as ATOMIC_REQUESTS with the benefit that you could also have other middleware to run queries like requested here, so that would solve this ticket.

comment:12 by Florian Apolloner, 8 years ago

@tim: Yes that was the main motivation of the middleware refactor. We should have TransactionMiddleware in 1.10 again.

comment:13 by Florian Apolloner, 8 years ago

Cc: Florian Apolloner added

comment:14 by Thomas Güttler, 8 years ago

The new "DEP 0005: Re-thinking middleware" would solve this issue:

..... It also allows more powerful idioms that aren't currently possible, like wrapping the call to get_response in a context manager (e.g. transaction.atomic) or in a try/finally block.

Source: https://github.com/django/deps/blob/master/draft/0005-improved-middleware.rst

A big thank you to Carl Meyer, the author of DEP 0005.

Last edited 8 years ago by Thomas Güttler (previous) (diff)

comment:15 by TZanke, 8 years ago

Cc: tzanke@… added

comment:16 by Tim Graham, 8 years ago

Resolution: wontfix
Status: newclosed

Closing in light of the middleware refactor.

Note: See TracTickets for help on using tickets.
Back to Top