From: Mike Bayer Date: Thu, 11 Apr 2013 23:10:02 +0000 (-0400) Subject: Improvements to Connection auto-invalidation X-Git-Tag: rel_0_8_1~17^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ee7bb9c1744b4b09263ef32032afd38f694df136;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Improvements to Connection auto-invalidation handling. If a non-disconnect error occurs, but leads to a delayed disconnect error within error handling (happens with MySQL), the disconnect condition is detected. The Connection can now also be closed when in an invalid state, meaning it will raise "closed" on next usage, and additionally the "close with result" feature will work even if the autorollback in an error handling routine fails and regardless of whether the condition is a disconnect or not. [ticket:2695] --- diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst index cdd96e01ea..50d31a094f 100644 --- a/doc/build/changelog/changelog_08.rst +++ b/doc/build/changelog/changelog_08.rst @@ -6,6 +6,21 @@ .. changelog:: :version: 0.8.1 + .. change:: + :tags: bug, sql + :tickets: 2695 + + Improvements to Connection auto-invalidation + handling. If a non-disconnect error occurs, + but leads to a delayed disconnect error within error + handling (happens with MySQL), the disconnect condition + is detected. The Connection can now also be closed + when in an invalid state, meaning it will raise "closed" + on next usage, and additionally the "close with result" + feature will work even if the autorollback in an error + handling routine fails and regardless of whether the + condition is a disconnect or not. + .. change:: :tags: bug, sql :tickets: 2702 diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py index 6242f08168..e40af62199 100644 --- a/lib/sqlalchemy/engine/base.py +++ b/lib/sqlalchemy/engine/base.py @@ -62,6 +62,7 @@ class Connection(Connectable): self.__savepoint_seq = 0 self.__branch = _branch self.__invalid = False + self.__can_reconnect = True if _dispatch: self.dispatch = _dispatch elif engine._has_events: @@ -213,8 +214,8 @@ class Connection(Connectable): def closed(self): """Return True if this connection is closed.""" - return not self.__invalid and '_Connection__connection' \ - not in self.__dict__ + return '_Connection__connection' not in self.__dict__ \ + and not self.__can_reconnect @property def invalidated(self): @@ -232,7 +233,7 @@ class Connection(Connectable): return self._revalidate_connection() def _revalidate_connection(self): - if self.__invalid: + if self.__can_reconnect and self.__invalid: if self.__transaction is not None: raise exc.InvalidRequestError( "Can't reconnect until invalid " @@ -577,15 +578,15 @@ class Connection(Connectable): and will allow no further operations. """ - try: conn = self.__connection except AttributeError: - return - if not self.__branch: - conn.close() - self.__invalid = False - del self.__connection + pass + else: + if not self.__branch: + conn.close() + del self.__connection + self.__can_reconnect = False self.__transaction = None def scalar(self, object, *multiparams, **params): @@ -972,13 +973,22 @@ class Connection(Connectable): if isinstance(e, (SystemExit, KeyboardInterrupt)): raise + _reentrant_error = False + _is_disconnect = False + def _handle_dbapi_exception(self, e, statement, parameters, cursor, context): - if getattr(self, '_reentrant_error', False): + + if not self._is_disconnect: + self._is_disconnect = isinstance(e, self.dialect.dbapi.Error) and \ + not self.closed and \ + self.dialect.is_disconnect(e, self.__connection, cursor) + + if self._reentrant_error: # Py3K #raise exc.DBAPIError.instance(statement, parameters, e, # self.dialect.dbapi.Error) from e @@ -1006,21 +1016,10 @@ class Connection(Connectable): e) context.handle_dbapi_exception(e) - is_disconnect = isinstance(e, self.dialect.dbapi.Error) and \ - self.dialect.is_disconnect(e, self.__connection, cursor) - - if is_disconnect: - dbapi_conn_wrapper = self.connection - self.invalidate(e) - if not hasattr(dbapi_conn_wrapper, '_pool') or \ - dbapi_conn_wrapper._pool is self.engine.pool: - self.engine.dispose() - else: + if not self._is_disconnect: if cursor: self._safe_close_cursor(cursor) self._autorollback() - if self.should_close_with_result: - self.close() if not should_wrap: return @@ -1031,7 +1030,7 @@ class Connection(Connectable): # parameters, # e, # self.dialect.dbapi.Error, - # connection_invalidated=is_disconnect) \ + # connection_invalidated=self._is_disconnect) \ # from e # Py2K raise exc.DBAPIError.instance( @@ -1039,12 +1038,21 @@ class Connection(Connectable): parameters, e, self.dialect.dbapi.Error, - connection_invalidated=is_disconnect), \ + connection_invalidated=self._is_disconnect), \ None, sys.exc_info()[2] # end Py2K finally: del self._reentrant_error + if self._is_disconnect: + del self._is_disconnect + dbapi_conn_wrapper = self.connection + self.invalidate(e) + if not hasattr(dbapi_conn_wrapper, '_pool') or \ + dbapi_conn_wrapper._pool is self.engine.pool: + self.engine.dispose() + if self.should_close_with_result: + self.close() # poor man's multimethod/generic function thingy executors = { diff --git a/test/engine/test_reconnect.py b/test/engine/test_reconnect.py index 6b283654b6..86f646f33c 100644 --- a/test/engine/test_reconnect.py +++ b/test/engine/test_reconnect.py @@ -11,7 +11,10 @@ from sqlalchemy import exc from sqlalchemy.testing import fixtures from sqlalchemy.testing.engines import testing_engine -class MockDisconnect(Exception): +class MockError(Exception): + pass + +class MockDisconnect(MockError): pass class MockDBAPI(object): @@ -20,17 +23,23 @@ class MockDBAPI(object): self.connections = weakref.WeakKeyDictionary() def connect(self, *args, **kwargs): return MockConnection(self) - def shutdown(self): + def shutdown(self, explode='execute'): for c in self.connections: - c.explode[0] = True - Error = MockDisconnect + c.explode = explode + Error = MockError class MockConnection(object): def __init__(self, dbapi): dbapi.connections[self] = True - self.explode = [False] + self.explode = "" def rollback(self): - pass + if self.explode == 'rollback': + raise MockDisconnect("Lost the DB connection on rollback") + if self.explode == 'rollback_no_disconnect': + raise MockError( + "something broke on rollback but we didn't lose the connection") + else: + return def commit(self): pass def cursor(self): @@ -43,8 +52,14 @@ class MockCursor(object): self.explode = parent.explode self.description = () def execute(self, *args, **kwargs): - if self.explode[0]: - raise MockDisconnect("Lost the DB connection") + if self.explode == 'execute': + raise MockDisconnect("Lost the DB connection on execute") + elif self.explode in ('execute_no_disconnect', ): + raise MockError( + "something broke on execute but we didn't lose the connection") + elif self.explode in ('rollback', 'rollback_no_disconnect'): + raise MockError( + "something broke on execute but we didn't lose the connection") else: return def close(self): @@ -167,12 +182,10 @@ class MockReconnectTest(fixtures.TestBase): dbapi.shutdown() - # raises error - try: - conn.execute(select([1])) - assert False - except tsa.exc.DBAPIError: - pass + assert_raises( + tsa.exc.DBAPIError, + conn.execute, select([1]) + ) assert not conn.closed assert conn.invalidated @@ -186,6 +199,101 @@ class MockReconnectTest(fixtures.TestBase): assert not conn.invalidated assert len(dbapi.connections) == 1 + def test_invalidated_close(self): + conn = db.connect() + + dbapi.shutdown() + + assert_raises( + tsa.exc.DBAPIError, + conn.execute, select([1]) + ) + + conn.close() + assert conn.closed + assert conn.invalidated + assert_raises_message( + tsa.exc.StatementError, + "This Connection is closed", + conn.execute, select([1]) + ) + + def test_noreconnect_execute_plus_closewresult(self): + conn = db.connect(close_with_result=True) + + dbapi.shutdown("execute_no_disconnect") + + # raises error + assert_raises_message( + tsa.exc.DBAPIError, + "something broke on execute but we didn't lose the connection", + conn.execute, select([1]) + ) + + assert conn.closed + assert not conn.invalidated + + def test_noreconnect_rollback_plus_closewresult(self): + conn = db.connect(close_with_result=True) + + dbapi.shutdown("rollback_no_disconnect") + + # raises error + assert_raises_message( + tsa.exc.DBAPIError, + "something broke on rollback but we didn't lose the connection", + conn.execute, select([1]) + ) + + assert conn.closed + assert not conn.invalidated + + assert_raises_message( + tsa.exc.StatementError, + "This Connection is closed", + conn.execute, select([1]) + ) + + def test_reconnect_on_reentrant(self): + conn = db.connect() + + conn.execute(select([1])) + + assert len(dbapi.connections) == 1 + + dbapi.shutdown("rollback") + + # raises error + assert_raises_message( + tsa.exc.DBAPIError, + "Lost the DB connection on rollback", + conn.execute, select([1]) + ) + + assert not conn.closed + assert conn.invalidated + + def test_reconnect_on_reentrant_plus_closewresult(self): + conn = db.connect(close_with_result=True) + + dbapi.shutdown("rollback") + + # raises error + assert_raises_message( + tsa.exc.DBAPIError, + "Lost the DB connection on rollback", + conn.execute, select([1]) + ) + + assert conn.closed + assert conn.invalidated + + assert_raises_message( + tsa.exc.StatementError, + "This Connection is closed", + conn.execute, select([1]) + ) + class CursorErrTest(fixtures.TestBase): def setup(self):