From 743263a1058bb54617446507cbb645aab571ac93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Tue, 12 Feb 2013 23:11:22 +0200 Subject: [PATCH] [1.5.x] Removed try-except in django.db.close_connection() The reason was that the except clause needed to remove a connection from the django.db.connections dict, but other parts of Django do not expect this to happen. In addition the except clause was silently swallowing the exception messages. Refs #19707, special thanks to Carl Meyer for pointing out that this approach should be taken. --- django/db/__init__.py | 13 +++++-------- django/db/utils.py | 3 --- tests/regressiontests/requests/tests.py | 18 ++++++++++-------- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/django/db/__init__.py b/django/db/__init__.py index 94eca13d41f..32e42bbfe9d 100644 --- a/django/db/__init__.py +++ b/django/db/__init__.py @@ -45,14 +45,11 @@ def close_connection(**kwargs): # Avoid circular imports from django.db import transaction for conn in connections: - try: - transaction.abort(conn) - connections[conn].close() - except Exception: - # The connection's state is unknown, so it has to be - # abandoned. This could happen for example if the network - # connection has a failure. - del connections[conn] + # If an error happens here the connection will be left in broken + # state. Once a good db connection is again available, the + # connection state will be cleaned up. + transaction.abort(conn) + connections[conn].close() signals.request_finished.connect(close_connection) # Register an event that resets connection.queries diff --git a/django/db/utils.py b/django/db/utils.py index fb9bdc1e710..842fd354d62 100644 --- a/django/db/utils.py +++ b/django/db/utils.py @@ -98,9 +98,6 @@ class ConnectionHandler(object): def __setitem__(self, key, value): setattr(self._connections, key, value) - def __delitem__(self, key): - delattr(self._connections, key) - def __iter__(self): return iter(self.databases) diff --git a/tests/regressiontests/requests/tests.py b/tests/regressiontests/requests/tests.py index 42f1abe4074..b7517fec0fc 100644 --- a/tests/regressiontests/requests/tests.py +++ b/tests/regressiontests/requests/tests.py @@ -562,9 +562,6 @@ class TransactionRequestTests(TransactionTestCase): 'This test will close the connection, in-memory ' 'sqlite connections must not be closed.') def test_request_finished_failed_connection(self): - # See comments in test_request_finished_db_state() for the self.client - # usage. - response = self.client.get('/') conn = connections[DEFAULT_DB_ALIAS] conn.enter_transaction_management() conn.managed(True) @@ -574,9 +571,14 @@ class TransactionRequestTests(TransactionTestCase): def fail_horribly(): raise Exception("Horrible failure!") conn._rollback = fail_horribly - signals.request_finished.send(sender=response._handler_class) - # As even rollback wasn't possible the connection wrapper itself was - # abandoned. Accessing the connections[alias] will create a new - # connection wrapper, whch must be different than the original one. - self.assertIsNot(conn, connections[DEFAULT_DB_ALIAS]) + try: + with self.assertRaises(Exception): + signals.request_finished.send(sender=self.__class__) + # The connection's state wasn't cleaned up + self.assertTrue(len(connection.transaction_state), 1) + finally: + del conn._rollback + # The connection will be cleaned on next request where the conn + # works again. + signals.request_finished.send(sender=self.__class__) self.assertEqual(len(connection.transaction_state), 0)