diff options
| author | Mike Bayer <mike_mp@zzzcomputing.com> | 2022-03-24 13:58:20 -0400 |
|---|---|---|
| committer | Mike Bayer <mike_mp@zzzcomputing.com> | 2022-03-24 13:59:02 -0400 |
| commit | 56366924673f88e51c74d94058c11132a057ecfa (patch) | |
| tree | 1d96a3c5f86b77f0879c2f6c3624653919400def | |
| parent | c565c470517e1cc70a7f33d1ad3d3256935f1121 (diff) | |
| download | sqlalchemy-56366924673f88e51c74d94058c11132a057ecfa.tar.gz | |
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
| -rw-r--r-- | doc/build/changelog/unreleased_14/7853.rst | 10 | ||||
| -rw-r--r-- | lib/sqlalchemy/engine/base.py | 17 | ||||
| -rw-r--r-- | lib/sqlalchemy/engine/interfaces.py | 5 | ||||
| -rw-r--r-- | test/engine/test_logging.py | 45 |
4 files changed, 72 insertions, 5 deletions
diff --git a/doc/build/changelog/unreleased_14/7853.rst b/doc/build/changelog/unreleased_14/7853.rst new file mode 100644 index 000000000..66856c29e --- /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 061794bde..6a79cdf02 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 1b178641e..aa75da614 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 ff8c3fc56..c6fd85684 100644 --- a/test/engine/test_logging.py +++ b/test/engine/test_logging.py @@ -626,6 +626,13 @@ class TransactionContextLoggingTest(fixtures.TestBase): 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, # not the echo parameter @@ -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", ] |
