From: Mike Bayer Date: Thu, 26 Aug 2010 05:33:51 +0000 (-0400) Subject: - replaced all self.__connection.is_valid with X-Git-Tag: rel_0_6_4~31 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7d3a916fc3bc5d81b6b251a42f196793f5643b76;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - replaced all self.__connection.is_valid with "safe" self._connection_is_valid. - Fixed bug in Connection whereby if a "disconnect" event occurred in the "initialize" phase of the first connection pool connect, an AttributeError would be raised when the Connection would attempt to invalidate the DBAPI connection. [ticket:1894] - Connection.invalidate() can be called more than once and subsequent calls do nothing. --- diff --git a/CHANGES b/CHANGES index 6e41063a50..8fe4c49f56 100644 --- a/CHANGES +++ b/CHANGES @@ -135,21 +135,6 @@ CHANGES of 64 for index names, separate from their overall max length of 255. [ticket:1412] - - Calling fetchone() or similar on a result that - has already been exhausted, has been closed, - or is not a result-returning result now - raises ResourceClosedError, a subclass of - InvalidRequestError, in all cases, regardless - of backend. Previously, some DBAPIs would - raise ProgrammingError (i.e. pysqlite), others - would return None leading to downstream breakages - (i.e. MySQL-python). - - - Connection, ResultProxy, as well as Session use - ResourceClosedError for all "this - connection/transaction/result is closed" types of - errors. - - the text() construct, if placed in a column oriented situation, will at least return NULLTYPE for its type instead of None, allowing it to @@ -175,7 +160,33 @@ CHANGES operations which depend on the identity of the _Labels themselves to return the correct result - fixes ORM bug [ticket:1852]. - + +- engine + + - Calling fetchone() or similar on a result that + has already been exhausted, has been closed, + or is not a result-returning result now + raises ResourceClosedError, a subclass of + InvalidRequestError, in all cases, regardless + of backend. Previously, some DBAPIs would + raise ProgrammingError (i.e. pysqlite), others + would return None leading to downstream breakages + (i.e. MySQL-python). + + - Fixed bug in Connection whereby if a "disconnect" + event occurred in the "initialize" phase of the + first connection pool connect, an AttributeError + would be raised when the Connection would attempt + to invalidate the DBAPI connection. [ticket:1894] + + - Connection, ResultProxy, as well as Session use + ResourceClosedError for all "this + connection/transaction/result is closed" types of + errors. + + - Connection.invalidate() can be called more than + once and subsequent calls do nothing. + - declarative - if @classproperty is used with a regular class-bound mapper property attribute, it will be called to get the diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py index b481261d58..14ebf916b1 100644 --- a/lib/sqlalchemy/engine/base.py +++ b/lib/sqlalchemy/engine/base.py @@ -910,6 +910,14 @@ class Connection(Connectable): return self.__connection raise exc.ResourceClosedError("This Connection is closed") + @property + def _connection_is_valid(self): + # use getattr() for is_valid to support exceptions raised in + # dialect initializer, where the connection is not wrapped in + # _ConnectionFairy + + return getattr(self.__connection, 'is_valid', False) + @property def info(self): """A collection of per-DB-API connection instance properties.""" @@ -954,15 +962,18 @@ class Connection(Connectable): operations in a non-transactional state. """ - + if self.invalidated: + return + if self.closed: raise exc.ResourceClosedError("This Connection is closed") - if self.__connection.is_valid: + if self._connection_is_valid: self.__connection.invalidate(exception) del self.__connection self.__invalid = True - + + def detach(self): """Detach the underlying DB-API connection from its connection pool. @@ -1055,11 +1066,8 @@ class Connection(Connectable): raise def _rollback_impl(self): - # use getattr() for is_valid to support exceptions raised in - # dialect initializer, - # where we do not yet have the pool wrappers plugged in if not self.closed and not self.invalidated and \ - getattr(self.__connection, 'is_valid', False): + self._connection_is_valid: if self._echo: self.engine.logger.info("ROLLBACK") try: @@ -1085,37 +1093,37 @@ class Connection(Connectable): if name is None: self.__savepoint_seq += 1 name = 'sa_savepoint_%s' % self.__savepoint_seq - if self.__connection.is_valid: + if self._connection_is_valid: self.engine.dialect.do_savepoint(self, name) return name def _rollback_to_savepoint_impl(self, name, context): - if self.__connection.is_valid: + if self._connection_is_valid: self.engine.dialect.do_rollback_to_savepoint(self, name) self.__transaction = context def _release_savepoint_impl(self, name, context): - if self.__connection.is_valid: + if self._connection_is_valid: self.engine.dialect.do_release_savepoint(self, name) self.__transaction = context def _begin_twophase_impl(self, xid): - if self.__connection.is_valid: + if self._connection_is_valid: self.engine.dialect.do_begin_twophase(self, xid) def _prepare_twophase_impl(self, xid): - if self.__connection.is_valid: + if self._connection_is_valid: assert isinstance(self.__transaction, TwoPhaseTransaction) self.engine.dialect.do_prepare_twophase(self, xid) def _rollback_twophase_impl(self, xid, is_prepared): - if self.__connection.is_valid: + if self._connection_is_valid: assert isinstance(self.__transaction, TwoPhaseTransaction) self.engine.dialect.do_rollback_twophase(self, xid, is_prepared) self.__transaction = None def _commit_twophase_impl(self, xid, is_prepared): - if self.__connection.is_valid: + if self._connection_is_valid: assert isinstance(self.__transaction, TwoPhaseTransaction) self.engine.dialect.do_commit_twophase(self, xid, is_prepared) self.__transaction = None diff --git a/test/engine/test_pool.py b/test/engine/test_pool.py index 9db65d2ab8..94d75a9a81 100644 --- a/test/engine/test_pool.py +++ b/test/engine/test_pool.py @@ -668,7 +668,7 @@ class QueuePoolTest(PoolTestBase): c1 = None c1 = p.connect() assert c1.connection.id != c_id - + def test_recreate(self): dbapi = MockDBAPI() p = pool.QueuePool(creator=lambda : dbapi.connect('foo.db'), diff --git a/test/engine/test_reconnect.py b/test/engine/test_reconnect.py index b92c880663..8ef8513589 100644 --- a/test/engine/test_reconnect.py +++ b/test/engine/test_reconnect.py @@ -1,13 +1,12 @@ -from sqlalchemy.test.testing import eq_ +from sqlalchemy.test.testing import eq_, assert_raises import time import weakref from sqlalchemy import select, MetaData, Integer, String, pool -from sqlalchemy.test.schema import Table -from sqlalchemy.test.schema import Column +from sqlalchemy.test.schema import Table, Column import sqlalchemy as tsa from sqlalchemy.test import TestBase, testing, engines from sqlalchemy.test.util import gc_collect - +from sqlalchemy import exc class MockDisconnect(Exception): pass @@ -257,7 +256,39 @@ class RealReconnectTest(TestBase): assert not conn.invalidated conn.close() + + def test_invalidate_twice(self): + conn = engine.connect() + conn.invalidate() + conn.invalidate() + + def test_explode_in_initializer(self): + engine = engines.testing_engine() + def broken_initialize(connection): + connection.execute("select fake_stuff from _fake_table") + + engine.dialect.initialize = broken_initialize + + # raises a DBAPIError, not an AttributeError + assert_raises(exc.DBAPIError, engine.connect) + # dispose connections so we get a new one on + # next go + engine.dispose() + + p1 = engine.pool + + def is_disconnect(e): + return True + + engine.dialect.is_disconnect = is_disconnect + + # invalidate() also doesn't screw up + assert_raises(exc.DBAPIError, engine.connect) + + # pool was recreated + assert engine.pool is not p1 + def test_null_pool(self): engine = \ engines.reconnecting_engine(options=dict(poolclass=pool.NullPool))