Warn on non-Connection present and accommodate for Engine
A warning is emitted when an object that's not a :class:`~sqlalchemy.engine.Connection` is passed to :meth:`.EnvironmentContext.configure`. For the case of a :class:`~sqlalchemy.engine.Engine` passed, the check for "in transaction" introduced in version 0.9.0 has been relaxed to work in the case of an attribute error, as some users appear to be passing an :class:`~sqlalchemy.engine.Engine` and not a :class:`~sqlalchemy.engine.Connection`. Change-Id: I95ef38955c00511d3055362a03284fb91677595f Fixes: #419
This commit is contained in:
parent
5191bc1e65
commit
adf0a7d1ea
|
@ -6,6 +6,7 @@ from sqlalchemy import MetaData, Table, Column, String, literal_column,\
|
|||
PrimaryKeyConstraint
|
||||
from sqlalchemy.engine.strategies import MockEngineStrategy
|
||||
from sqlalchemy.engine import url as sqla_url
|
||||
from sqlalchemy.engine import Connection
|
||||
|
||||
from ..util.compat import callable, EncodedIO
|
||||
from .. import ddl, util
|
||||
|
@ -152,6 +153,11 @@ class MigrationContext(object):
|
|||
opts = {}
|
||||
|
||||
if connection:
|
||||
if not isinstance(connection, Connection):
|
||||
util.warn(
|
||||
"'connection' argument to configure() is expected "
|
||||
"to be a sqlalchemy.engine.Connection instance, "
|
||||
"got %r" % connection)
|
||||
dialect = connection.dialect
|
||||
elif url:
|
||||
url = sqla_url.make_url(url)
|
||||
|
@ -309,7 +315,7 @@ class MigrationContext(object):
|
|||
head_maintainer = HeadMaintainer(self, heads)
|
||||
|
||||
starting_in_transaction = not self.as_sql and \
|
||||
self.connection.in_transaction()
|
||||
self._in_connection_transaction()
|
||||
|
||||
for step in self._migrations_fn(heads, self):
|
||||
with self.begin_transaction(_per_migration=True):
|
||||
|
@ -330,8 +336,8 @@ class MigrationContext(object):
|
|||
head_maintainer.update_to_step(step)
|
||||
|
||||
if not starting_in_transaction and not self.as_sql and \
|
||||
not self.impl.transactional_ddl and \
|
||||
self.connection.in_transaction():
|
||||
not self.impl.transactional_ddl and \
|
||||
self._in_connection_transaction():
|
||||
raise util.CommandError(
|
||||
"Migration \"%s\" has left an uncommitted "
|
||||
"transaction opened; transactional_ddl is False so "
|
||||
|
@ -341,6 +347,14 @@ class MigrationContext(object):
|
|||
if self.as_sql and not head_maintainer.heads:
|
||||
self._version.drop(self.connection)
|
||||
|
||||
def _in_connection_transaction(self):
|
||||
try:
|
||||
meth = self.connection.in_transaction
|
||||
except AttributeError:
|
||||
return False
|
||||
else:
|
||||
return meth()
|
||||
|
||||
def execute(self, sql, execution_options=None):
|
||||
"""Execute a SQL construct or string statement.
|
||||
|
||||
|
|
|
@ -7,6 +7,19 @@ Changelog
|
|||
:version: 0.9.2
|
||||
:released:
|
||||
|
||||
.. change:: 419
|
||||
:tags: bug environment
|
||||
:tickets: 419
|
||||
|
||||
A warning is emitted when an object that's not a
|
||||
:class:`~sqlalchemy.engine.Connection` is passed to
|
||||
:meth:`.EnvironmentContext.configure`. For the case of a
|
||||
:class:`~sqlalchemy.engine.Engine` passed, the check for "in transaction"
|
||||
introduced in version 0.9.0 has been relaxed to work in the case of an
|
||||
attribute error, as some users appear to be passing an
|
||||
:class:`~sqlalchemy.engine.Engine` and not a
|
||||
:class:`~sqlalchemy.engine.Connection`.
|
||||
|
||||
.. changelog::
|
||||
:version: 0.9.1
|
||||
:released: March 1, 2017
|
||||
|
|
|
@ -4,9 +4,10 @@ from alembic.script import ScriptDirectory
|
|||
from alembic.environment import EnvironmentContext
|
||||
from alembic.migration import MigrationContext
|
||||
from alembic.testing.fixtures import TestBase
|
||||
from alembic.testing.mock import Mock, call
|
||||
from alembic.testing.mock import Mock, call, MagicMock
|
||||
from alembic.testing.env import _no_sql_testing_config, \
|
||||
staging_env, clear_staging_env
|
||||
staging_env, clear_staging_env, write_script, _sqlite_file_db
|
||||
from alembic.testing.assertions import expect_warnings
|
||||
|
||||
from alembic.testing import eq_, is_
|
||||
|
||||
|
@ -74,3 +75,46 @@ class EnvironmentTest(TestBase):
|
|||
|
||||
ctx = MigrationContext(ctx.dialect, None, {})
|
||||
is_(ctx.config, None)
|
||||
|
||||
def test_warning_on_passing_engine(self):
|
||||
env = self._fixture()
|
||||
|
||||
engine = _sqlite_file_db()
|
||||
|
||||
a_rev = 'arev'
|
||||
env.script.generate_revision(a_rev, "revision a", refresh=True)
|
||||
write_script(env.script, a_rev, """\
|
||||
"Rev A"
|
||||
revision = '%s'
|
||||
down_revision = None
|
||||
|
||||
from alembic import op
|
||||
|
||||
|
||||
def upgrade():
|
||||
pass
|
||||
|
||||
|
||||
def downgrade():
|
||||
pass
|
||||
|
||||
""" % a_rev)
|
||||
migration_fn = MagicMock()
|
||||
|
||||
def upgrade(rev, context):
|
||||
migration_fn(rev, context)
|
||||
return env.script._upgrade_revs(a_rev, rev)
|
||||
|
||||
with expect_warnings(
|
||||
r"'connection' argument to configure\(\) is "
|
||||
r"expected to be a sqlalchemy.engine.Connection "):
|
||||
env.configure(
|
||||
connection=engine, fn=upgrade,
|
||||
transactional_ddl=False)
|
||||
|
||||
env.run_migrations()
|
||||
|
||||
eq_(
|
||||
migration_fn.mock_calls,
|
||||
[call((), env._migration_context)]
|
||||
)
|
||||
|
|
Loading…
Reference in New Issue