Don't raise on open transaction if we already started in one

An adjustment to the bug fix for :ticket:`369` to accommodate for
env.py scripts that use an enclosing transaction distinct from the
one that the context provides, so that the check for "didn't commit
the transaction" doesn't trigger in this scenario.

Change-Id: I3c1fa614495b61532999a84b2af3773e4d33c30b
Fixes: #417

diff --git a/alembic/runtime/migration.py b/alembic/runtime/migration.py
index 0f1d88e0a5ecdcf720d3a8afc16990df1c357bff..d13b999666f7576c3b6aa157bbf670bcfd6a7f2c 100644 (file)
--- a/alembic/runtime/migration.py
+++ b/alembic/runtime/migration.py
@@ -308,6 +308,9 @@ class MigrationContext(object):
 
         head_maintainer = HeadMaintainer(self, heads)
 
+        starting_in_transaction = not self.as_sql and \
+            self.connection.in_transaction()
+
         for step in self._migrations_fn(heads, self):
             with self.begin_transaction(_per_migration=True):
                 if self.as_sql and not head_maintainer.heads:
@@ -326,7 +329,8 @@ class MigrationContext(object):
                 # just to run the operations on every version
                 head_maintainer.update_to_step(step)
 
-            if not self.as_sql and not self.impl.transactional_ddl and \
+            if not starting_in_transaction and not self.as_sql and \
+                    not self.impl.transactional_ddl and \
                     self.connection.in_transaction():
                 raise util.CommandError(
                     "Migration \"%s\" has left an uncommitted "

diff --git a/docs/build/changelog.rst b/docs/build/changelog.rst
index 3a4f0e2a08288dabe94f0910a9b66dfe9fcaf5ae..fcd9aae63a05ed97762c7a272b1e61123e8e6aa8 100644 (file)
--- a/docs/build/changelog.rst
+++ b/docs/build/changelog.rst
@@ -7,6 +7,15 @@ Changelog
     :version: 0.9.1
     :released:
 
+    .. change:: 417
+      :tags: bug, commands
+      :tickets: 417, 369
+
+      An adjustment to the bug fix for :ticket:`369` to accommodate for
+      env.py scripts that use an enclosing transaction distinct from the
+      one that the context provides, so that the check for "didn't commit
+      the transaction" doesn't trigger in this scenario.
+
 .. changelog::
     :version: 0.9.0
     :released: February 28, 2017

diff --git a/tests/test_script_consumption.py b/tests/test_script_consumption.py
index 6a229584626b0cc2fca11344a422a597f63ddcbc..b313273af78384b71526f562147e4d6c290ba11b 100644 (file)
--- a/tests/test_script_consumption.py
+++ b/tests/test_script_consumption.py
@@ -8,7 +8,7 @@ from alembic.util import compat
 from alembic.script import ScriptDirectory, Script
 from alembic.testing.env import clear_staging_env, staging_env, \
     _sqlite_testing_config, write_script, _sqlite_file_db, \
-    three_rev_fixture, _no_sql_testing_config
+    three_rev_fixture, _no_sql_testing_config, env_file_fixture
 from alembic.testing import eq_, assert_raises_message
 from alembic.testing.fixtures import TestBase, capture_context_buffer
 from alembic.environment import EnvironmentContext
@@ -281,6 +281,35 @@ def downgrade():
                 transactional_ddl=True, transaction_per_migration=False):
             command.upgrade(self.cfg, c)
 
+    def test_noerr_transaction_opened_externally(self):
+        a, b, c = self._opened_transaction_fixture()
+
+        env_file_fixture("""
+from sqlalchemy import engine_from_config, pool
+
+def run_migrations_online():
+    connectable = engine_from_config(
+        config.get_section(config.config_ini_section),
+        prefix='sqlalchemy.',
+        poolclass=pool.NullPool)
+
+    with connectable.connect() as connection:
+        with connection.begin() as real_trans:
+            context.configure(
+                connection=connection,
+                transactional_ddl=False,
+                transaction_per_migration=False
+            )
+
+            with context.begin_transaction():
+                context.run_migrations()
+
+run_migrations_online()
+
+""")
+
+        command.stamp(self.cfg, c)
+
 
 class EncodingTest(TestBase):