Merge "Deprecate bound metadata"

8 files changed, 169 insertions, 15 deletions

diff --git a/doc/build/changelog/unreleased_14/4634.rst b/doc/build/changelog/unreleased_14/4634.rst
new file mode 100644
index 000000000..2beaae834
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/4634.rst
@@ -0,0 +1,25 @@
+.. change::
+    :tags: change, engine
+    :tickets: 4634
+
+    The :paramref:`_schema.MetaData.bind` argument as well as the overall
+    concept of "bound metadata" is deprecated in SQLAlchemy 1.4 and will be
+    removed in SQLAlchemy 2.0.  The parameter as well as related functions now
+    emit a :class:`_exc.RemovedIn20Warning` when :ref:`deprecation_20_mode` is
+    in use.
+
+    .. seealso::
+
+        :ref:`migration_20_implicit_execution`
+
+
+
+.. change::
+    :tags: change, ext
+    :tickets: 5142
+
+    Added new parameter :paramref:`_automap.AutomapBase.prepare.autoload_with`
+    which supersedes :paramref:`_automap.AutomapBase.prepare.reflect`
+    and :paramref:`_automap.AutomapBase.prepare.engine`.
+
+
diff --git a/lib/sqlalchemy/ext/automap.py b/lib/sqlalchemy/ext/automap.py
index 2dc7d54de..97dff7f4e 100644
--- a/lib/sqlalchemy/ext/automap.py
+++ b/lib/sqlalchemy/ext/automap.py
@@ -720,16 +720,35 @@ class AutomapBase(object):
     """
 
     @classmethod
+    @util.deprecated_params(
+        engine=(
+            "2.0",
+            "The :paramref:`_automap.AutomapBase.prepare.engine` parameter "
+            "is deprecated and will be removed in a future release.  "
+            "Please use the "
+            ":paramref:`_automap.AutomapBase.prepare.autoload_with` "
+            "parameter.",
+        ),
+        reflect=(
+            "2.0",
+            "The :paramref:`_automap.AutomapBase.prepare.reflect` "
+            "parameter is deprecated and will be removed in a future "
+            "release.  Reflection is enabled when "
+            ":paramref:`_automap.AutomapBase.prepare.autoload_with` "
+            "is passed.",
+        ),
+    )
     def prepare(
         cls,
+        autoload_with=None,
         engine=None,
         reflect=False,
         schema=None,
-        classname_for_table=classname_for_table,
-        collection_class=list,
-        name_for_scalar_relationship=name_for_scalar_relationship,
-        name_for_collection_relationship=name_for_collection_relationship,
-        generate_relationship=generate_relationship,
+        classname_for_table=None,
+        collection_class=None,
+        name_for_scalar_relationship=None,
+        name_for_collection_relationship=None,
+        generate_relationship=None,
     ):
         """Extract mapped classes and relationships from the
         :class:`_schema.MetaData` and
