From: Mike Bayer Date: Thu, 24 Mar 2022 17:58:20 +0000 (-0400) Subject: more autocommit messaging X-Git-Tag: rel_2_0_0b1~406^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=56366924673f88e51c74d94058c11132a057ecfa;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git more autocommit messaging Further clarified connection-level logging to indicate the BEGIN, ROLLBACK and COMMIT log messages do not actually indicate a real transaction when the AUTOCOMMIT isolation level is in use; messaging has been extended to include the BEGIN message itself, and the messaging has also been fixed to accommodate when the :class:`.Engine` level :paramref:`.create_engine.isolation_level` parameter was used directly. Fixes: #7853 Change-Id: Iafc78070737ad117f84262e4bde84b81a81e4ea1 --- diff --git a/doc/build/changelog/unreleased_14/7853.rst b/doc/build/changelog/unreleased_14/7853.rst new file mode 100644 index 0000000000..66856c29e0 --- /dev/null +++ b/doc/build/changelog/unreleased_14/7853.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: bug, engine + :tickets: 7853 + + Further clarified connection-level logging to indicate the BEGIN, ROLLBACK + and COMMIT log messages do not actually indicate a real transaction when + the AUTOCOMMIT isolation level is in use; messaging has been extended to + include the BEGIN message itself, and the messaging has also been fixed to + accommodate when the :class:`.Engine` level + :paramref:`.create_engine.isolation_level` parameter was used directly. diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py index 061794bded..6a79cdf026 100644 --- a/lib/sqlalchemy/engine/base.py +++ b/lib/sqlalchemy/engine/base.py @@ -925,9 +925,14 @@ class Connection(ConnectionEventsTarget, inspection.Inspectable["Inspector"]): ) def _is_autocommit_isolation(self) -> bool: + opt_iso = self._execution_options.get("isolation_level", None) return bool( - self._execution_options.get("isolation_level", None) - == "AUTOCOMMIT" + opt_iso == "AUTOCOMMIT" + or ( + opt_iso is None + and self.engine.dialect._on_connect_isolation_level + == "AUTOCOMMIT" + ) ) def get_transaction(self) -> Optional[RootTransaction]: @@ -949,7 +954,13 @@ class Connection(ConnectionEventsTarget, inspection.Inspectable["Inspector"]): def _begin_impl(self, transaction: RootTransaction) -> None: if self._echo: - self._log_info("BEGIN (implicit)") + if self._is_autocommit_isolation(): + self._log_info( + "BEGIN (implicit; DBAPI should not BEGIN due to " + "autocommit mode)" + ) + else: + self._log_info("BEGIN (implicit)") self.__in_begin = True diff --git a/lib/sqlalchemy/engine/interfaces.py b/lib/sqlalchemy/engine/interfaces.py index 1b178641e1..aa75da6141 100644 --- a/lib/sqlalchemy/engine/interfaces.py +++ b/lib/sqlalchemy/engine/interfaces.py @@ -677,9 +677,14 @@ class Dialect(EventTarget): """ + # NOTE: this does not take into effect engine-level isolation level. + # not clear if this should be changed, seems like it should default_isolation_level: Optional[_IsolationLevel] """the isolation that is implicitly present on new connections""" + # create_engine() -> isolation_level currently goes here + _on_connect_isolation_level: Optional[_IsolationLevel] + execution_ctx_cls: Type["ExecutionContext"] """a :class:`.ExecutionContext` class used to handle statement execution""" diff --git a/test/engine/test_logging.py b/test/engine/test_logging.py index ff8c3fc56d..c6fd856847 100644 --- a/test/engine/test_logging.py +++ b/test/engine/test_logging.py @@ -625,6 +625,13 @@ class TransactionContextLoggingTest(fixtures.TestBase): e.connect().close() return e + @testing.fixture() + def autocommit_iso_logging_engine(self, testing_engine): + kw = {"echo": True, "future": True, "isolation_level": "AUTOCOMMIT"} + e = testing_engine(options=kw) + e.connect().close() + return e + @testing.fixture() def plain_logging_engine(self, testing_engine): # deliver an engine with logging using the plain logging API, @@ -660,6 +667,38 @@ class TransactionContextLoggingTest(fixtures.TestBase): assert_buf(["BEGIN (implicit)", "ROLLBACK"]) + def test_commit_as_you_go_block_commit_engine_level_autocommit( + self, autocommit_iso_logging_engine, assert_buf + ): + with autocommit_iso_logging_engine.connect() as conn: + conn.begin() + conn.commit() + + assert_buf( + [ + "BEGIN (implicit; DBAPI should not " + "BEGIN due to autocommit mode)", + "COMMIT using DBAPI connection.commit(), DBAPI " + "should ignore due to autocommit mode", + ] + ) + + def test_commit_engine_level_autocommit_exec_opt_nonauto( + self, autocommit_iso_logging_engine, assert_buf + ): + with autocommit_iso_logging_engine.execution_options( + isolation_level=testing.db.dialect.default_isolation_level + ).connect() as conn: + conn.begin() + conn.commit() + + assert_buf( + [ + "BEGIN (implicit)", + "COMMIT", + ] + ) + def test_commit_as_you_go_block_commit_autocommit( self, logging_engine, assert_buf ): @@ -671,7 +710,8 @@ class TransactionContextLoggingTest(fixtures.TestBase): assert_buf( [ - "BEGIN (implicit)", + "BEGIN (implicit; DBAPI should not " + "BEGIN due to autocommit mode)", "COMMIT using DBAPI connection.commit(), DBAPI " "should ignore due to autocommit mode", ] @@ -688,7 +728,8 @@ class TransactionContextLoggingTest(fixtures.TestBase): assert_buf( [ - "BEGIN (implicit)", + "BEGIN (implicit; DBAPI should not " + "BEGIN due to autocommit mode)", "ROLLBACK using DBAPI connection.rollback(), DBAPI " "should ignore due to autocommit mode", ]