From: Ben Darnell <ben@bendarnell.com>
Date: Sun, 22 Apr 2018 04:09:26 +0000 (-0400)
Subject: iostream: Deprecate callback argument to connect()
X-Git-Tag: v5.1.0b1~23^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=661bf12283eed4abea6d341c7009765cc052d839;p=thirdparty%2Ftornado.git

iostream: Deprecate callback argument to connect()
---

diff --git a/tornado/iostream.py b/tornado/iostream.py
index c28dacfba..a302d0eb7 100644
--- a/tornado/iostream.py
+++ b/tornado/iostream.py
@@ -1291,9 +1291,17 @@ class IOStream(BaseIOStream):
            ``ssl_options=dict(cert_reqs=ssl.CERT_NONE)`` or a
            suitably-configured `ssl.SSLContext` to the
            `SSLIOStream` constructor to disable.
+
+        .. deprecated:: 5.1
+
+           The ``callback`` argument is deprecated and will be removed
+           in Tornado 6.0. Use the returned `.Future` instead.
+
         """
         self._connecting = True
         if callback is not None:
+            warnings.warn("callback argument is deprecated, use returned Future instead",
+                          DeprecationWarning)
             self._connect_callback = stack_context.wrap(callback)
             future = None
         else:
@@ -1577,9 +1585,13 @@ class SSLIOStream(IOStream):
 
     def connect(self, address, callback=None, server_hostname=None):
         self._server_hostname = server_hostname
-        # Pass a dummy callback to super.connect(), which is slightly
-        # more efficient than letting it return a Future we ignore.
-        super(SSLIOStream, self).connect(address, callback=lambda: None)
+        # Ignore the result of connect(). If it fails,
+        # wait_for_handshake will raise an error too. This is
+        # necessary for the old semantics of the connect callback
+        # (which takes no arguments). In 6.0 this can be refactored to
+        # be a regular coroutine.
+        fut = super(SSLIOStream, self).connect(address)
+        fut.add_done_callback(lambda f: f.exception())
         return self.wait_for_handshake(callback)
 
     def _handle_connect(self):
diff --git a/tornado/test/httpserver_test.py b/tornado/test/httpserver_test.py
index 19bfabb39..4bca757a6 100644
--- a/tornado/test/httpserver_test.py
+++ b/tornado/test/httpserver_test.py
@@ -13,7 +13,7 @@ from tornado.log import gen_log
 from tornado.netutil import ssl_options_to_context
 from tornado.simple_httpclient import SimpleAsyncHTTPClient
 from tornado.testing import AsyncHTTPTestCase, AsyncHTTPSTestCase, AsyncTestCase, ExpectLog, gen_test  # noqa: E501
-from tornado.test.util import unittest, skipOnTravis
+from tornado.test.util import unittest, skipOnTravis, ignore_deprecation
 from tornado.web import Application, RequestHandler, asynchronous, stream_request_body
 
 from contextlib import closing
@@ -209,7 +209,8 @@ class HTTPConnectionTest(AsyncHTTPTestCase):
 
     def raw_fetch(self, headers, body, newline=b"\r\n"):
         with closing(IOStream(socket.socket())) as stream:
-            stream.connect(('127.0.0.1', self.get_http_port()), self.stop)
+            with ignore_deprecation():
+                stream.connect(('127.0.0.1', self.get_http_port()), self.stop)
             self.wait()
             stream.write(
                 newline.join(headers +
@@ -389,8 +390,7 @@ class HTTPServerRawTest(AsyncHTTPTestCase):
     def setUp(self):
         super(HTTPServerRawTest, self).setUp()
         self.stream = IOStream(socket.socket())
-        self.stream.connect(('127.0.0.1', self.get_http_port()), self.stop)
-        self.wait()
+        self.io_loop.run_sync(lambda: self.stream.connect(('127.0.0.1', self.get_http_port())))
 
     def tearDown(self):
         self.stream.close()
@@ -618,8 +618,7 @@ class UnixSocketTest(AsyncTestCase):
         self.server = HTTPServer(app)
         self.server.add_socket(sock)
         self.stream = IOStream(socket.socket(socket.AF_UNIX))
-        self.stream.connect(self.sockfile, self.stop)
-        self.wait()
+        self.io_loop.run_sync(lambda: self.stream.connect(self.sockfile))
 
     def tearDown(self):
         self.stream.close()
diff --git a/tornado/test/iostream_test.py b/tornado/test/iostream_test.py
index 8f3ef1077..f12fbc190 100644
--- a/tornado/test/iostream_test.py
+++ b/tornado/test/iostream_test.py
@@ -100,8 +100,9 @@ class TestIOStreamWebMixin(object):
         def connected_callback():
             connected[0] = True
             cond.notify()
-        stream.connect(("127.0.0.1", self.get_http_port()),
-                       callback=connected_callback)
+        with ignore_deprecation():
+            stream.connect(("127.0.0.1", self.get_http_port()),
+                           callback=connected_callback)
         # unlike the previous tests, try to write before the connection
         # is complete.
         written = [False]
@@ -885,7 +886,8 @@ class TestIOStreamMixin(TestReadWriteMixin):
         stream.set_close_callback(self.stop)
         # log messages vary by platform and ioloop implementation
         with ExpectLog(gen_log, ".*", required=False):
-            stream.connect(("127.0.0.1", port), connect_callback)
+            with ignore_deprecation():
+                stream.connect(("127.0.0.1", port), connect_callback)
             self.wait()
         self.assertFalse(self.connect_called)
         self.assertTrue(isinstance(stream.error, socket.error), stream.error)
@@ -909,7 +911,8 @@ class TestIOStreamMixin(TestReadWriteMixin):
         with mock.patch('socket.socket.connect',
                         side_effect=socket.gaierror(errno.EIO, 'boom')):
             with ExpectLog(gen_log, "Connect error"):
-                stream.connect(('localhost', 80), callback=self.stop)
+                with ignore_deprecation():
+                    stream.connect(('localhost', 80), callback=self.stop)
                 self.wait()
                 self.assertIsInstance(stream.error, socket.gaierror)