Fixed :meth:`.MetaData.reflect` to correctly use

the given :class:`.Connection`, if given, without
opening a second connection from that connection's
:class:`.Engine`. [ticket:2604]

diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst
index 9025f8c30172dd3e84b28d2fce02ba86a2ad0ef8..c34aadb8a594e6f870f25170ce3a1df0117b33f7 100644 (file)
--- a/doc/build/changelog/changelog_08.rst
+++ b/doc/build/changelog/changelog_08.rst
@@ -6,9 +6,24 @@
 .. changelog::
     :version: 0.8.0b2
 
+    .. change::
+        :tags: engine
+
+      The "reflect=True" argument to :class:`MetaData` is deprecated.
+      Please use the :meth:`.MetaData.reflect` method.
+
+    .. change::
+        :tags: engine, bug
+        :tickets: 2604
+
+      Fixed :meth:`.MetaData.reflect` to correctly use
+      the given :class:`.Connection`, if given, without
+      opening a second connection from that connection's
+      :class:`.Engine`.  Also in 0.7.10.
+
     .. change::
         :tags: mssql, bug
-        :tickets:2607
+        :tickets: 2607
 
       Fixed bug whereby using "key" with Column
       in conjunction with "schema" for the owning

diff --git a/lib/sqlalchemy/schema.py b/lib/sqlalchemy/schema.py
index 0b1fac1e784d8179e45bc521b0c0d5c9d7434e45..ac8be377c2ccaf6dc116fcb15276cd72912b13eb 100644 (file)
--- a/lib/sqlalchemy/schema.py
+++ b/lib/sqlalchemy/schema.py
@@ -2428,8 +2428,9 @@ class MetaData(SchemaItem):
         :param reflect:
           Optional, automatically load all tables from the bound database.
           Defaults to False. ``bind`` is required when this option is set.
-          For finer control over loaded tables, use the ``reflect`` method of
-          ``MetaData``.
+
+          .. deprecated:: 0.8
+                Please use the :meth:`.MetaData.reflect` method.
 
         :param schema:
            The default schema to use for the :class:`.Table`, :class:`.Sequence`, and other
@@ -2451,6 +2452,8 @@ class MetaData(SchemaItem):
         self._sequences = {}
         self.bind = bind
         if reflect:
+            util.warn("reflect=True is deprecate; please "
+                            "use the reflect() method.")
             if not bind:
                 raise exc.ArgumentError(
                     "A bind must be supplied in conjunction "
@@ -2592,13 +2595,20 @@ class MetaData(SchemaItem):
           arguments and should return a true value for any table to reflect.
 
         """
-        reflect_opts = {'autoload': True}
         if bind is None:
             bind = _bind_or_error(self)
-            conn = None
+
+        if bind.engine is not bind:
+            conn = bind
+            close = False
         else:
-            reflect_opts['autoload_with'] = bind
             conn = bind.contextual_connect()
+            close = True
+
+        reflect_opts = {
+            'autoload': True,
+            'autoload_with': bind
+        }
 
         if schema is None:
             schema = self.schema
@@ -2611,7 +2621,7 @@ class MetaData(SchemaItem):
                                                             connection=conn))
             if views:
                 available.update(
-                    bind.dialect.get_view_names(conn or bind, schema)
+                    bind.dialect.get_view_names(conn, schema)
                 )
 
             current = set(self.tables.iterkeys())
@@ -2634,8 +2644,7 @@ class MetaData(SchemaItem):
             for name in load:
                 Table(name, self, **reflect_opts)
         finally:
-            if conn is not None and \
-                conn is not bind:
+            if close:
                 conn.close()
 
     def append_ddl_listener(self, event_name, listener):

diff --git a/test/engine/test_reflection.py b/test/engine/test_reflection.py
index 086f3cc24d66416de4ec4eb9f791c1dd5ae4fba3..2ff3140a56e38435541d95bec2773fa30483a750 100644 (file)
--- a/test/engine/test_reflection.py
+++ b/test/engine/test_reflection.py
@@ -750,6 +750,33 @@ class ReflectionTest(fixtures.TestBase, ComparesTables):
         table_b2 = Table('false', meta2, autoload=True)
         table_c2 = Table('is', meta2, autoload=True)
 
+    @testing.provide_metadata
+    def _test_reflect_uses_bind(self, fn):
+        from sqlalchemy.pool import AssertionPool
+        e = engines.testing_engine(options={"poolclass": AssertionPool})
+        fn(e)
+
+    @testing.uses_deprecated
+    def test_reflect_uses_bind_constructor_conn(self):
+        self._test_reflect_uses_bind(lambda e: MetaData(e.connect(),
+                    reflect=True))
+
+    @testing.uses_deprecated
+    def test_reflect_uses_bind_constructor_engine(self):
+        self._test_reflect_uses_bind(lambda e: MetaData(e, reflect=True))
+
+    def test_reflect_uses_bind_constructor_conn_reflect(self):
+        self._test_reflect_uses_bind(lambda e: MetaData(e.connect()).reflect())
+
+    def test_reflect_uses_bind_constructor_engine_reflect(self):
+        self._test_reflect_uses_bind(lambda e: MetaData(e).reflect())
+
+    def test_reflect_uses_bind_conn_reflect(self):
+        self._test_reflect_uses_bind(lambda e: MetaData().reflect(e.connect()))
+
+    def test_reflect_uses_bind_engine_reflect(self):
+        self._test_reflect_uses_bind(lambda e: MetaData().reflect(e))
+
     @testing.provide_metadata
     def test_reflect_all(self):
         existing = testing.db.table_names()
@@ -795,16 +822,15 @@ class ReflectionTest(fixtures.TestBase, ComparesTables):
         m6.reflect(only=lambda n, m: False)
         self.assert_(not m6.tables)
 
-        m7 = MetaData(testing.db, reflect=True)
+        m7 = MetaData(testing.db)
+        m7.reflect()
         self.assert_(nameset.issubset(set(m7.tables.keys())))
 
-        try:
-            m8 = MetaData(reflect=True)
-            self.assert_(False)
-        except sa.exc.ArgumentError, e:
-            self.assert_(e.args[0]
-                         == 'A bind must be supplied in '
-                         'conjunction with reflect=True')
+        m8 = MetaData()
+        assert_raises(
+            sa.exc.UnboundExecutionError,
+            m8.reflect
+        )
 
         if existing:
             print "Other tables present in database, skipping some checks."
@@ -1081,7 +1107,7 @@ class UnicodeReflectionTest(fixtures.TestBase):
 
         # Jython 2.5 on Java 5 lacks unicodedata.normalize
 
-        if not names.issubset(reflected) and hasattr(unicodedata,'normalize'):
+        if not names.issubset(reflected) and hasattr(unicodedata, 'normalize'):
 
             # Python source files in the utf-8 coding seem to
             # normalize literals as NFC (and the above are
@@ -1094,7 +1120,8 @@ class UnicodeReflectionTest(fixtures.TestBase):
             # Yep.  But still ensure that bulk reflection and
             # create/drop work with either normalization.
 
-        r = MetaData(bind, reflect=True)
+        r = MetaData(bind)
+        r.reflect()
         r.drop_all(checkfirst=False)
         r.create_all(checkfirst=False)