Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Deprecated transaction.commit/rollback_unless_managed.

Since "unless managed" now means "if database-level autocommit",
committing or rolling back doesn't have any effect.

Restored transactional integrity in a few places that relied on
automatically-started transactions with a transitory API.
  • Loading branch information...
commit ba5138b1c0253fcf390b7509ad7b954117b3be88 1 parent 14aa563
@aaugustin aaugustin authored
View
4 django/contrib/gis/utils/layermapping.py
@@ -555,10 +555,6 @@ def _save(feat_range=default_range, num_feat=0, num_saved=0):
except SystemExit:
raise
except Exception as msg:
- if self.transaction_mode == 'autocommit':
- # Rolling back the transaction so that other model saves
- # will work.
- transaction.rollback_unless_managed()
if strict:
# Bailing out if the `strict` keyword is set.
if not silent:
View
1  django/contrib/sessions/backends/db.py
@@ -74,7 +74,6 @@ def delete(self, session_key=None):
@classmethod
def clear_expired(cls):
Session.objects.filter(expire_date__lt=timezone.now()).delete()
- transaction.commit_unless_managed()
# At bottom to avoid circular import
View
7 django/core/cache/backends/db.py
@@ -10,7 +10,7 @@
from django.conf import settings
from django.core.cache.backends.base import BaseCache
-from django.db import connections, router, transaction, DatabaseError
+from django.db import connections, router, DatabaseError
from django.utils import timezone, six
from django.utils.encoding import force_bytes
@@ -70,7 +70,6 @@ def get(self, key, default=None, version=None):
cursor = connections[db].cursor()
cursor.execute("DELETE FROM %s "
"WHERE cache_key = %%s" % table, [key])
- transaction.commit_unless_managed(using=db)
return default
value = connections[db].ops.process_clob(row[1])
return pickle.loads(base64.b64decode(force_bytes(value)))
@@ -124,10 +123,8 @@ def _base_set(self, mode, key, value, timeout=None):
[key, b64encoded, connections[db].ops.value_to_db_datetime(exp)])
except DatabaseError:
# To be threadsafe, updates/inserts are allowed to fail silently
- transaction.rollback_unless_managed(using=db)
return False
else:
- transaction.commit_unless_managed(using=db)
return True
def delete(self, key, version=None):
@@ -139,7 +136,6 @@ def delete(self, key, version=None):
cursor = connections[db].cursor()
cursor.execute("DELETE FROM %s WHERE cache_key = %%s" % table, [key])
- transaction.commit_unless_managed(using=db)
def has_key(self, key, version=None):
key = self.make_key(key, version=version)
@@ -184,7 +180,6 @@ def clear(self):
table = connections[db].ops.quote_name(self._table)
cursor = connections[db].cursor()
cursor.execute('DELETE FROM %s' % table)
- transaction.commit_unless_managed(using=db)
# For backwards compatibility
class CacheClass(DatabaseCache):
View
21 django/core/management/commands/createcachetable.py
@@ -53,14 +53,13 @@ def handle_label(self, tablename, **options):
for i, line in enumerate(table_output):
full_statement.append(' %s%s' % (line, i < len(table_output)-1 and ',' or ''))
full_statement.append(');')
- curs = connection.cursor()
- try:
- curs.execute("\n".join(full_statement))
- except DatabaseError as e:
- transaction.rollback_unless_managed(using=db)
- raise CommandError(
- "Cache table '%s' could not be created.\nThe error was: %s." %
- (tablename, force_text(e)))
- for statement in index_output:
- curs.execute(statement)
- transaction.commit_unless_managed(using=db)
+ with transaction.commit_on_success_unless_managed():
+ curs = connection.cursor()
+ try:
+ curs.execute("\n".join(full_statement))
+ except DatabaseError as e:
+ raise CommandError(
+ "Cache table '%s' could not be created.\nThe error was: %s." %
+ (tablename, force_text(e)))
+ for statement in index_output:
+ curs.execute(statement)
View
9 django/core/management/commands/flush.py
@@ -57,18 +57,17 @@ def handle_noargs(self, **options):
if confirm == 'yes':
try:
- cursor = connection.cursor()
- for sql in sql_list:
- cursor.execute(sql)
+ with transaction.commit_on_success_unless_managed():
+ cursor = connection.cursor()
+ for sql in sql_list:
+ cursor.execute(sql)
except Exception as e:
- transaction.rollback_unless_managed(using=db)
raise CommandError("""Database %s couldn't be flushed. Possible reasons:
* The database isn't running or isn't configured correctly.
* At least one of the expected database tables doesn't exist.
* The SQL was invalid.
Hint: Look at the output of 'django-admin.py sqlflush'. That's the SQL this command wasn't able to run.
The full error: %s""" % (connection.settings_dict['NAME'], e))
- transaction.commit_unless_managed(using=db)
# Emit the post sync signal. This allows individual
# applications to respond as if the database had been
View
1  django/core/management/commands/loaddata.py
@@ -73,7 +73,6 @@ def handle(self, *fixture_labels, **options):
# Start transaction management. All fixtures are installed in a
# single transaction to ensure that all references are resolved.
if commit:
- transaction.commit_unless_managed(using=self.using)
transaction.enter_transaction_management(using=self.using)
class SingleZipReader(zipfile.ZipFile):
View
77 django/core/management/commands/syncdb.py
@@ -83,26 +83,25 @@ def model_installed(model):
# Create the tables for each model
if verbosity >= 1:
self.stdout.write("Creating tables ...\n")
- for app_name, model_list in manifest.items():
- for model in model_list:
- # Create the model's database table, if it doesn't already exist.
- if verbosity >= 3:
- self.stdout.write("Processing %s.%s model\n" % (app_name, model._meta.object_name))
- sql, references = connection.creation.sql_create_model(model, self.style, seen_models)
- seen_models.add(model)
- created_models.add(model)
- for refto, refs in references.items():
- pending_references.setdefault(refto, []).extend(refs)
- if refto in seen_models:
- sql.extend(connection.creation.sql_for_pending_references(refto, self.style, pending_references))
- sql.extend(connection.creation.sql_for_pending_references(model, self.style, pending_references))
- if verbosity >= 1 and sql:
- self.stdout.write("Creating table %s\n" % model._meta.db_table)
- for statement in sql:
- cursor.execute(statement)
- tables.append(connection.introspection.table_name_converter(model._meta.db_table))
-
- transaction.commit_unless_managed(using=db)
+ with transaction.commit_on_success_unless_managed(using=db):
+ for app_name, model_list in manifest.items():
+ for model in model_list:
+ # Create the model's database table, if it doesn't already exist.
+ if verbosity >= 3:
+ self.stdout.write("Processing %s.%s model\n" % (app_name, model._meta.object_name))
+ sql, references = connection.creation.sql_create_model(model, self.style, seen_models)
+ seen_models.add(model)
+ created_models.add(model)
+ for refto, refs in references.items():
+ pending_references.setdefault(refto, []).extend(refs)
+ if refto in seen_models:
+ sql.extend(connection.creation.sql_for_pending_references(refto, self.style, pending_references))
+ sql.extend(connection.creation.sql_for_pending_references(model, self.style, pending_references))
+ if verbosity >= 1 and sql:
+ self.stdout.write("Creating table %s\n" % model._meta.db_table)
+ for statement in sql:
+ cursor.execute(statement)
+ tables.append(connection.introspection.table_name_converter(model._meta.db_table))
# Send the post_syncdb signal, so individual apps can do whatever they need
# to do at this point.
@@ -122,17 +121,16 @@ def model_installed(model):
if custom_sql:
if verbosity >= 2:
self.stdout.write("Installing custom SQL for %s.%s model\n" % (app_name, model._meta.object_name))
- try:
- for sql in custom_sql:
- cursor.execute(sql)
- except Exception as e:
- self.stderr.write("Failed to install custom SQL for %s.%s model: %s\n" % \
- (app_name, model._meta.object_name, e))
- if show_traceback:
- traceback.print_exc()
- transaction.rollback_unless_managed(using=db)
- else:
- transaction.commit_unless_managed(using=db)
+ with transaction.commit_on_success_unless_managed(using=db):
+ try:
+ for sql in custom_sql:
+ cursor.execute(sql)
+ except Exception as e:
+ self.stderr.write("Failed to install custom SQL for %s.%s model: %s\n" % \
+ (app_name, model._meta.object_name, e))
+ if show_traceback:
+ traceback.print_exc()
+ raise
else:
if verbosity >= 3:
self.stdout.write("No custom SQL for %s.%s model\n" % (app_name, model._meta.object_name))
@@ -147,15 +145,14 @@ def model_installed(model):
if index_sql:
if verbosity >= 2:
self.stdout.write("Installing index for %s.%s model\n" % (app_name, model._meta.object_name))
- try:
- for sql in index_sql:
- cursor.execute(sql)
- except Exception as e:
- self.stderr.write("Failed to install index for %s.%s model: %s\n" % \
- (app_name, model._meta.object_name, e))
- transaction.rollback_unless_managed(using=db)
- else:
- transaction.commit_unless_managed(using=db)
+ with transaction.commit_on_success_unless_managed(using=db):
+ try:
+ for sql in index_sql:
+ cursor.execute(sql)
+ except Exception as e:
+ self.stderr.write("Failed to install index for %s.%s model: %s\n" % \
+ (app_name, model._meta.object_name, e))
+ raise
# Load initial_data fixtures (unless that has been disabled)
if load_initial_data:
View
11 django/db/__init__.py
@@ -77,14 +77,3 @@ def close_old_connections(**kwargs):
conn.close_if_unusable_or_obsolete()
signals.request_started.connect(close_old_connections)
signals.request_finished.connect(close_old_connections)
-
-# Register an event that rolls back the connections
-# when a Django request has an exception.
-def _rollback_on_exception(**kwargs):
- from django.db import transaction
- for conn in connections:
- try:
- transaction.rollback_unless_managed(using=conn)
- except DatabaseError:
- pass
-signals.got_request_exception.connect(_rollback_on_exception)
View
21 django/db/backends/__init__.py
@@ -339,27 +339,6 @@ def is_managed(self):
return self.transaction_state[-1]
return settings.TRANSACTIONS_MANAGED
- def commit_unless_managed(self):
- """
- Commits changes if the system is not in managed transaction mode.
- """
- self.validate_thread_sharing()
- if not self.is_managed():
- self.commit()
- self.clean_savepoints()
- else:
- self.set_dirty()
-
- def rollback_unless_managed(self):
- """
- Rolls back changes if the system is not in managed transaction mode.
- """
- self.validate_thread_sharing()
- if not self.is_managed():
- self.rollback()
- else:
- self.set_dirty()
-
##### Foreign key constraints checks handling #####
@contextmanager
View
2  django/db/backends/dummy/base.py
@@ -58,8 +58,6 @@ class DatabaseWrapper(BaseDatabaseWrapper):
_set_autocommit = complain
set_dirty = complain
set_clean = complain
- commit_unless_managed = complain
- rollback_unless_managed = ignore
def __init__(self, *args, **kwargs):
super(DatabaseWrapper, self).__init__(*args, **kwargs)
View
84 django/db/models/base.py
@@ -609,48 +609,48 @@ def save_base(self, raw=False, cls=None, origin=None, force_insert=False,
if update_fields:
non_pks = [f for f in non_pks if f.name in update_fields or f.attname in update_fields]
- # First, try an UPDATE. If that doesn't update anything, do an INSERT.
- pk_val = self._get_pk_val(meta)
- pk_set = pk_val is not None
- record_exists = True
- manager = cls._base_manager
- if pk_set:
- # Determine if we should do an update (pk already exists, forced update,
- # no force_insert)
- if ((force_update or update_fields) or (not force_insert and
- manager.using(using).filter(pk=pk_val).exists())):
- if force_update or non_pks:
- values = [(f, None, (raw and getattr(self, f.attname) or f.pre_save(self, False))) for f in non_pks]
- if values:
- rows = manager.using(using).filter(pk=pk_val)._update(values)
- if force_update and not rows:
- raise DatabaseError("Forced update did not affect any rows.")
- if update_fields and not rows:
- raise DatabaseError("Save with update_fields did not affect any rows.")
- else:
- record_exists = False
- if not pk_set or not record_exists:
- if meta.order_with_respect_to:
- # If this is a model with an order_with_respect_to
- # autopopulate the _order field
- field = meta.order_with_respect_to
- order_value = manager.using(using).filter(**{field.name: getattr(self, field.attname)}).count()
- self._order = order_value
-
- fields = meta.local_fields
- if not pk_set:
- if force_update or update_fields:
- raise ValueError("Cannot force an update in save() with no primary key.")
- fields = [f for f in fields if not isinstance(f, AutoField)]
+ with transaction.commit_on_success_unless_managed(using=using):
+ # First, try an UPDATE. If that doesn't update anything, do an INSERT.
+ pk_val = self._get_pk_val(meta)
+ pk_set = pk_val is not None
+ record_exists = True
+ manager = cls._base_manager
+ if pk_set:
+ # Determine if we should do an update (pk already exists, forced update,
+ # no force_insert)
+ if ((force_update or update_fields) or (not force_insert and
+ manager.using(using).filter(pk=pk_val).exists())):
+ if force_update or non_pks:
+ values = [(f, None, (raw and getattr(self, f.attname) or f.pre_save(self, False))) for f in non_pks]
+ if values:
+ rows = manager.using(using).filter(pk=pk_val)._update(values)
+ if force_update and not rows:
+ raise DatabaseError("Forced update did not affect any rows.")
+ if update_fields and not rows:
+ raise DatabaseError("Save with update_fields did not affect any rows.")
+ else:
+ record_exists = False
+ if not pk_set or not record_exists:
+ if meta.order_with_respect_to:
+ # If this is a model with an order_with_respect_to
+ # autopopulate the _order field
+ field = meta.order_with_respect_to
+ order_value = manager.using(using).filter(**{field.name: getattr(self, field.attname)}).count()
+ self._order = order_value
+
+ fields = meta.local_fields
+ if not pk_set:
+ if force_update or update_fields:
+ raise ValueError("Cannot force an update in save() with no primary key.")
+ fields = [f for f in fields if not isinstance(f, AutoField)]
- record_exists = False
+ record_exists = False
- update_pk = bool(meta.has_auto_field and not pk_set)
- result = manager._insert([self], fields=fields, return_id=update_pk, using=using, raw=raw)
+ update_pk = bool(meta.has_auto_field and not pk_set)
+ result = manager._insert([self], fields=fields, return_id=update_pk, using=using, raw=raw)
- if update_pk:
- setattr(self, meta.pk.attname, result)
- transaction.commit_unless_managed(using=using)
+ if update_pk:
+ setattr(self, meta.pk.attname, result)
# Store the database on which the object was saved
self._state.db = using
@@ -963,9 +963,9 @@ def method_set_order(ordered_obj, self, id_list, using=None):
order_name = ordered_obj._meta.order_with_respect_to.name
# FIXME: It would be nice if there was an "update many" version of update
# for situations like this.
- for i, j in enumerate(id_list):
- ordered_obj.objects.filter(**{'pk': j, order_name: rel_val}).update(_order=i)
- transaction.commit_unless_managed(using=using)
+ with transaction.commit_on_success_unless_managed(using=using):
+ for i, j in enumerate(id_list):
+ ordered_obj.objects.filter(**{'pk': j, order_name: rel_val}).update(_order=i)
def method_get_order(ordered_obj, self):
View
2  django/db/models/deletion.py
@@ -62,8 +62,6 @@ def decorated(self, *args, **kwargs):
func(self, *args, **kwargs)
if forced_managed:
transaction.commit(using=self.using)
- else:
- transaction.commit_unless_managed(using=self.using)
finally:
if forced_managed:
transaction.leave_transaction_management(using=self.using)
View
4 django/db/models/query.py
@@ -460,8 +460,6 @@ def bulk_create(self, objs, batch_size=None):
self._batched_insert(objs_without_pk, fields, batch_size)
if forced_managed:
transaction.commit(using=self.db)
- else:
- transaction.commit_unless_managed(using=self.db)
finally:
if forced_managed:
transaction.leave_transaction_management(using=self.db)
@@ -590,8 +588,6 @@ def update(self, **kwargs):
rows = query.get_compiler(self.db).execute_sql(None)
if forced_managed:
transaction.commit(using=self.db)
- else:
- transaction.commit_unless_managed(using=self.db)
finally:
if forced_managed:
transaction.leave_transaction_management(using=self.db)
View
27 django/db/transaction.py
@@ -123,16 +123,12 @@ def managed(flag=True, using=None):
PendingDeprecationWarning, stacklevel=2)
def commit_unless_managed(using=None):
- """
- Commits changes if the system is not in managed transaction mode.
- """
- get_connection(using).commit_unless_managed()
+ warnings.warn("'commit_unless_managed' is now a no-op.",
+ PendingDeprecationWarning, stacklevel=2)
def rollback_unless_managed(using=None):
- """
- Rolls back changes if the system is not in managed transaction mode.
- """
- get_connection(using).rollback_unless_managed()
+ warnings.warn("'rollback_unless_managed' is now a no-op.",
+ PendingDeprecationWarning, stacklevel=2)
###############
# Public APIs #
@@ -280,3 +276,18 @@ def exiting(exc_value, using):
leave_transaction_management(using=using)
return _transaction_func(entering, exiting, using)
+
+def commit_on_success_unless_managed(using=None):
+ """
+ Transitory API to preserve backwards-compatibility while refactoring.
+ """
+ if is_managed(using):
+ def entering(using):
+ pass
+
+ def exiting(exc_value, using):
+ set_dirty(using=using)
+
+ return _transaction_func(entering, exiting, using)
+ else:
+ return commit_on_success(using)
View
19 django/test/testcases.py
@@ -157,14 +157,6 @@ def __init__(self, *args, **kwargs):
doctest.DocTestRunner.__init__(self, *args, **kwargs)
self.optionflags = doctest.ELLIPSIS
- def report_unexpected_exception(self, out, test, example, exc_info):
- doctest.DocTestRunner.report_unexpected_exception(self, out, test,
- example, exc_info)
- # Rollback, in case of database errors. Otherwise they'd have
- # side effects on other tests.
- for conn in connections:
- transaction.rollback_unless_managed(using=conn)
-
class _AssertNumQueriesContext(CaptureQueriesContext):
def __init__(self, test_case, num, connection):
@@ -490,14 +482,10 @@ def _reset_sequences(self, db_name):
conn.ops.sequence_reset_by_name_sql(no_style(),
conn.introspection.sequence_list())
if sql_list:
- try:
+ with transaction.commit_on_success_unless_managed(using=db_name):
cursor = conn.cursor()
for sql in sql_list:
cursor.execute(sql)
- except Exception:
- transaction.rollback_unless_managed(using=db_name)
- raise
- transaction.commit_unless_managed(using=db_name)
def _fixture_setup(self):
for db_name in self._databases_names(include_mirrors=False):
@@ -537,11 +525,6 @@ def _post_teardown(self):
conn.close()
def _fixture_teardown(self):
- # Roll back any pending transactions in order to avoid a deadlock
- # during flush when TEST_MIRROR is used (#18984).
- for conn in connections.all():
- conn.rollback_unless_managed()
-
for db in self._databases_names(include_mirrors=False):
call_command('flush', verbosity=0, interactive=False, database=db,
skip_validation=True, reset_sequences=False)
View
2  docs/internals/deprecation.txt
@@ -352,6 +352,8 @@ these changes.
- ``django.db.close_connection()``
- ``django.db.backends.creation.BaseDatabaseCreation.set_autocommit()``
- ``django.db.transaction.managed()``
+ - ``django.db.transaction.commit_unless_managed()``
+ - ``django.db.transaction.rollback_unless_managed()``
2.0
---
View
32 tests/transactions_regress/tests.py
@@ -208,38 +208,6 @@ def test_enter_exit_management(self):
connection.leave_transaction_management()
self.assertEqual(orig_dirty, connection._dirty)
- # TODO: update this test to account for database-level autocommit.
- @expectedFailure
- def test_commit_unless_managed(self):
- cursor = connection.cursor()
- cursor.execute("INSERT into transactions_regress_mod (fld) values (2)")
- connection.commit_unless_managed()
- self.assertFalse(connection.is_dirty())
- self.assertEqual(len(Mod.objects.all()), 1)
- self.assertTrue(connection.is_dirty())
- connection.commit_unless_managed()
- self.assertFalse(connection.is_dirty())
-
- # TODO: update this test to account for database-level autocommit.
- @expectedFailure
- def test_commit_unless_managed_in_managed(self):
- cursor = connection.cursor()
- connection.enter_transaction_management()
- cursor.execute("INSERT into transactions_regress_mod (fld) values (2)")
- connection.commit_unless_managed()
- self.assertTrue(connection.is_dirty())
- connection.rollback()
- self.assertFalse(connection.is_dirty())
- self.assertEqual(len(Mod.objects.all()), 0)
- connection.commit()
- connection.leave_transaction_management()
- self.assertFalse(connection.is_dirty())
- self.assertEqual(len(Mod.objects.all()), 0)
- self.assertTrue(connection.is_dirty())
- connection.commit_unless_managed()
- self.assertFalse(connection.is_dirty())
- self.assertEqual(len(Mod.objects.all()), 0)
-
@skipUnless(connection.vendor == 'postgresql',
"This test only valid for PostgreSQL")
Please sign in to comment.
Something went wrong with that request. Please try again.