From: Mike Bayer Date: Sat, 20 Jul 2019 16:54:37 +0000 (-0400) Subject: Add dialect_options to environment; set paramstyle=named for offline X-Git-Tag: rel_1_1_0~12 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7995199012173d77e2dcaf02d4ded5d2d7a6f634;p=thirdparty%2Fsqlalchemy%2Falembic.git Add dialect_options to environment; set paramstyle=named for offline Fixed bug where the double-percent logic applied to some dialects such as psycopg2 would be rendered in ``--sql`` mode, by allowing dialect options to be passed through to the dialect used to generate SQL and then providing ``paramstyle="named"`` so that percent signs need not be doubled. For users having this issue, existing env.py scripts need to add ``dialect_opts={"paramstyle": "named"}`` to their offline context.configure(). See the ``alembic/templates/generic/env.py`` template for an example. Change-Id: Ia6a495704b029eaff43fb3b6df602ca667002b7a Fixes: #562 --- diff --git a/alembic/runtime/environment.py b/alembic/runtime/environment.py index c4d6586b..b918269e 100644 --- a/alembic/runtime/environment.py +++ b/alembic/runtime/environment.py @@ -289,6 +289,7 @@ class EnvironmentContext(util.ModuleClsProxy): connection=None, url=None, dialect_name=None, + dialect_opts=None, transactional_ddl=None, transaction_per_migration=False, output_buffer=None, @@ -355,6 +356,11 @@ class EnvironmentContext(util.ModuleClsProxy): "postgresql", "mssql", etc. The type of dialect to be used will be derived from this if ``connection`` and ``url`` are not passed. + :param dialect_opts: dictionary of options to be passed to dialect + constructor. + + .. versionadded:: 1.0.11 + :param transactional_ddl: Force the usage of "transactional" DDL on or off; this otherwise defaults to whether or not the dialect in @@ -812,6 +818,7 @@ class EnvironmentContext(util.ModuleClsProxy): url=url, dialect_name=dialect_name, environment_context=self, + dialect_opts=dialect_opts, opts=opts, ) diff --git a/alembic/runtime/migration.py b/alembic/runtime/migration.py index b3a98633..aabfc499 100644 --- a/alembic/runtime/migration.py +++ b/alembic/runtime/migration.py @@ -145,6 +145,7 @@ class MigrationContext(object): dialect_name=None, dialect=None, environment_context=None, + dialect_opts=None, opts=None, ): """Create a new :class:`.MigrationContext`. @@ -169,6 +170,8 @@ class MigrationContext(object): """ if opts is None: opts = {} + if dialect_opts is None: + dialect_opts = {} if connection: if not isinstance(connection, Connection): @@ -176,15 +179,15 @@ class MigrationContext(object): "'connection' argument to configure() is expected " "to be a sqlalchemy.engine.Connection instance, " "got %r" % connection, - stacklevel=3 + stacklevel=3, ) dialect = connection.dialect elif url: url = sqla_url.make_url(url) - dialect = url.get_dialect()() + dialect = url.get_dialect()(**dialect_opts) elif dialect_name: url = sqla_url.make_url("%s://" % dialect_name) - dialect = url.get_dialect()() + dialect = url.get_dialect()(**dialect_opts) elif not dialect: raise Exception("Connection, url, or dialect_name is required.") diff --git a/alembic/templates/generic/env.py b/alembic/templates/generic/env.py index 15cb4725..70518a2e 100644 --- a/alembic/templates/generic/env.py +++ b/alembic/templates/generic/env.py @@ -1,4 +1,3 @@ - from logging.config import fileConfig from sqlalchemy import engine_from_config @@ -40,7 +39,10 @@ def run_migrations_offline(): """ url = config.get_main_option("sqlalchemy.url") context.configure( - url=url, target_metadata=target_metadata, literal_binds=True + url=url, + target_metadata=target_metadata, + literal_binds=True, + dialect_opts={"paramstyle": "named"}, ) with context.begin_transaction(): diff --git a/alembic/templates/multidb/env.py b/alembic/templates/multidb/env.py index 607efaa0..f1a9a9ad 100644 --- a/alembic/templates/multidb/env.py +++ b/alembic/templates/multidb/env.py @@ -1,4 +1,3 @@ - import logging from logging.config import fileConfig import re @@ -73,6 +72,7 @@ def run_migrations_offline(): output_buffer=buffer, target_metadata=target_metadata.get(name), literal_binds=True, + dialect_opts={"paramstyle": "named"}, ) with context.begin_transaction(): context.run_migrations(engine_name=name) diff --git a/alembic/templates/pylons/env.py b/alembic/templates/pylons/env.py index f8abf447..b2d610d7 100644 --- a/alembic/templates/pylons/env.py +++ b/alembic/templates/pylons/env.py @@ -53,6 +53,7 @@ def run_migrations_offline(): url=meta.engine.url, target_metadata=target_metadata, literal_binds=True, + dialect_opts={"paramstyle": "named"}, ) with context.begin_transaction(): context.run_migrations() diff --git a/alembic/testing/requirements.py b/alembic/testing/requirements.py index cf570a5c..b4c147be 100644 --- a/alembic/testing/requirements.py +++ b/alembic/testing/requirements.py @@ -61,6 +61,14 @@ class SuiteRequirements(Requirements): def reflects_fk_options(self): return exclusions.closed() + @property + def sqlalchemy_issue_3740(self): + """Fixes percent sign escaping for paramstyles that don't require it""" + return exclusions.skip_if( + lambda config: not util.sqla_120, + "SQLAlchemy 1.2 or greater required", + ) + @property def sqlalchemy_12(self): return exclusions.skip_if( diff --git a/docs/build/unreleased/562.rst b/docs/build/unreleased/562.rst new file mode 100644 index 00000000..7cd8fc1b --- /dev/null +++ b/docs/build/unreleased/562.rst @@ -0,0 +1,12 @@ +.. change:: + :tags: bug, commands + :tickets: 562 + + Fixed bug where the double-percent logic applied to some dialects such as + psycopg2 would be rendered in ``--sql`` mode, by allowing dialect options + to be passed through to the dialect used to generate SQL and then providing + ``paramstyle="named"`` so that percent signs need not be doubled. For + users having this issue, existing env.py scripts need to add + ``dialect_opts={"paramstyle": "named"}`` to their offline + context.configure(). See the ``alembic/templates/generic/env.py`` template + for an example. diff --git a/tests/test_environment.py b/tests/test_environment.py index 6109ed76..fadab54f 100644 --- a/tests/test_environment.py +++ b/tests/test_environment.py @@ -1,8 +1,9 @@ #!coding: utf-8 - +from alembic import command from alembic.environment import EnvironmentContext from alembic.migration import MigrationContext from alembic.script import ScriptDirectory +from alembic.testing import config from alembic.testing import eq_ from alembic.testing import is_ from alembic.testing.assertions import expect_warnings @@ -11,6 +12,7 @@ from alembic.testing.env import _sqlite_file_db from alembic.testing.env import clear_staging_env 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.testing.mock import call from alembic.testing.mock import MagicMock @@ -61,6 +63,35 @@ class EnvironmentTest(TestBase): ctx = MigrationContext(ctx.dialect, None, {}) is_(ctx.config, None) + @config.requirements.sqlalchemy_issue_3740 + def test_sql_mode_parameters(self): + env = self._fixture() + + a_rev = "arev" + env.script.generate_revision(a_rev, "revision a", refresh=True) + write_script( + env.script, + a_rev, + """\ +"Rev A" +revision = '{}' +down_revision = None + +from alembic import op + +def upgrade(): + op.execute(''' + do some SQL thing with a % percent sign % + ''') + +""".format( + a_rev + ), + ) + with capture_context_buffer(transactional_ddl=True) as buf: + command.upgrade(self.cfg, "arev", sql=True) + assert "do some SQL thing with a % percent sign %" in buf.getvalue() + def test_warning_on_passing_engine(self): env = self._fixture()