From: Mike Bayer Date: Sat, 7 Dec 2013 22:20:05 +0000 (-0500) Subject: - A DBAPI that raises an error on ``connect()`` which is not a subclass X-Git-Tag: rel_0_9_0~48^2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6d5eae78a7dd79ad7bd0a0951bc6c95437d0fa8e;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - A DBAPI that raises an error on ``connect()`` which is not a subclass of dbapi.Error (such as ``TypeError``, ``NotImplementedError``, etc.) will propagate the exception unchanged. Previously, the error handling specific to the ``connect()`` routine would both inappropriately run the exception through the dialect's :meth:`.Dialect.is_disconnect` routine as well as wrap it in a :class:`sqlalchemy.exc.DBAPIError`. It is now propagated unchanged in the same way as occurs within the execute process. [ticket:2881] - add tests for this in test_parseconnect, but also add tests in test_execute to ensure the execute() behavior as well --- diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst index 4c9cc85d93..0902ff7e46 100644 --- a/doc/build/changelog/changelog_08.rst +++ b/doc/build/changelog/changelog_08.rst @@ -11,6 +11,20 @@ .. changelog:: :version: 0.8.4 + .. change:: + :tags: bug, engine + :versions: 0.9.0b2 + :tickets: 2881 + + A DBAPI that raises an error on ``connect()`` which is not a subclass + of dbapi.Error (such as ``TypeError``, ``NotImplementedError``, etc.) + will propagate the exception unchanged. Previously, + the error handling specific to the ``connect()`` routine would both + inappropriately run the exception through the dialect's + :meth:`.Dialect.is_disconnect` routine as well as wrap it in + a :class:`sqlalchemy.exc.DBAPIError`. It is now propagated unchanged + in the same way as occurs within the execute process. + .. change:: :tags: bug, engine, pool :versions: 0.9.0b2 diff --git a/lib/sqlalchemy/engine/strategies.py b/lib/sqlalchemy/engine/strategies.py index ab9d370a3c..5a3b2c5af8 100644 --- a/lib/sqlalchemy/engine/strategies.py +++ b/lib/sqlalchemy/engine/strategies.py @@ -78,7 +78,7 @@ class DefaultEngineStrategy(EngineStrategy): def connect(): try: return dialect.connect(*cargs, **cparams) - except Exception as e: + except dialect.dbapi.Error as e: invalidated = dialect.is_disconnect(e, None, None) util.raise_from_cause( exc.DBAPIError.instance(None, None, diff --git a/test/engine/test_execute.py b/test/engine/test_execute.py index 1f7bad31a2..7312bb0412 100644 --- a/test/engine/test_execute.py +++ b/test/engine/test_execute.py @@ -203,19 +203,37 @@ class ExecuteTest(fixtures.TestBase): finally: conn.close() + @testing.engines.close_open_connections def test_exception_wrapping_dbapi(self): - def go(conn): + conn = testing.db.connect() + for _c in testing.db, conn: assert_raises_message( tsa.exc.DBAPIError, r"not_a_valid_statement", - conn.execute, 'not_a_valid_statement' + _c.execute, 'not_a_valid_statement' ) - go(testing.db) - conn = testing.db.connect() - try: - go(conn) - finally: - conn.close() + + @testing.requires.sqlite + def test_exception_wrapping_non_dbapi_error(self): + e = create_engine('sqlite://') + e.dialect.is_disconnect = is_disconnect = Mock() + + c = e.connect() + + c.connection.cursor = Mock( + return_value=Mock( + execute=Mock( + side_effect=TypeError("I'm not a DBAPI error") + )) + ) + + assert_raises_message( + TypeError, + "I'm not a DBAPI error", + c.execute, "select " + ) + eq_(is_disconnect.call_count, 0) + def test_exception_wrapping_non_dbapi_statement(self): class MyType(TypeDecorator): diff --git a/test/engine/test_parseconnect.py b/test/engine/test_parseconnect.py index 0ae747b9c6..c4d8b8edc6 100644 --- a/test/engine/test_parseconnect.py +++ b/test/engine/test_parseconnect.py @@ -1,4 +1,4 @@ -from sqlalchemy.testing import assert_raises, eq_ +from sqlalchemy.testing import assert_raises, eq_, assert_raises_message from sqlalchemy.util.compat import configparser, StringIO import sqlalchemy.engine.url as url from sqlalchemy import create_engine, engine_from_config, exc, pool @@ -256,17 +256,38 @@ pool_timeout=10 @testing.requires.sqlite def test_wraps_connect_in_dbapi(self): - # sqlite uses SingletonThreadPool which doesnt have max_overflow + e = create_engine('sqlite://') + sqlite3 = e.dialect.dbapi - assert_raises(TypeError, create_engine, 'sqlite://', - max_overflow=5, module=mock_sqlite_dbapi) - e = create_engine('sqlite://', connect_args={'use_unicode' - : True}, convert_unicode=True) + dbapi = MockDBAPI() + dbapi.Error = sqlite3.Error, + dbapi.ProgrammingError = sqlite3.ProgrammingError + dbapi.connect = Mock(side_effect=sqlite3.ProgrammingError("random error")) try: - e.connect() + create_engine('sqlite://', module=dbapi).connect() + assert False except tsa.exc.DBAPIError as de: assert not de.connection_invalidated + + @testing.requires.sqlite + def test_dont_touch_non_dbapi_exception_on_connect(self): + e = create_engine('sqlite://') + sqlite3 = e.dialect.dbapi + + dbapi = MockDBAPI() + dbapi.Error = sqlite3.Error, + dbapi.ProgrammingError = sqlite3.ProgrammingError + dbapi.connect = Mock(side_effect=TypeError("I'm not a DBAPI error")) + e = create_engine('sqlite://', module=dbapi) + e.dialect.is_disconnect = is_disconnect = Mock() + assert_raises_message( + TypeError, + "I'm not a DBAPI error", + e.connect + ) + eq_(is_disconnect.call_count, 0) + def test_ensure_dialect_does_is_disconnect_no_conn(self): """test that is_disconnect() doesn't choke if no connection, cursor given.""" dialect = testing.db.dialect