From ac37ed21b3d66dde1748f6edf3279656b0267b70 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Wed, 6 Mar 2013 11:12:24 +0100 Subject: [PATCH] Deprecated TransactionMiddleware and TRANSACTIONS_MANAGED. Replaced them with per-database options, for proper multi-db support. Also toned down the recommendation to tie transactions to HTTP requests. Thanks Jeremy for sharing his experience. --- django/core/handlers/base.py | 12 ++- django/db/backends/__init__.py | 4 +- django/db/utils.py | 8 ++ django/middleware/transaction.py | 13 ++- docs/internals/deprecation.txt | 11 ++- docs/ref/middleware.txt | 4 + docs/ref/settings.txt | 30 +++++++ docs/releases/1.6.txt | 8 +- docs/topics/db/transactions.txt | 134 ++++++++++++++++++++++--------- tests/handlers/tests.py | 30 ++++++- tests/handlers/urls.py | 9 ++- tests/handlers/views.py | 17 ++++ 12 files changed, 223 insertions(+), 57 deletions(-) create mode 100644 tests/handlers/views.py diff --git a/django/core/handlers/base.py b/django/core/handlers/base.py index 0dcd9794c7198..5327ce58911c9 100644 --- a/django/core/handlers/base.py +++ b/django/core/handlers/base.py @@ -6,10 +6,10 @@ from django import http from django.conf import settings -from django.core import exceptions from django.core import urlresolvers from django.core import signals from django.core.exceptions import MiddlewareNotUsed, PermissionDenied +from django.db import connections, transaction from django.utils.encoding import force_text from django.utils.module_loading import import_by_path from django.utils import six @@ -65,6 +65,13 @@ def load_middleware(self): # as a flag for initialization being complete. self._request_middleware = request_middleware + def make_view_atomic(self, view): + if getattr(view, 'transactions_per_request', True): + for db in connections.all(): + if db.settings_dict['ATOMIC_REQUESTS']: + view = transaction.atomic(using=db.alias)(view) + return view + def get_response(self, request): "Returns an HttpResponse object for the given HttpRequest" try: @@ -101,8 +108,9 @@ def get_response(self, request): break if response is None: + wrapped_callback = self.make_view_atomic(callback) try: - response = callback(request, *callback_args, **callback_kwargs) + response = wrapped_callback(request, *callback_args, **callback_kwargs) except Exception as e: # If the view raised an exception, run it through exception # middleware, and if the exception middleware returns a diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 68551aad51f16..2cf75bd5285aa 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -104,7 +104,7 @@ def connect(self): conn_params = self.get_connection_params() self.connection = self.get_new_connection(conn_params) self.init_connection_state() - if not settings.TRANSACTIONS_MANAGED: + if self.settings_dict['AUTOCOMMIT']: self.set_autocommit() connection_created.send(sender=self.__class__, connection=self) @@ -299,7 +299,7 @@ def leave_transaction_management(self): if self.transaction_state: managed = self.transaction_state[-1] else: - managed = settings.TRANSACTIONS_MANAGED + managed = not self.settings_dict['AUTOCOMMIT'] if self._dirty: self.rollback() diff --git a/django/db/utils.py b/django/db/utils.py index 71b89f93fb5a6..936b42039d507 100644 --- a/django/db/utils.py +++ b/django/db/utils.py @@ -2,6 +2,7 @@ import os import pkgutil from threading import local +import warnings from django.conf import settings from django.core.exceptions import ImproperlyConfigured @@ -158,6 +159,13 @@ def ensure_defaults(self, alias): except KeyError: raise ConnectionDoesNotExist("The connection %s doesn't exist" % alias) + conn.setdefault('ATOMIC_REQUESTS', False) + if settings.TRANSACTIONS_MANAGED: + warnings.warn( + "TRANSACTIONS_MANAGED is deprecated. Use AUTOCOMMIT instead.", + PendingDeprecationWarning, stacklevel=2) + conn.setdefault('AUTOCOMMIT', False) + conn.setdefault('AUTOCOMMIT', True) conn.setdefault('ENGINE', 'django.db.backends.dummy') if conn['ENGINE'] == 'django.db.backends.' or not conn['ENGINE']: conn['ENGINE'] = 'django.db.backends.dummy' diff --git a/django/middleware/transaction.py b/django/middleware/transaction.py index 35f765d99fa76..95cc9a21f3bcc 100644 --- a/django/middleware/transaction.py +++ b/django/middleware/transaction.py @@ -1,4 +1,7 @@ -from django.db import transaction +import warnings + +from django.core.exceptions import MiddlewareNotUsed +from django.db import connection, transaction class TransactionMiddleware(object): """ @@ -7,6 +10,14 @@ class TransactionMiddleware(object): commit, the commit is done when a successful response is created. If an exception happens, the database is rolled back. """ + + def __init__(self): + warnings.warn( + "TransactionMiddleware is deprecated in favor of ATOMIC_REQUESTS.", + PendingDeprecationWarning, stacklevel=2) + if connection.settings_dict['ATOMIC_REQUESTS']: + raise MiddlewareNotUsed + def process_request(self, request): """Enters transaction management""" transaction.enter_transaction_management() diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 6c13af7ae41f7..19675801e43c6 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -329,9 +329,14 @@ these changes. 1.8 --- -* The decorators and context managers ``django.db.transaction.autocommit``, - ``commit_on_success`` and ``commit_manually`` will be removed. See - :ref:`transactions-upgrading-from-1.5`. +* The following transaction management APIs will be removed: + + - ``TransactionMiddleware``, + - the decorators and context managers ``autocommit``, ``commit_on_success``, + and ``commit_manually``, + - the ``TRANSACTIONS_MANAGED`` setting. + + Upgrade paths are described in :ref:`transactions-upgrading-from-1.5`. * The :ttag:`cycle` and :ttag:`firstof` template tags will auto-escape their arguments. In 1.6 and 1.7, this behavior is provided by the version of these diff --git a/docs/ref/middleware.txt b/docs/ref/middleware.txt index 1e6e57f720677..20bb2fb751bc8 100644 --- a/docs/ref/middleware.txt +++ b/docs/ref/middleware.txt @@ -205,6 +205,10 @@ Transaction middleware .. class:: TransactionMiddleware +.. versionchanged:: 1.6 + ``TransactionMiddleware`` is deprecated. The documentation of transactions + contains :ref:`upgrade instructions `. + Binds commit and rollback of the default database to the request/response phase. If a view function runs successfully, a commit is done. If it fails with an exception, a rollback is done. diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt index 0cd141bcef6a9..2b80527d8b0eb 100644 --- a/docs/ref/settings.txt +++ b/docs/ref/settings.txt @@ -408,6 +408,30 @@ SQLite. This can be configured using the following:: For other database backends, or more complex SQLite configurations, other options will be required. The following inner options are available. +.. setting:: DATABASE-ATOMIC_REQUESTS + +ATOMIC_REQUESTS +~~~~~~~~~~~~~~~ + +.. versionadded:: 1.6 + +Default: ``False`` + +Set this to ``True`` to wrap each HTTP request in a transaction on this +database. See :ref:`tying-transactions-to-http-requests`. + +.. setting:: DATABASE-AUTOCOMMIT + +AUTOCOMMIT +~~~~~~~~~~ + +.. versionadded:: 1.6 + +Default: ``True`` + +Set this to ``False`` if you want to :ref:`disable Django's transaction +management ` and implement your own. + .. setting:: DATABASE-ENGINE ENGINE @@ -1807,6 +1831,12 @@ to ensure your processes are running in the correct environment. TRANSACTIONS_MANAGED -------------------- +.. deprecated:: 1.6 + + This setting was deprecated because its name is very misleading. Use the + :setting:`AUTOCOMMIT ` key in :setting:`DATABASES` + entries instead. + Default: ``False`` Set this to ``True`` if you want to :ref:`disable Django's transaction diff --git a/docs/releases/1.6.txt b/docs/releases/1.6.txt index cc3bf94ef54dd..a1fe69229c071 100644 --- a/docs/releases/1.6.txt +++ b/docs/releases/1.6.txt @@ -262,9 +262,11 @@ Transaction management APIs Transaction management was completely overhauled in Django 1.6, and the current APIs are deprecated: -- :func:`django.db.transaction.autocommit` -- :func:`django.db.transaction.commit_on_success` -- :func:`django.db.transaction.commit_manually` +- ``django.middleware.transaction.TransactionMiddleware`` +- ``django.db.transaction.autocommit`` +- ``django.db.transaction.commit_on_success`` +- ``django.db.transaction.commit_manually`` +- the ``TRANSACTIONS_MANAGED`` setting The reasons for this change and the upgrade path are described in the :ref:`transactions documentation `. diff --git a/docs/topics/db/transactions.txt b/docs/topics/db/transactions.txt index 91b2cf41b32b2..37a369a02f85e 100644 --- a/docs/topics/db/transactions.txt +++ b/docs/topics/db/transactions.txt @@ -26,45 +26,61 @@ immediately committed to the database. :ref:`See below for details Previous version of Django featured :ref:`a more complicated default behavior `. +.. _tying-transactions-to-http-requests: + Tying transactions to HTTP requests ----------------------------------- -The recommended way to handle transactions in Web requests is to tie them to -the request and response phases via Django's ``TransactionMiddleware``. +A common way to handle transactions on the web is to wrap each request in a +transaction. Set :setting:`ATOMIC_REQUESTS ` to +``True`` in the configuration of each database for which you want to enable +this behavior. It works like this. When a request starts, Django starts a transaction. If the -response is produced without problems, Django commits any pending transactions. -If the view function produces an exception, Django rolls back any pending -transactions. - -To activate this feature, just add the ``TransactionMiddleware`` middleware to -your :setting:`MIDDLEWARE_CLASSES` setting:: - - MIDDLEWARE_CLASSES = ( - 'django.middleware.cache.UpdateCacheMiddleware', - 'django.contrib.sessions.middleware.SessionMiddleware', - 'django.middleware.common.CommonMiddleware', - 'django.middleware.transaction.TransactionMiddleware', - 'django.middleware.cache.FetchFromCacheMiddleware', - ) - -The order is quite important. The transaction middleware applies not only to -view functions, but also for all middleware modules that come after it. So if -you use the session middleware after the transaction middleware, session -creation will be part of the transaction. - -The various cache middlewares are an exception: ``CacheMiddleware``, -:class:`~django.middleware.cache.UpdateCacheMiddleware`, and -:class:`~django.middleware.cache.FetchFromCacheMiddleware` are never affected. -Even when using database caching, Django's cache backend uses its own database -connection internally. - -.. note:: - - The ``TransactionMiddleware`` only affects the database aliased - as "default" within your :setting:`DATABASES` setting. If you are using - multiple databases and want transaction control over databases other than - "default", you will need to write your own transaction middleware. +response is produced without problems, Django commits the transaction. If the +view function produces an exception, Django rolls back the transaction. +Middleware always runs outside of this transaction. + +You may perfom partial commits and rollbacks in your view code, typically with +the :func:`atomic` context manager. However, at the end of the view, either +all the changes will be committed, or none of them. + +To disable this behavior for a specific view, you must set the +``transactions_per_request`` attribute of the view function itself to +``False``, like this:: + + def my_view(request): + do_stuff() + my_view.transactions_per_request = False + +.. warning:: + + While the simplicity of this transaction model is appealing, it also makes it + inefficient when traffic increases. Opening a transaction for every view has + some overhead. The impact on performance depends on the query patterns of your + application and on how well your database handles locking. + +.. admonition:: Per-request transactions and streaming responses + + When a view returns a :class:`~django.http.StreamingHttpResponse`, reading + the contents of the response will often execute code to generate the + content. Since the view has already returned, such code runs outside of + the transaction. + + Generally speaking, it isn't advisable to write to the database while + generating a streaming response, since there's no sensible way to handle + errors after starting to send the response. + +In practice, this feature simply wraps every view function in the :func:`atomic` +decorator described below. + +Note that only the execution of your view in enclosed in the transactions. +Middleware run outside of the transaction, and so does the rendering of +template responses. + +.. versionchanged:: 1.6 + Django used to provide this feature via ``TransactionMiddleware``, which is + now deprecated. Controlling transactions explicitly ----------------------------------- @@ -283,18 +299,20 @@ if autocommit is off. Django will also refuse to turn autocommit off when an Deactivating transaction management ----------------------------------- -Control freaks can totally disable all transaction management by setting -:setting:`TRANSACTIONS_MANAGED` to ``True`` in the Django settings file. If -you do this, Django won't enable autocommit. You'll get the regular behavior -of the underlying database library. +You can totally disable Django's transaction management for a given database +by setting :setting:`AUTOCOMMIT ` to ``False`` in its +configuration. If you do this, Django won't enable autocommit, and won't +perform any commits. You'll get the regular behavior of the underlying +database library. This requires you to commit explicitly every transaction, even those started by Django or by third-party libraries. Thus, this is best used in situations where you want to run your own transaction-controlling middleware or do something really strange. -In almost all situations, you'll be better off using the default behavior, or -the transaction middleware, and only modify selected functions as needed. +.. versionchanged:: 1.6 + This used to be controlled by the ``TRANSACTIONS_MANAGED`` setting. + Database-specific notes ======================= @@ -459,6 +477,35 @@ atomicity of the outer block. API changes ----------- +Transaction middleware +~~~~~~~~~~~~~~~~~~~~~~ + +In Django 1.6, ``TransactionMiddleware`` is deprecated and replaced +:setting:`ATOMIC_REQUESTS `. While the general +behavior is the same, there are a few differences. + +With the transaction middleware, it was still possible to switch to autocommit +or to commit explicitly in a view. Since :func:`atomic` guarantees atomicity, +this isn't allowed any longer. + +To avoid wrapping a particular view in a transaction, instead of:: + + @transaction.autocommit + def my_view(request): + do_stuff() + +you must now use this pattern:: + + def my_view(request): + do_stuff() + my_view.transactions_per_request = False + +The transaction middleware applied not only to view functions, but also to +middleware modules that come after it. For instance, if you used the session +middleware after the transaction middleware, session creation was part of the +transaction. :setting:`ATOMIC_REQUESTS ` only +applies to the view itself. + Managing transactions ~~~~~~~~~~~~~~~~~~~~~ @@ -508,6 +555,13 @@ you should now use:: finally: transaction.set_autocommit(autocommit=False) +Disabling transaction management +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Instead of setting ``TRANSACTIONS_MANAGED = True``, set the ``AUTOCOMMIT`` key +to ``False`` in the configuration of each database, as explained in :ref +:`deactivate-transaction-management`. + Backwards incompatibilities --------------------------- diff --git a/tests/handlers/tests.py b/tests/handlers/tests.py index 6eb9bd23fe7c9..3680eecdd2f59 100644 --- a/tests/handlers/tests.py +++ b/tests/handlers/tests.py @@ -1,9 +1,8 @@ from django.core.handlers.wsgi import WSGIHandler from django.core.signals import request_started, request_finished -from django.db import close_old_connections -from django.test import RequestFactory, TestCase +from django.db import close_old_connections, connection +from django.test import RequestFactory, TestCase, TransactionTestCase from django.test.utils import override_settings -from django.utils import six class HandlerTests(TestCase): @@ -37,6 +36,31 @@ def test_bad_path_info(self): self.assertEqual(response.status_code, 400) +class TransactionsPerRequestTests(TransactionTestCase): + urls = 'handlers.urls' + + def test_no_transaction(self): + response = self.client.get('/in_transaction/') + self.assertContains(response, 'False') + + def test_auto_transaction(self): + old_atomic_requests = connection.settings_dict['ATOMIC_REQUESTS'] + try: + connection.settings_dict['ATOMIC_REQUESTS'] = True + response = self.client.get('/in_transaction/') + finally: + connection.settings_dict['ATOMIC_REQUESTS'] = old_atomic_requests + self.assertContains(response, 'True') + + def test_no_auto_transaction(self): + old_atomic_requests = connection.settings_dict['ATOMIC_REQUESTS'] + try: + connection.settings_dict['ATOMIC_REQUESTS'] = True + response = self.client.get('/not_in_transaction/') + finally: + connection.settings_dict['ATOMIC_REQUESTS'] = old_atomic_requests + self.assertContains(response, 'False') + class SignalsTests(TestCase): urls = 'handlers.urls' diff --git a/tests/handlers/urls.py b/tests/handlers/urls.py index 8570f04696090..29858055abd4a 100644 --- a/tests/handlers/urls.py +++ b/tests/handlers/urls.py @@ -1,9 +1,12 @@ from __future__ import unicode_literals from django.conf.urls import patterns, url -from django.http import HttpResponse, StreamingHttpResponse + +from . import views urlpatterns = patterns('', - url(r'^regular/$', lambda request: HttpResponse(b"regular content")), - url(r'^streaming/$', lambda request: StreamingHttpResponse([b"streaming", b" ", b"content"])), + url(r'^regular/$', views.regular), + url(r'^streaming/$', views.streaming), + url(r'^in_transaction/$', views.in_transaction), + url(r'^not_in_transaction/$', views.not_in_transaction), ) diff --git a/tests/handlers/views.py b/tests/handlers/views.py new file mode 100644 index 0000000000000..22d9ea4c7d820 --- /dev/null +++ b/tests/handlers/views.py @@ -0,0 +1,17 @@ +from __future__ import unicode_literals + +from django.db import connection +from django.http import HttpResponse, StreamingHttpResponse + +def regular(request): + return HttpResponse(b"regular content") + +def streaming(request): + return StreamingHttpResponse([b"streaming", b" ", b"content"]) + +def in_transaction(request): + return HttpResponse(str(connection.in_atomic_block)) + +def not_in_transaction(request): + return HttpResponse(str(connection.in_atomic_block)) +not_in_transaction.transactions_per_request = False