@@ -782,9 +801,31 @@ class AutomapBase(object):
          .. versionadded:: 1.1
 
         """
+        glbls = globals()
+        if classname_for_table is None:
+            classname_for_table = glbls["classname_for_table"]
+        if name_for_scalar_relationship is None:
+            name_for_scalar_relationship = glbls[
+                "name_for_scalar_relationship"
+            ]
+        if name_for_collection_relationship is None:
+            name_for_collection_relationship = glbls[
+                "name_for_collection_relationship"
+            ]
+        if generate_relationship is None:
+            generate_relationship = glbls["generate_relationship"]
+        if collection_class is None:
+            collection_class = list
+
+        if autoload_with:
+            reflect = True
+
+        if engine:
+            autoload_with = engine
+
         if reflect:
             cls.metadata.reflect(
-                engine,
+                autoload_with,
                 schema=schema,
                 extend_existing=True,
                 autoload_replace=False,
diff --git a/lib/sqlalchemy/sql/base.py b/lib/sqlalchemy/sql/base.py
index f912163bc..ba1107eac 100644
--- a/lib/sqlalchemy/sql/base.py
+++ b/lib/sqlalchemy/sql/base.py
@@ -1485,6 +1485,11 @@ class ColumnSet(util.ordered_column_set):
 
 
 def _bind_or_error(schemaitem, msg=None):
+
+    util.warn_deprecated_20(
+        "The ``bind`` argument for schema methods that invoke SQL "
+        "against an engine or connection will be required in SQLAlchemy 2.0."
+    )
     bind = schemaitem.bind
     if not bind:
         name = schemaitem.__class__.__name__
diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py
index f1cfaaef4..d764002a6 100644
--- a/lib/sqlalchemy/sql/schema.py
+++ b/lib/sqlalchemy/sql/schema.py
@@ -858,6 +858,9 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
         :class:`_schema.Table`, using the given :class:`.Connectable`
         for connectivity.
 
+        .. note:: the "bind" argument will be required in
+           SQLAlchemy 2.0.
+
         .. seealso::
 
             :meth:`_schema.MetaData.create_all`.
@@ -873,6 +876,9 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
         :class:`_schema.Table`, using the given :class:`.Connectable`
         for connectivity.
 
+        .. note:: the "bind" argument will be required in
+           SQLAlchemy 2.0.
+
         .. seealso::
 
             :meth:`_schema.MetaData.drop_all`.
@@ -2696,14 +2702,24 @@ class Sequence(IdentityOptions, roles.StatementRole, DefaultGenerator):
             return None
 
     def create(self, bind=None, checkfirst=True):
-        """Creates this sequence in the database."""
+        """Creates this sequence in the database.
+
+        .. note:: the "bind" argument will be required in
+           SQLAlchemy 2.0.
+
+        """
 
         if bind is None:
             bind = _bind_or_error(self)
         bind._run_ddl_visitor(ddl.SchemaGenerator, self, checkfirst=checkfirst)
 
     def drop(self, bind=None, checkfirst=True):
-        """Drops this sequence from the database."""
+        """Drops this sequence from the database.
+
+        .. note:: the "bind" argument will be required in
+           SQLAlchemy 2.0.
+
+        """
 
         if bind is None:
             bind = _bind_or_error(self)
@@ -3928,6 +3944,13 @@ class MetaData(SchemaItem):
 
     __visit_name__ = "metadata"
 
+    @util.deprecated_params(
+        bind=(
+            "2.0",
+            "The :paramref:`_schema.MetaData.bind` argument is deprecated and "
+            "will be removed in SQLAlchemy 2.0.",
+        ),
+    )
     def __init__(
         self,
         bind=None,
@@ -4455,6 +4478,9 @@ class MetaData(SchemaItem):
           database; if None, uses the existing bind on this ``MetaData``, if
           any.
 
+          .. note:: the "bind" argument will be required in
+             SQLAlchemy 2.0.
+
         :param tables:
           Optional list of ``Table`` objects, which is a subset of the total
           tables in the ``MetaData`` (others are ignored).
@@ -4481,6 +4507,9 @@ class MetaData(SchemaItem):
           database; if None, uses the existing bind on this ``MetaData``, if
           any.
 
+          .. note:: the "bind" argument will be required in
+             SQLAlchemy 2.0.
+
         :param tables:
           Optional list of ``Table`` objects, which is a subset of the
           total tables in the ``MetaData`` (others are ignored).
diff --git a/lib/sqlalchemy/testing/warnings.py b/lib/sqlalchemy/testing/warnings.py
index 5704cf2a6..bfa09d00a 100644
--- a/lib/sqlalchemy/testing/warnings.py
+++ b/lib/sqlalchemy/testing/warnings.py
@@ -60,6 +60,12 @@ def setup_filters():
         r".*DefaultGenerator.execute\(\)",
         r"The autoload parameter is deprecated and will be removed ",
         #
+        #
+        # bound metadaa
+        #
+        r"The MetaData.bind argument is deprecated",
+        r"The ``bind`` argument for schema methods that invoke SQL ",
+        #
         # result sets
         #
         r"The Row.keys\(\) function/method",
diff --git a/test/ext/test_automap.py b/test/ext/test_automap.py
index b022f0357..da0e7c133 100644
--- a/test/ext/test_automap.py
+++ b/test/ext/test_automap.py
@@ -113,7 +113,7 @@ class AutomapTest(fixtures.MappedTest):
         Base = automap_base(metadata=self.metadata)
         engine_mock = Mock()
         with patch.object(Base.metadata, "reflect") as reflect_mock:
-            Base.prepare(engine_mock, reflect=True, schema="some_schema")
+            Base.prepare(autoload_with=engine_mock, schema="some_schema")
             reflect_mock.assert_called_once_with(
                 engine_mock,
                 schema="some_schema",
@@ -131,7 +131,7 @@ class AutomapTest(fixtures.MappedTest):
         Base = automap_base(metadata=self.metadata)
         engine_mock = Mock()
         with patch.object(Base.metadata, "reflect") as reflect_mock:
-            Base.prepare(engine_mock, reflect=True)
+            Base.prepare(autoload_with=engine_mock)
             reflect_mock.assert_called_once_with(
                 engine_mock,
                 schema=None,
@@ -352,7 +352,7 @@ class AutomapInhTest(fixtures.MappedTest):
         class SubUser2(Single):
             __mapper_args__ = {"polymorphic_identity": "u2"}
 
-        Base.prepare(engine=testing.db, reflect=True)
+        Base.prepare(autoload_with=testing.db)
 
         assert SubUser2.__mapper__.inherits is Single.__mapper__
 
@@ -373,7 +373,7 @@ class AutomapInhTest(fixtures.MappedTest):
             __tablename__ = "joined_inh"
             __mapper_args__ = {"polymorphic_identity": "u1"}
 
-        Base.prepare(engine=testing.db, reflect=True)
+        Base.prepare(autoload_with=testing.db)
 
         assert SubJoined.__mapper__.inherits is Joined.__mapper__
 
@@ -387,8 +387,7 @@ class AutomapInhTest(fixtures.MappedTest):
             return None
 
         Base.prepare(
-            engine=testing.db,
-            reflect=True,
+            autoload_with=testing.db,
             generate_relationship=_gen_relationship,
         )
 
@@ -416,7 +415,7 @@ class ConcurrentAutomapTest(fixtures.TestBase):
     def _automap(self, e):
         Base = automap_base()
 
-        Base.prepare(e, reflect=True)
+        Base.prepare(autoload_with=e)
 
         time.sleep(0.01)
         configure_mappers()
diff --git a/test/ext/test_deprecations.py b/test/ext/test_deprecations.py
index 510f5f415..b209de36d 100644
--- a/test/ext/test_deprecations.py
+++ b/test/ext/test_deprecations.py
@@ -1,4 +1,5 @@
 from sqlalchemy import testing
+from sqlalchemy.ext.automap import automap_base
 from sqlalchemy.ext.horizontal_shard import ShardedSession
 from sqlalchemy.orm import mapper
 from sqlalchemy.testing import eq_
@@ -6,6 +7,31 @@ from sqlalchemy.testing import fixtures
 from sqlalchemy.testing import mock
 from . import test_mutable
 from .test_mutable import Foo
+from ..orm._fixtures import FixtureTest
+
+
+class AutomapTest(fixtures.MappedTest):
+    @classmethod
+    def define_tables(cls, metadata):
+        FixtureTest.define_tables(metadata)
+
+    def test_reflect_true(self):
+        Base = automap_base(metadata=self.metadata)
+        engine_mock = mock.Mock()
+        with mock.patch.object(Base.metadata, "reflect") as reflect_mock:
+            with testing.expect_deprecated(
+                "The AutomapBase.prepare.reflect parameter is deprecated",
+                "The AutomapBase.prepare.engine parameter is deprecated",
+            ):
+                Base.prepare(
+                    engine=engine_mock, reflect=True, schema="some_schema"
+                )
+            reflect_mock.assert_called_once_with(
+                engine_mock,
+                schema="some_schema",
+                extend_existing=True,
+                autoload_replace=False,
+            )
 
 
 class MutableIncludeNonPrimaryTest(test_mutable.MutableWithScalarJSONTest):
diff --git a/test/sql/test_deprecations.py b/test/sql/test_deprecations.py
index f418eab6b..176c16208 100644
--- a/test/sql/test_deprecations.py
+++ b/test/sql/test_deprecations.py
@@ -14,6 +14,7 @@ from sqlalchemy import exc
 from sqlalchemy import exists
 from sqlalchemy import ForeignKey
 from sqlalchemy import func
+from sqlalchemy import inspect
 from sqlalchemy import INT
 from sqlalchemy import Integer
 from sqlalchemy import join
@@ -67,6 +68,28 @@ class ToMetaDataTest(fixtures.TestBase):
             eq_(t2.name, "t")
 
 
+class BoundMetadataTest(fixtures.TestBase):
+    def test_arg_deprecated(self):
+        with testing.expect_deprecated_20(
+            "The MetaData.bind argument is deprecated"
+        ):
+            m1 = MetaData(testing.db)
+
+        Table("t", m1, Column("q", Integer))
+
+        with testing.expect_deprecated_20(
+            "The ``bind`` argument for schema methods that invoke SQL "
+            "against an engine or connection will be required"
+        ):
+            m1.create_all()
+        try:
+            assert "t" in inspect(testing.db).get_table_names()
+        finally:
+            m1.drop_all(testing.db)
+
+        assert "t" not in inspect(testing.db).get_table_names()
+
+
 class DeprecationWarningsTest(fixtures.TestBase, AssertsCompiledSQL):
     __backend__ = True