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

@@ -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)

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():