Skip to content

Commit

Permalink
Don't raise on open transaction if we already started in one
Browse files Browse the repository at this point in the history
An adjustment to the bug fix for 🎫`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
  • Loading branch information
zzzeek committed Mar 1, 2017
1 parent 910fc22 commit cfb9d34
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 2 deletions.
6 changes: 5 additions & 1 deletion alembic/runtime/migration.py
Expand Up @@ -308,6 +308,9 @@ def run_migrations(self, **kw):

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:
Expand All @@ -326,7 +329,8 @@ def run_migrations(self, **kw):
# 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 "
Expand Down
9 changes: 9 additions & 0 deletions docs/build/changelog.rst
Expand Up @@ -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
Expand Down
31 changes: 30 additions & 1 deletion tests/test_script_consumption.py
Expand Up @@ -8,7 +8,7 @@
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
Expand Down Expand Up @@ -281,6 +281,35 @@ def test_noerr_rev_leaves_open_transaction_transactional_ddl(self):
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):

Expand Down

0 comments on commit cfb9d34

Please sign in to comment.