Opened 9 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 , 9 years ago
Description: | modified (diff) |
---|
comment:2 by , 9 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:3 by , 9 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 , 9 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): 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(view)) return view def yet_another_wrapper(view): def wrapped_view(request, *args, **kwargs): # do what legacy systems usually do # weird stuff # custom things return view(request, *args, **kwargs) return wrapped_view
comment:5 by , 9 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 , 9 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 , 9 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 , 9 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 , 9 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 , 9 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 , 9 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 , 9 years ago
@tim: Yes that was the main motivation of the middleware refactor. We should have TransactionMiddleware
in 1.10 again.
comment:13 by , 9 years ago
Cc: | added |
---|
comment:14 by , 9 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 , 9 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: True
to yourDATABASES
setting aliases doesn't work for your.