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 )
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 , 10 years ago
| Description: | modified (diff) | 
|---|
comment:2 by , 10 years ago
| Resolution: | → invalid | 
|---|---|
| Status: | new → closed | 
comment:3 by , 10 years ago
| Resolution: | invalid | 
|---|---|
| Status: | closed → new | 
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 , 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
comment:5 by , 10 years ago
| Summary: | Missing Middlewares for Changing Transaction Parameters → Add a hook to run queries between the start of a transaction and a view | 
|---|---|
| Triage Stage: | Unreviewed → Accepted | 
| Type: | Bug → New feature | 
comment:6 by , 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.
comment:7 by , 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 , 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 , 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 , 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 , 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 , 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 , 10 years ago
| Cc: | added | 
|---|
comment:14 by , 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.
comment:15 by , 10 years ago
| Cc: | added | 
|---|
comment:16 by , 9 years ago
| Resolution: | → wontfix | 
|---|---|
| Status: | new → closed | 
Closing in light of the middleware refactor.
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: Trueto yourDATABASESsetting aliases doesn't work for your.