Opened 10 years ago

Closed 9 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, 10 years ago

Description: modified (diff)

comment:2 by Simon Charette, 10 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, 10 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, 10 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 10 years ago by Sven R. Kunze (previous) (diff)

comment:5 by Tim Graham, 10 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, 10 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 10 years ago by Sven R. Kunze (previous) (diff)

comment:7 by Thomas Güttler, 10 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, 10 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, 10 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, 10 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, 10 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, 10 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, 10 years ago

Cc: Florian Apolloner added

comment:14 by Thomas Güttler, 10 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 10 years ago by Thomas Güttler (previous) (diff)

comment:15 by TZanke, 10 years ago

Cc: tzanke@… added

comment:16 by Tim Graham, 9 years ago

Resolution: wontfix
Status: newclosed

Closing in light of the middleware refactor.

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