From: Ben Darnell Date: Sun, 3 Mar 2013 23:17:29 +0000 (-0500) Subject: return_future and friends now pass the result, not the Future, to the callback. X-Git-Tag: v3.0.0~67 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e97f9afef36f989eb0d5ffd7e7007526f0d1dedc;p=thirdparty%2Ftornado.git return_future and friends now pass the result, not the Future, to the callback. Functions and decorators that take an optional callback and return a future (return_future, gen.concurrent, and run_executor) no longer pass the Future object to the callback. This results in somewhat less flexible error handling, but is more consistent with prevailing practice without Futures. --- diff --git a/tornado/concurrent.py b/tornado/concurrent.py index e24a768ac..c92bda0b2 100644 --- a/tornado/concurrent.py +++ b/tornado/concurrent.py @@ -110,7 +110,8 @@ def run_on_executor(fn): callback = kwargs.pop("callback", None) future = self.executor.submit(fn, self, *args, **kwargs) if callback: - self.io_loop.add_future(future, callback) + self.io_loop.add_future(future, + lambda future: callback(future.result())) return future return wrapper @@ -125,10 +126,13 @@ def return_future(f): From the caller's perspective, the callback argument is optional. If one is given, it will be invoked when the function is complete - with the `Future` as an argument. If no callback is given, the caller - should use the `Future` to wait for the function to complete - (perhaps by yielding it in a `gen.engine` function, or passing it - to `IOLoop.add_future`). + with `Future.result()` as an argument. If the function fails, + the callback will not be run and an exception will be raised into + the surrounding `StackContext`. + + If no callback is given, the caller should use the `Future` to + wait for the function to complete (perhaps by yielding it in a + `gen.engine` function, or passing it to `IOLoop.add_future`). Usage:: @return_future @@ -142,7 +146,8 @@ def return_future(f): callback() Note that ``@return_future`` and ``@gen.engine`` can be applied to the - same function, provided ``@return_future`` appears first. + same function, provided ``@return_future`` appears first. However, + consider using ``@gen.coroutine`` instead of this combination. """ replacer = ArgReplacer(f, 'callback') @functools.wraps(f) @@ -178,7 +183,9 @@ def return_future(f): # immediate exception, and again when the future resolves and # the callback triggers its exception by calling future.result()). if callback is not None: - future.add_done_callback(wrap(callback)) + def run_callback(future): + callback(future.result()) + future.add_done_callback(wrap(run_callback)) return future return wrapper diff --git a/tornado/gen.py b/tornado/gen.py index 6192fe498..97a4dd140 100644 --- a/tornado/gen.py +++ b/tornado/gen.py @@ -156,7 +156,9 @@ def coroutine(func): Functions with this decorator return a `Future`. Additionally, they may be called with a ``callback`` keyword argument, which will - be invoked with the future when it resolves. + be invoked with the future's result when it resolves. If the coroutine + fails, the callback will not be run and an exception will be raised + into the surrounding `StackContext`. From the caller's perspective, ``@gen.coroutine`` is similar to the combination of ``@return_future`` and ``@gen.engine``. @@ -167,7 +169,9 @@ def coroutine(func): future = Future() if 'callback' in kwargs: - IOLoop.current().add_future(future, kwargs.pop('callback')) + callback = kwargs.pop('callback') + IOLoop.current().add_future( + future, lambda future: callback(future.result())) def handle_exception(typ, value, tb): try: diff --git a/tornado/netutil.py b/tornado/netutil.py index 06c88ab61..e60bfedf1 100644 --- a/tornado/netutil.py +++ b/tornado/netutil.py @@ -178,7 +178,7 @@ class Resolver(Configurable): pairs, where address is a tuple suitable to pass to `socket.connect` (i.e. a (host, port) pair for IPv4; additional fields may be present for IPv6). If a callback is - passed, it will be run with the `Future` as an argument when + passed, it will be run with the result as an argument when it is complete. """ raise NotImplementedError() diff --git a/tornado/simple_httpclient.py b/tornado/simple_httpclient.py index 92021f266..760b9b484 100644 --- a/tornado/simple_httpclient.py +++ b/tornado/simple_httpclient.py @@ -152,8 +152,8 @@ class _HTTPConnection(object): self.resolver.resolve(host, port, af, callback=self._on_resolve) - def _on_resolve(self, future): - af, sockaddr = future.result()[0] + def _on_resolve(self, addrinfo): + af, sockaddr = addrinfo[0] if self.parsed.scheme == "https": ssl_options = {} diff --git a/tornado/test/concurrent_test.py b/tornado/test/concurrent_test.py index f58e5c062..8e758404c 100644 --- a/tornado/test/concurrent_test.py +++ b/tornado/test/concurrent_test.py @@ -23,6 +23,7 @@ from tornado.concurrent import Future, return_future, ReturnValueIgnoredError from tornado.escape import utf8, to_unicode from tornado import gen from tornado.iostream import IOStream +from tornado import stack_context from tornado.tcpserver import TCPServer from tornado.testing import AsyncTestCase, LogTrapTestCase, bind_unused_port, gen_test @@ -66,16 +67,16 @@ class ReturnFutureTest(AsyncTestCase): def test_callback_kw(self): future = self.sync_future(callback=self.stop) - future2 = self.wait() - self.assertIs(future, future2) + result = self.wait() + self.assertEqual(result, 42) self.assertEqual(future.result(), 42) def test_callback_positional(self): # When the callback is passed in positionally, future_wrap shouldn't # add another callback in the kwargs. future = self.sync_future(self.stop) - future2 = self.wait() - self.assertIs(future, future2) + result = self.wait() + self.assertEqual(result, 42) self.assertEqual(future.result(), 42) def test_no_callback(self): @@ -170,7 +171,8 @@ class ManualCapClient(BaseCapClient): callback=self.handle_connect) self.future = Future() if callback is not None: - self.future.add_done_callback(callback) + self.future.add_done_callback( + stack_context.wrap(lambda future: callback(future.result()))) return self.future def handle_connect(self): @@ -238,13 +240,12 @@ class ClientTestMixin(object): def test_callback(self): self.client.capitalize("hello", callback=self.stop) - future = self.wait() - self.assertEqual(future.result(), "HELLO") + result = self.wait() + self.assertEqual(result, "HELLO") def test_callback_error(self): self.client.capitalize("HELLO", callback=self.stop) - future = self.wait() - self.assertRaisesRegexp(CapError, "already capitalized", future.result) + self.assertRaisesRegexp(CapError, "already capitalized", self.wait) def test_future(self): future = self.client.capitalize("hello") diff --git a/tornado/test/gen_test.py b/tornado/test/gen_test.py index 687fd6758..f76f9324e 100644 --- a/tornado/test/gen_test.py +++ b/tornado/test/gen_test.py @@ -512,10 +512,8 @@ class GenCoroutineTest(AsyncTestCase): @gen.coroutine def f(): raise gen.Return(42) - # The callback version passes a future to the callback without - # resolving it so exception information is available to the caller. - future = yield gen.Task(f) - self.assertEqual(future.result(), 42) + result = yield gen.Task(f) + self.assertEqual(result, 42) self.finished = True @gen_test diff --git a/tornado/test/netutil_test.py b/tornado/test/netutil_test.py index 53c84c2b9..e83189fc0 100644 --- a/tornado/test/netutil_test.py +++ b/tornado/test/netutil_test.py @@ -28,9 +28,8 @@ else: class _ResolverTestMixin(object): def test_localhost(self): self.resolver.resolve('localhost', 80, callback=self.stop) - future = self.wait() - self.assertIn((socket.AF_INET, ('127.0.0.1', 80)), - future.result()) + result = self.wait() + self.assertIn((socket.AF_INET, ('127.0.0.1', 80)), result) @gen_test def test_future_interface(self):