From: Mike Bayer Date: Tue, 17 Sep 2019 19:26:32 +0000 (-0400) Subject: Add autocommit_block X-Git-Tag: rel_1_2_0~16 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1dc690d40cbb61496ad8de1f1f225ddaa18f7351;p=thirdparty%2Fsqlalchemy%2Falembic.git Add autocommit_block Added new feature :meth:`.MigrationContext.autocommit_block`, a special directive which will provide for a non-transactional block inside of a migration script. The feature requres that: the database driver (e.g. DBAPI) supports the AUTOCOMMIT isolation mode. The directive also necessarily needs to COMMIT the existing transaction in progress in order to enter autocommit mode. Change-Id: I107fe9772595db189b6ebeba6535ac8f275b3fe5 Fixes: #123 --- diff --git a/alembic/runtime/migration.py b/alembic/runtime/migration.py index aabfc499..61bceeb9 100644 --- a/alembic/runtime/migration.py +++ b/alembic/runtime/migration.py @@ -20,6 +20,27 @@ from ..util.compat import EncodedIO log = logging.getLogger(__name__) +class _ProxyTransaction(object): + def __init__(self, migration_context): + self.migration_context = migration_context + + @property + def _proxied_transaction(self): + return self.migration_context._transaction + + def rollback(self): + self._proxied_transaction.rollback() + + def commit(self): + self._proxied_transaction.commit() + + def __enter__(self): + return self + + def __exit__(self, type_, value, traceback): + self._proxied_transaction.__exit__(type_, value, traceback) + + class MigrationContext(object): """Represent the database state made available to a migration @@ -78,6 +99,7 @@ class MigrationContext(object): "transaction_per_migration", False ) self.on_version_apply_callbacks = opts.get("on_version_apply", ()) + self._transaction = None if as_sql: self.connection = self._stdout_connection(connection) @@ -193,7 +215,124 @@ class MigrationContext(object): return MigrationContext(dialect, connection, opts, environment_context) + @contextmanager + def autocommit_block(self): + """Enter an "autocommit" block, for databases that support AUTOCOMMIT + isolation levels. + + This special directive is intended to support the occasional database + DDL or system operation that specifically has to be run outside of + any kind of transaction block. The PostgreSQL database platform + is the most common target for this style of operation, as many + of its DDL operations must be run outside of transaction blocks, even + though the database overall supports transactional DDL. + + The method is used as a context manager within a migration script, by + calling on :meth:`.Operations.get_context` to retrieve the + :class:`.MigrationContext`, then invoking + :meth:`.MigrationContext.autocommit_block` using the ``with:`` + statement:: + + def upgrade(): + with op.get_context().autocommit_block(): + op.execute("ALTER TYPE mood ADD VALUE 'soso'") + + Above, a PostgreSQL "ALTER TYPE..ADD VALUE" directive is emitted, + which must be run outside of a transaction block at the database level. + The :meth:`.MigrationContext.autocommit_block` method makes use of the + SQLAlchemy ``AUTOCOMMIT`` isolation level setting, which against the + psycogp2 DBAPI corresponds to the ``connection.autocommit`` setting, + to ensure that the database driver is not inside of a DBAPI level + transaction block. + + .. warning:: + + As is necessary, **the database transaction preceding the block is + unconditionally committed**. This means that the run of migrations + preceding the operation will be committed, before the overall + migration operation is complete. + + It is recommended that when an application includes migrations with + "autocommit" blocks, that + :paramref:`.EnvironmentContext.transaction_per_migration` be used + so that the calling environment is tuned to expect short per-file + migrations whether or not one of them has an autocommit block. + + + .. versionadded:: 1.1.1 + + """ + _in_connection_transaction = self._in_connection_transaction() + + if self.impl.transactional_ddl: + if self.as_sql: + self.impl.emit_commit() + + elif _in_connection_transaction: + assert self._transaction is not None + + self._transaction.commit() + self._transaction = None + + if not self.as_sql: + current_level = self.connection.get_isolation_level() + self.connection.execution_options(isolation_level="AUTOCOMMIT") + try: + yield + finally: + if not self.as_sql: + self.connection.execution_options( + isolation_level=current_level + ) + + if self.impl.transactional_ddl: + if self.as_sql: + self.impl.emit_begin() + + elif _in_connection_transaction: + self._transaction = self.bind.begin() + def begin_transaction(self, _per_migration=False): + """Begin a logical transaction for migration operations. + + This method is used within an ``env.py`` script to demarcate where + the outer "transaction" for a series of migrations begins. Example:: + + def run_migrations_online(): + connectable = create_engine(...) + + with connectable.connect() as connection: + context.configure( + connection=connection, target_metadata=target_metadata + ) + + with context.begin_transaction(): + context.run_migrations() + + Above, :meth:`.MigrationContext.begin_transaction` is used to demarcate + where the outer logical transaction occurs around the + :meth:`.MigrationContext.run_migrations` operation. + + A "Logical" transaction means that the operation may or may not + correspond to a real database transaction. If the target database + supports transactional DDL (or + :paramref:`.EnvironmentContext.configure.transactional_ddl` is true), + the :paramref:`.EnvironmentContext.configure.transaction_per_migration` + flag is not set, and the migration is against a real database + connection (as opposed to using "offline" ``--sql`` mode), a real + transaction will be started. If ``--sql`` mode is in effect, the + operation would instead correspond to a string such as "BEGIN" being + emitted to the string output. + + The returned object is a Python context manager that should only be + used in the context of a ``with:`` statement as indicated above. + The object has no other guaranteed API features present. + + .. seealso:: + + :meth:`.MigrationContext.autocommit_block` + + """ transaction_now = _per_migration == self._transaction_per_migration if not transaction_now: @@ -221,7 +360,8 @@ class MigrationContext(object): return begin_commit() else: - return self.bind.begin() + self._transaction = self.bind.begin() + return _ProxyTransaction(self) def get_current_revision(self): """Return the current revision, usually that which is present diff --git a/alembic/testing/__init__.py b/alembic/testing/__init__.py index ad0ae4c4..4b669268 100644 --- a/alembic/testing/__init__.py +++ b/alembic/testing/__init__.py @@ -12,6 +12,8 @@ from .assertions import emits_python_deprecation_warning # noqa from .assertions import eq_ # noqa from .assertions import eq_ignore_whitespace # noqa from .assertions import is_ # noqa +from .assertions import is_false # noqa from .assertions import is_not_ # noqa +from .assertions import is_true # noqa from .assertions import ne_ # noqa from .fixtures import TestBase # noqa diff --git a/alembic/testing/assertions.py b/alembic/testing/assertions.py index 750c526b..3dc08f0b 100644 --- a/alembic/testing/assertions.py +++ b/alembic/testing/assertions.py @@ -8,7 +8,9 @@ from sqlalchemy.testing.assertions import assert_raises # noqa from sqlalchemy.testing.assertions import assert_raises_message # noqa from sqlalchemy.testing.assertions import eq_ # noqa from sqlalchemy.testing.assertions import is_ # noqa +from sqlalchemy.testing.assertions import is_false # noqa from sqlalchemy.testing.assertions import is_not_ # noqa +from sqlalchemy.testing.assertions import is_true # noqa from sqlalchemy.testing.assertions import ne_ # noqa from sqlalchemy.util import decorator diff --git a/alembic/testing/requirements.py b/alembic/testing/requirements.py index c8d76b85..9038a45c 100644 --- a/alembic/testing/requirements.py +++ b/alembic/testing/requirements.py @@ -15,6 +15,12 @@ class SuiteRequirements(Requirements): return exclusions.open() + @property + def autocommit_isolation(self): + """target database should support 'AUTOCOMMIT' isolation level""" + + return exclusions.closed() + @property def unique_constraint_reflection(self): def doesnt_have_check_uq_constraints(config): diff --git a/docs/build/unreleased/123.rst b/docs/build/unreleased/123.rst new file mode 100644 index 00000000..1d0e8d44 --- /dev/null +++ b/docs/build/unreleased/123.rst @@ -0,0 +1,14 @@ +.. change:: + :tags: feature, runtime + :tickets: 123 + + Added new feature :meth:`.MigrationContext.autocommit_block`, a special + directive which will provide for a non-transactional block inside of a + migration script. The feature requres that: the database driver + (e.g. DBAPI) supports the AUTOCOMMIT isolation mode. The directive + also necessarily needs to COMMIT the existing transaction in progress + in order to enter autocommit mode. + + .. seealso:: + + :meth:`.MigrationContext.autocommit_block` diff --git a/tests/requirements.py b/tests/requirements.py index e9e1b078..e07a5171 100644 --- a/tests/requirements.py +++ b/tests/requirements.py @@ -129,6 +129,12 @@ class DefaultRequirements(SuiteRequirements): """if a compare of Integer and BigInteger is supported yet.""" return exclusions.skip_if(["oracle"], "not supported by alembic impl") + @property + def autocommit_isolation(self): + """target database should support 'AUTOCOMMIT' isolation level""" + + return exclusions.only_on("postgresql", "mysql") + @property def check_constraint_reflection(self): return exclusions.fails_on_everything_except( diff --git a/tests/test_environment.py b/tests/test_environment.py index 5e074672..68eab3d2 100644 --- a/tests/test_environment.py +++ b/tests/test_environment.py @@ -3,9 +3,12 @@ from alembic import command from alembic.environment import EnvironmentContext from alembic.migration import MigrationContext from alembic.script import ScriptDirectory +from alembic.testing import assert_raises from alembic.testing import config from alembic.testing import eq_ from alembic.testing import is_ +from alembic.testing import is_false +from alembic.testing import is_true from alembic.testing import mock from alembic.testing.assertions import expect_warnings from alembic.testing.env import _no_sql_testing_config @@ -15,6 +18,7 @@ from alembic.testing.env import staging_env from alembic.testing.env import write_script from alembic.testing.fixtures import capture_context_buffer from alembic.testing.fixtures import TestBase +from alembic.util import compat class EnvironmentTest(TestBase): @@ -135,3 +139,292 @@ def downgrade(): env.run_migrations() eq_(migration_fn.mock_calls, [mock.call((), env._migration_context)]) + + +class MigrationTransactionTest(TestBase): + __backend__ = True + + conn = None + + def _fixture(self, opts): + self.conn = conn = config.db.connect() + + if opts.get("as_sql", False): + self.context = MigrationContext.configure( + dialect=conn.dialect, opts=opts + ) + self.context.output_buffer = ( + self.context.impl.output_buffer + ) = compat.StringIO() + else: + self.context = MigrationContext.configure( + connection=conn, opts=opts + ) + return self.context + + def teardown(self): + if self.conn: + self.conn.close() + + def test_proxy_transaction_rollback(self): + context = self._fixture( + {"transaction_per_migration": True, "transactional_ddl": True} + ) + + is_false(self.conn.in_transaction()) + proxy = context.begin_transaction(_per_migration=True) + is_true(self.conn.in_transaction()) + proxy.rollback() + is_false(self.conn.in_transaction()) + + def test_proxy_transaction_commit(self): + context = self._fixture( + {"transaction_per_migration": True, "transactional_ddl": True} + ) + proxy = context.begin_transaction(_per_migration=True) + is_true(self.conn.in_transaction()) + proxy.commit() + is_false(self.conn.in_transaction()) + + def test_proxy_transaction_contextmanager_commit(self): + context = self._fixture( + {"transaction_per_migration": True, "transactional_ddl": True} + ) + proxy = context.begin_transaction(_per_migration=True) + is_true(self.conn.in_transaction()) + with proxy: + pass + is_false(self.conn.in_transaction()) + + def test_proxy_transaction_contextmanager_rollback(self): + context = self._fixture( + {"transaction_per_migration": True, "transactional_ddl": True} + ) + proxy = context.begin_transaction(_per_migration=True) + is_true(self.conn.in_transaction()) + + def go(): + with proxy: + raise Exception("hi") + + assert_raises(Exception, go) + is_false(self.conn.in_transaction()) + + def test_transaction_per_migration_transactional_ddl(self): + context = self._fixture( + {"transaction_per_migration": True, "transactional_ddl": True} + ) + + is_false(self.conn.in_transaction()) + + with context.begin_transaction(): + is_false(self.conn.in_transaction()) + with context.begin_transaction(_per_migration=True): + is_true(self.conn.in_transaction()) + + is_false(self.conn.in_transaction()) + is_false(self.conn.in_transaction()) + + def test_transaction_per_migration_non_transactional_ddl(self): + context = self._fixture( + {"transaction_per_migration": True, "transactional_ddl": False} + ) + + is_false(self.conn.in_transaction()) + + with context.begin_transaction(): + is_false(self.conn.in_transaction()) + with context.begin_transaction(_per_migration=True): + is_false(self.conn.in_transaction()) + + is_false(self.conn.in_transaction()) + is_false(self.conn.in_transaction()) + + def test_transaction_per_all_transactional_ddl(self): + context = self._fixture({"transactional_ddl": True}) + + is_false(self.conn.in_transaction()) + + with context.begin_transaction(): + is_true(self.conn.in_transaction()) + with context.begin_transaction(_per_migration=True): + is_true(self.conn.in_transaction()) + + is_true(self.conn.in_transaction()) + is_false(self.conn.in_transaction()) + + def test_transaction_per_all_non_transactional_ddl(self): + context = self._fixture({"transactional_ddl": False}) + + is_false(self.conn.in_transaction()) + + with context.begin_transaction(): + is_false(self.conn.in_transaction()) + with context.begin_transaction(_per_migration=True): + is_false(self.conn.in_transaction()) + + is_false(self.conn.in_transaction()) + is_false(self.conn.in_transaction()) + + def test_transaction_per_all_sqlmode(self): + context = self._fixture({"as_sql": True}) + + context.execute("step 1") + with context.begin_transaction(): + context.execute("step 2") + with context.begin_transaction(_per_migration=True): + context.execute("step 3") + + context.execute("step 4") + context.execute("step 5") + + if context.impl.transactional_ddl: + self._assert_impl_steps( + "step 1", + "BEGIN", + "step 2", + "step 3", + "step 4", + "COMMIT", + "step 5", + ) + else: + self._assert_impl_steps( + "step 1", "step 2", "step 3", "step 4", "step 5" + ) + + def test_transaction_per_migration_sqlmode(self): + context = self._fixture( + {"as_sql": True, "transaction_per_migration": True} + ) + + context.execute("step 1") + with context.begin_transaction(): + context.execute("step 2") + with context.begin_transaction(_per_migration=True): + context.execute("step 3") + + context.execute("step 4") + context.execute("step 5") + + if context.impl.transactional_ddl: + self._assert_impl_steps( + "step 1", + "step 2", + "BEGIN", + "step 3", + "COMMIT", + "step 4", + "step 5", + ) + else: + self._assert_impl_steps( + "step 1", "step 2", "step 3", "step 4", "step 5" + ) + + @config.requirements.autocommit_isolation + def test_autocommit_block(self): + context = self._fixture({"transaction_per_migration": True}) + + is_false(self.conn.in_transaction()) + + with context.begin_transaction(): + is_false(self.conn.in_transaction()) + with context.begin_transaction(_per_migration=True): + if context.impl.transactional_ddl: + is_true(self.conn.in_transaction()) + else: + is_false(self.conn.in_transaction()) + + with context.autocommit_block(): + is_false(self.conn.in_transaction()) + + if context.impl.transactional_ddl: + is_true(self.conn.in_transaction()) + else: + is_false(self.conn.in_transaction()) + + is_false(self.conn.in_transaction()) + is_false(self.conn.in_transaction()) + + @config.requirements.autocommit_isolation + def test_autocommit_block_no_transaction(self): + context = self._fixture({"transaction_per_migration": True}) + + is_false(self.conn.in_transaction()) + + with context.autocommit_block(): + is_false(self.conn.in_transaction()) + is_false(self.conn.in_transaction()) + + def test_autocommit_block_transactional_ddl_sqlmode(self): + context = self._fixture( + { + "transaction_per_migration": True, + "transactional_ddl": True, + "as_sql": True, + } + ) + + with context.begin_transaction(): + context.execute("step 1") + with context.begin_transaction(_per_migration=True): + context.execute("step 2") + + with context.autocommit_block(): + context.execute("step 3") + + context.execute("step 4") + + context.execute("step 5") + + self._assert_impl_steps( + "step 1", + "BEGIN", + "step 2", + "COMMIT", + "step 3", + "BEGIN", + "step 4", + "COMMIT", + "step 5", + ) + + def test_autocommit_block_nontransactional_ddl_sqlmode(self): + context = self._fixture( + { + "transaction_per_migration": True, + "transactional_ddl": False, + "as_sql": True, + } + ) + + with context.begin_transaction(): + context.execute("step 1") + with context.begin_transaction(_per_migration=True): + context.execute("step 2") + + with context.autocommit_block(): + context.execute("step 3") + + context.execute("step 4") + + context.execute("step 5") + + self._assert_impl_steps( + "step 1", "step 2", "step 3", "step 4", "step 5" + ) + + def _assert_impl_steps(self, *steps): + to_check = self.context.output_buffer.getvalue() + + self.context.impl.output_buffer = buf = compat.StringIO() + for step in steps: + if step == "BEGIN": + self.context.impl.emit_begin() + elif step == "COMMIT": + self.context.impl.emit_commit() + else: + self.context.impl._exec(step) + + eq_(to_check, buf.getvalue()) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 28de4873..abc43ea7 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -269,6 +269,24 @@ class PostgresqlOpTest(TestBase): context.assert_("COMMENT ON TABLE foo.t2 IS NULL") +class PGAutocommitBlockTest(TestBase): + def setUp(self): + self.conn = conn = config.db.connect() + + with conn.begin(): + conn.execute("CREATE TYPE mood AS ENUM ('sad', 'ok', 'happy');") + + def tearDown(self): + with self.conn.begin(): + self.conn.execute("DROP TYPE mood") + + def test_alter_enum(self): + context = MigrationContext.configure(connection=self.conn) + with context.begin_transaction(_per_migration=True): + with context.autocommit_block(): + context.execute("ALTER TYPE mood ADD VALUE 'soso'") + + class PGOfflineEnumTest(TestBase): def setUp(self): staging_env()