From: Mike Bayer Date: Tue, 21 Oct 2014 17:14:06 +0000 (-0400) Subject: - the original rationale for defaulting the user-defined namespace X-Git-Tag: rel_0_7_0~58 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=44d5eea91b62cca15b21052241601f4243ff48a3;p=thirdparty%2Fsqlalchemy%2Falembic.git - the original rationale for defaulting the user-defined namespace to "sa." was to force users to deal with making sure their custom types came from a fixed module somewhere. However, it's not worth defending this rationale. The default value of the :paramref:`.EnvironmentContext.configure.user_module_prefix` parameter is **no longer the same as the SQLAlchemy prefix**. When omitted, user-defined types will now use the ``__module__`` attribute of the type class itself when rendering in an autogenerated module. fixes #229 --- diff --git a/alembic/autogenerate/render.py b/alembic/autogenerate/render.py index 7d9817a4..67fb2ca7 100644 --- a/alembic/autogenerate/render.py +++ b/alembic/autogenerate/render.py @@ -298,10 +298,10 @@ def _modify_col(tname, cname, return text -def _user_autogenerate_prefix(autogen_context): +def _user_autogenerate_prefix(autogen_context, target): prefix = autogen_context['opts']['user_module_prefix'] if prefix is None: - return _sqlalchemy_autogenerate_prefix(autogen_context) + return "%s." % target.__module__ else: return prefix @@ -386,7 +386,7 @@ def _repr_type(type_, autogen_context): prefix = _sqlalchemy_autogenerate_prefix(autogen_context) return "%s%r" % (prefix, type_) else: - prefix = _user_autogenerate_prefix(autogen_context) + prefix = _user_autogenerate_prefix(autogen_context, type_) return "%s%r" % (prefix, type_) diff --git a/alembic/environment.py b/alembic/environment.py index 7df13bfb..1dfb0006 100644 --- a/alembic/environment.py +++ b/alembic/environment.py @@ -604,10 +604,18 @@ class EnvironmentContext(object): :param user_module_prefix: When autogenerate refers to a SQLAlchemy type (e.g. :class:`.TypeEngine`) where the module name is not under the ``sqlalchemy`` namespace, this prefix will be used - within autogenerate, if non-``None``; if left at its default of - ``None``, the - :paramref:`.EnvironmentContext.configure.sqlalchemy_module_prefix` - is used instead. + within autogenerate. If left at its default of + ``None``, the ``__module__`` attribute of the type is used to + render the import module. It's a good practice to set this + and to have all custom types be available from a fixed module space, + in order to future-proof migration files against reorganizations + in modules. + + .. versionchanged:: 0.7.0 + :paramref:`.EnvironmentContext.configure.user_module_prefix` + no longer defaults to the value of + :paramref:`.EnvironmentContext.configure.sqlalchemy_module_prefix` + when left at ``None``; the ``__module__`` attribute is now used. .. versionadded:: 0.6.3 added :paramref:`.EnvironmentContext.configure.user_module_prefix` diff --git a/docs/build/changelog.rst b/docs/build/changelog.rst index dc87462e..8702cacd 100644 --- a/docs/build/changelog.rst +++ b/docs/build/changelog.rst @@ -5,6 +5,17 @@ Changelog .. changelog:: :version: 0.7.0 + .. change:: + :tags: changed, autogenerate + :tickets: 229 + + The default value of the + :paramref:`.EnvironmentContext.configure.user_module_prefix` + parameter is **no longer the same as the SQLAlchemy prefix**. + When omitted, user-defined types will now use the ``__module__`` + attribute of the type class itself when rendering in an + autogenerated module. + .. change:: :tags: bug, templates :tickets: 234 diff --git a/docs/build/conf.py b/docs/build/conf.py index 80a23bc6..9e0f077c 100644 --- a/docs/build/conf.py +++ b/docs/build/conf.py @@ -34,7 +34,7 @@ extensions = ['sphinx.ext.autodoc', 'sphinx.ext.intersphinx', 'changelog', 'sphinx_paramlinks'] # tags to sort on inside of sections -changelog_sections = ["feature", "bug", "moved", "changed", "removed"] +changelog_sections = ["changed", "feature", "bug", "moved", "removed"] changelog_render_ticket = "https://bitbucket.org/zzzeek/alembic/issue/%s/" changelog_render_pullreq = "https://bitbucket.org/zzzeek/alembic/pull-request/%s" diff --git a/docs/build/tutorial.rst b/docs/build/tutorial.rst index 82e7284b..012bcc5d 100644 --- a/docs/build/tutorial.rst +++ b/docs/build/tutorial.rst @@ -716,17 +716,31 @@ When using :paramref:`.EnvironmentContext.configure.render_item`, note that we deliver not just the reproduction of the type, but we can also deliver the "module prefix", which is a module namespace from which our type can be found within our migration script. When Alembic renders SQLAlchemy types, it will -typically use the value of :paramref:`.EnvironmentContext.configure.sqlalchemy_module_prefix`, +typically use the value of +:paramref:`.EnvironmentContext.configure.sqlalchemy_module_prefix`, which defaults to ``"sa."``, to achieve this:: Column("my_column", sa.Integer()) When we use a custom type that is not within the ``sqlalchemy.`` module namespace, -by default Alembic will still use the ``"sa."`` prefix:: +by default Alembic will use the **value of __module__ for the custom type**:: - Column("my_column", sa.MyCustomType()) + Column("my_column", myapp.models.utils.types.MyCustomType()) -We can provide an alternate prefix here using the :paramref:`.EnvironmentContext.configure.user_module_prefix` +Above, it seems our custom type is in a very specific location, based on +the length of what ``__module__`` reports. It's a good practice to +not have this long name render into our migration scripts, as it means +this long and arbitrary name will be hardcoded into all our migration +scripts; instead, we should create a module that is +explicitly for custom types that our migration files will use. Suppose +we call it ``myapp.migration_types``:: + + # myapp/migration_types.py + + from myapp.models.utils.types import MyCustomType + +We can provide the name of this module to our autogenerate context using +:paramref:`.EnvironmentContext.configure.user_module_prefix` option:: @@ -736,7 +750,7 @@ option:: context.configure( connection=connection, target_metadata=target_metadata, - user_module_prefix="mymodel.types", + user_module_prefix="myapp.migration_types.", # ... ) @@ -744,7 +758,17 @@ option:: Where we'd get a migration like:: - Column("my_column", mymodel.types.MyCustomType()) + Column("my_column", myapp.migration_types.MyCustomType()) + +Now, when we inevitably refactor our application to move ``MyCustomType`` +somewhere else, we only need modify the ``myapp.migration_types`` module, +instead of searching and replacing all instances within our migration scripts. + +.. versionchanged:: 0.7.0 + :paramref:`.EnvironmentContext.configure.user_module_prefix` + no longer defaults to the value of + :paramref:`.EnvironmentContext.configure.sqlalchemy_module_prefix` + when left at ``None``; the ``__module__`` attribute is now used. .. versionadded:: 0.6.3 Added :paramref:`.EnvironmentContext.configure.user_module_prefix`. diff --git a/tests/test_autogen_render.py b/tests/test_autogen_render.py index da17bd2a..889af2d5 100644 --- a/tests/test_autogen_render.py +++ b/tests/test_autogen_render.py @@ -772,7 +772,7 @@ render:primary_key\n)""" eq_ignore_whitespace( autogenerate.render._repr_type(type_, autogen_context), - "sa.MyType()" + "tests.test_autogen_render.MyType()" ) def test_repr_user_type_user_prefix_present(self):