Merge pull request #2318 from bdarnell/concurrent-deprecation

concurrent: Add deprecation warning

tornado/concurrent.py

@@ -426,6 +426,12 @@ def run_on_executor(*args, **kwargs):
     .. versionchanged:: 5.1
        Returns a `.Future` compatible with ``await`` instead of a
        `concurrent.futures.Future`.
+
+    .. deprecated:: 5.1
+
+       The ``callback`` argument is deprecated and will be removed in
+       6.0. The decorator itself is discouraged in new code but will
+       not be removed in 6.0.
     """
     def run_on_executor_decorator(fn):
         executor = kwargs.get("executor", "executor")
@@ -437,6 +443,8 @@ def run_on_executor(*args, **kwargs):
             conc_future = getattr(self, executor).submit(fn, self, *args, **kwargs)
             chain_future(conc_future, async_future)
             if callback:
+                warnings.warn("callback arguments are deprecated, use the returned Future instead",
+                              DeprecationWarning)
                 from tornado.ioloop import IOLoop
                 IOLoop.current().add_future(
                     async_future, lambda future: callback(future.result()))

tornado/escape.py

@@ -290,24 +290,24 @@ def linkify(text, shorten=False, extra_params="",
     * ``shorten``: Long urls will be shortened for display.
 
     * ``extra_params``: Extra text to include in the link tag, or a callable
-        taking the link as an argument and returning the extra text
-        e.g. ``linkify(text, extra_params='rel="nofollow" class="external"')``,
-        or::
+      taking the link as an argument and returning the extra text
+      e.g. ``linkify(text, extra_params='rel="nofollow" class="external"')``,
+      or::
 
-            def extra_params_cb(url):
-                if url.startswith("http://example.com"):
-                    return 'class="internal"'
-                else:
-                    return 'class="external" rel="nofollow"'
-            linkify(text, extra_params=extra_params_cb)
+          def extra_params_cb(url):
+              if url.startswith("http://example.com"):
+                  return 'class="internal"'
+              else:
+                  return 'class="external" rel="nofollow"'
+          linkify(text, extra_params=extra_params_cb)
 
     * ``require_protocol``: Only linkify urls which include a protocol. If
-        this is False, urls such as www.facebook.com will also be linkified.
+      this is False, urls such as www.facebook.com will also be linkified.
 
     * ``permitted_protocols``: List (or set) of protocols which should be
-        linkified, e.g. ``linkify(text, permitted_protocols=["http", "ftp",
-        "mailto"])``. It is very unsafe to include protocols such as
-        ``javascript``.
+      linkified, e.g. ``linkify(text, permitted_protocols=["http", "ftp",
+      "mailto"])``. It is very unsafe to include protocols such as
+      ``javascript``.
     """
     if extra_params and not callable(extra_params):
         extra_params = " " + extra_params.strip()

tornado/netutil.py

@@ -328,6 +328,10 @@ class Resolver(Configurable):
 
         .. versionchanged:: 4.4
            Standardized all implementations to raise `IOError`.
+
+        .. deprecated:: 5.1
+           The ``callback`` argument is deprecated and will be removed in 6.0.
+           Use the returned awaitable object instead.
         """
         raise NotImplementedError()
 

tornado/test/netutil_test.py

@@ -13,7 +13,7 @@ from tornado.netutil import (
 )
 from tornado.stack_context import ExceptionStackContext
 from tornado.testing import AsyncTestCase, gen_test, bind_unused_port
-from tornado.test.util import unittest, skipIfNoNetwork
+from tornado.test.util import unittest, skipIfNoNetwork, ignore_deprecation
 
 try:
     from concurrent import futures
@@ -38,7 +38,8 @@ else:
 
 class _ResolverTestMixin(object):
     def test_localhost(self):
-        self.resolver.resolve('localhost', 80, callback=self.stop)
+        with ignore_deprecation():
+            self.resolver.resolve('localhost', 80, callback=self.stop)
         result = self.wait()
         self.assertIn((socket.AF_INET, ('127.0.0.1', 80)), result)
 
@@ -59,7 +60,8 @@ class _ResolverErrorTestMixin(object):
             return True  # Halt propagation.
 
         with ExceptionStackContext(handler):
-            self.resolver.resolve('an invalid domain', 80, callback=self.stop)
+            with ignore_deprecation():
+                self.resolver.resolve('an invalid domain', 80, callback=self.stop)
 
         result = self.wait()
         self.assertIsInstance(result, Exception)
@@ -108,13 +110,12 @@ class OverrideResolverTest(AsyncTestCase, _ResolverTestMixin):
         }
         self.resolver = OverrideResolver(BlockingResolver(), mapping)
 
+    @gen_test
     def test_resolve_multiaddr(self):
-        self.resolver.resolve('google.com', 80, socket.AF_INET, callback=self.stop)
-        result = self.wait()
+        result = yield self.resolver.resolve('google.com', 80, socket.AF_INET)
         self.assertIn((socket.AF_INET, ('1.2.3.4', 80)), result)
 
-        self.resolver.resolve('google.com', 80, socket.AF_INET6, callback=self.stop)
-        result = self.wait()
+        result = yield self.resolver.resolve('google.com', 80, socket.AF_INET6)
         self.assertIn((socket.AF_INET6, ('2a02:6b8:7c:40c:c51e:495f:e23a:3', 80, 0, 0)), result)