From 172557e49da244a67bc7bfe1438bde5ddbc0f30a Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Sat, 21 Apr 2018 21:58:32 -0400 Subject: [PATCH] iostream: Deprecate streaming_callback arguments Currently, mixing futures and streaming_callback does not guarantee ordering (so the returned future could become ready before the last streaming_callback fires). Specifically, this happens with test_streaming_read_until_close_after_close if that test is modified to not use the callback argument. This would cause problems when we remove the callback argument if we left streaming_callback in place. These problems are solvable, but probably not worth it since partial=True is an alternative (and used internally instead of streaming_callback). --- tornado/iostream.py | 22 ++++++++++++++++------ tornado/test/iostream_test.py | 9 ++++++--- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/tornado/iostream.py b/tornado/iostream.py index f7f7e3c8..0220e179 100644 --- a/tornado/iostream.py +++ b/tornado/iostream.py @@ -423,15 +423,20 @@ class BaseIOStream(object): .. deprecated:: 5.1 - The ``callback`` argument is deprecated and will be removed - in Tornado 6.0. Use the returned `.Future` instead. + The ``callback`` and ``streaming_callback`` arguments are + deprecated and will be removed in Tornado 6.0. Use the + returned `.Future` (and ``partial=True`` for + ``streaming_callback``) instead. """ future = self._set_read_callback(callback) assert isinstance(num_bytes, numbers.Integral) self._read_bytes = num_bytes self._read_partial = partial - self._streaming_callback = stack_context.wrap(streaming_callback) + if streaming_callback is not None: + warnings.warn("streaming_callback is deprecated, use partial instead", + DeprecationWarning) + self._streaming_callback = stack_context.wrap(streaming_callback) try: self._try_inline_read() except: @@ -511,12 +516,17 @@ class BaseIOStream(object): .. deprecated:: 5.1 - The ``callback`` argument is deprecated and will be removed - in Tornado 6.0. Use the returned `.Future` instead. + The ``callback`` and ``streaming_callback`` arguments are + deprecated and will be removed in Tornado 6.0. Use the + returned `.Future` (and `read_bytes` with ``partial=True`` + for ``streaming_callback``) instead. """ future = self._set_read_callback(callback) - self._streaming_callback = stack_context.wrap(streaming_callback) + if streaming_callback is not None: + warnings.warn("streaming_callback is deprecated, use read_bytes(partial=True) instead", + DeprecationWarning) + self._streaming_callback = stack_context.wrap(streaming_callback) if self.closed(): if self._streaming_callback is not None: self._run_read_callback(self._read_buffer_size, True) diff --git a/tornado/test/iostream_test.py b/tornado/test/iostream_test.py index 200a3a0c..c6049dd4 100644 --- a/tornado/test/iostream_test.py +++ b/tornado/test/iostream_test.py @@ -195,7 +195,8 @@ class TestReadWriteMixin(object): chunks.append(data) cond.notify() - fut = rs.read_bytes(6, streaming_callback=streaming_callback) + with ignore_deprecation(): + fut = rs.read_bytes(6, streaming_callback=streaming_callback) ws.write(b"1234") while not chunks: yield cond.wait() @@ -253,7 +254,8 @@ class TestReadWriteMixin(object): self.assertEqual(data, b"abcd\r\n") streaming_fut = Future() - rs.read_until_close(streaming_callback=streaming_fut.set_result) + with ignore_deprecation(): + rs.read_until_close(streaming_callback=streaming_fut.set_result) data = yield streaming_fut self.assertEqual(data, b"efgh") rs.close() @@ -298,7 +300,8 @@ class TestReadWriteMixin(object): @gen.coroutine def rs_task(): - yield rs.read_until_close(streaming_callback=chunks.append) + with ignore_deprecation(): + yield rs.read_until_close(streaming_callback=chunks.append) @gen.coroutine def ws_task(): -- GitLab