Skip to content

Commit

Permalink
Merge pull request #3223 from takluyver/undeprecate-event-loop-setting
Browse files Browse the repository at this point in the history
Revert some deprecations, following asyncio changes
  • Loading branch information
bdarnell committed Feb 16, 2023
2 parents 1f3f28a + 65cd10a commit d0132b5
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 147 deletions.
50 changes: 24 additions & 26 deletions tornado/ioloop.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ async def main():
and instead initialize the `asyncio` event loop and use `IOLoop.current()`.
In some cases, such as in test frameworks when initializing an `IOLoop`
to be run in a secondary thread, it may be appropriate to construct
an `IOLoop` with ``IOLoop(make_current=False)``. Constructing an `IOLoop`
without the ``make_current=False`` argument is deprecated since Tornado 6.2.
an `IOLoop` with ``IOLoop(make_current=False)``.
In general, an `IOLoop` cannot survive a fork or be shared across processes
in any way. When multiple processes are being used, each process should
Expand All @@ -145,12 +144,10 @@ async def main():
cannot be used on Python 3 except to redundantly specify the `asyncio`
event loop.
.. deprecated:: 6.2
It is deprecated to create an event loop that is "current" but not
running. This means it is deprecated to pass
``make_current=True`` to the ``IOLoop`` constructor, or to create
an ``IOLoop`` while no asyncio event loop is running unless
``make_current=False`` is used.
.. versionchanged:: 6.3
``make_current=True`` is now the default when creating an IOLoop -
previously the default was to make the event loop current if there wasn't
already a current one.
"""

# These constants were originally based on constants from the epoll module.
Expand Down Expand Up @@ -263,17 +260,20 @@ def current(instance: bool = True) -> Optional["IOLoop"]: # noqa: F811
"""
try:
loop = asyncio.get_event_loop()
except (RuntimeError, AssertionError):
except RuntimeError:
if not instance:
return None
raise
# Create a new asyncio event loop for this thread.
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)

try:
return IOLoop._ioloop_for_asyncio[loop]
except KeyError:
if instance:
from tornado.platform.asyncio import AsyncIOMainLoop

current = AsyncIOMainLoop(make_current=True) # type: Optional[IOLoop]
current = AsyncIOMainLoop() # type: Optional[IOLoop]
else:
current = None
return current
Expand All @@ -295,12 +295,17 @@ def make_current(self) -> None:
This method also sets the current `asyncio` event loop.
.. deprecated:: 6.2
The concept of an event loop that is "current" without
currently running is deprecated in asyncio since Python
3.10. All related functionality in Tornado is also
deprecated. Instead, start the event loop with `asyncio.run`
before interacting with it.
Setting and clearing the current event loop through Tornado is
deprecated. Use ``asyncio.set_event_loop`` instead if you need this.
"""
warnings.warn(
"make_current is deprecated; start the event loop first",
DeprecationWarning,
stacklevel=2,
)
self._make_current()

def _make_current(self) -> None:
# The asyncio event loops override this method.
raise NotImplementedError()

Expand Down Expand Up @@ -344,16 +349,9 @@ def configurable_default(cls) -> Type[Configurable]:

return AsyncIOLoop

def initialize(self, make_current: Optional[bool] = None) -> None:
if make_current is None:
if IOLoop.current(instance=False) is None:
self.make_current()
elif make_current:
current = IOLoop.current(instance=False)
# AsyncIO loops can already be current by this point.
if current is not None and current is not self:
raise RuntimeError("current IOLoop already exists")
self.make_current()
def initialize(self, make_current: bool = True) -> None:
if make_current:
self._make_current()

def close(self, all_fds: bool = False) -> None:
"""Closes the `IOLoop`, freeing any resources used.
Expand Down
9 changes: 2 additions & 7 deletions tornado/platform/asyncio.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ class AsyncIOMainLoop(BaseAsyncIOLoop):
def initialize(self, **kwargs: Any) -> None: # type: ignore
super().initialize(asyncio.get_event_loop(), **kwargs)

def make_current(self) -> None:
def _make_current(self) -> None:
# AsyncIOMainLoop already refers to the current asyncio loop so
# nothing to do here.
pass
Expand Down Expand Up @@ -327,12 +327,7 @@ def close(self, all_fds: bool = False) -> None:
self._clear_current()
super().close(all_fds=all_fds)

def make_current(self) -> None:
warnings.warn(
"make_current is deprecated; start the event loop first",
DeprecationWarning,
stacklevel=2,
)
def _make_current(self) -> None:
if not self.is_current:
try:
self.old_asyncio = asyncio.get_event_loop()
Expand Down
8 changes: 2 additions & 6 deletions tornado/tcpserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,7 @@ def bind(
.. deprecated:: 6.2
Use either ``listen()`` or ``add_sockets()`` instead of ``bind()``
and ``start()``. The ``bind()/start()`` pattern depends on
interfaces that have been deprecated in Python 3.10 and will be
removed in future versions of Python.
and ``start()``.
"""
sockets = bind_sockets(
port,
Expand Down Expand Up @@ -295,9 +293,7 @@ def start(
.. deprecated:: 6.2
Use either ``listen()`` or ``add_sockets()`` instead of ``bind()``
and ``start()``. The ``bind()/start()`` pattern depends on
interfaces that have been deprecated in Python 3.10 and will be
removed in future versions of Python.
and ``start()``.
"""
assert not self._started
self._started = True
Expand Down
30 changes: 20 additions & 10 deletions tornado/test/asyncio_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ def get_new_ioloop(self):
io_loop = AsyncIOLoop(make_current=False)
return io_loop

@property
def asyncio_loop(self):
return self.io_loop.asyncio_loop # type: ignore

def test_asyncio_callback(self):
# Basic test that the asyncio loop is set up correctly.
async def add_callback():
Expand Down Expand Up @@ -104,9 +108,6 @@ async def native_coroutine_with_adapter2():
self.asyncio_loop.run_until_complete(native_coroutine_with_adapter2()),
42,
)
# I'm not entirely sure why this manual cleanup is necessary but without
# it we have at-a-distance failures in ioloop_test.TestIOLoopCurrent.
asyncio.set_event_loop(None)


class LeakTest(unittest.TestCase):
Expand Down Expand Up @@ -179,25 +180,34 @@ def get_and_close_event_loop():
future = self.executor.submit(get_and_close_event_loop)
return future.result()

def run_policy_test(self, accessor, expected_type):
def test_asyncio_accessor(self):
with warnings.catch_warnings():
warnings.simplefilter("ignore", DeprecationWarning)
# With the default policy, non-main threads don't get an event
# loop.
self.assertRaises(
(RuntimeError, AssertionError), self.executor.submit(accessor).result
RuntimeError, self.executor.submit(asyncio.get_event_loop).result
)
# Set the policy and we can get a loop.
asyncio.set_event_loop_policy(AnyThreadEventLoopPolicy())
self.assertIsInstance(
self.executor.submit(accessor).result(), expected_type
self.executor.submit(asyncio.get_event_loop).result(),
asyncio.AbstractEventLoop,
)
# Clean up to silence leak warnings. Always use asyncio since
# IOLoop doesn't (currently) close the underlying loop.
self.executor.submit(lambda: asyncio.get_event_loop().close()).result() # type: ignore

def test_asyncio_accessor(self):
self.run_policy_test(asyncio.get_event_loop, asyncio.AbstractEventLoop)

def test_tornado_accessor(self):
self.run_policy_test(IOLoop.current, IOLoop)
# Tornado's IOLoop.current() API can create a loop for any thread,
# regardless of this event loop policy.
with warnings.catch_warnings():
warnings.simplefilter("ignore", DeprecationWarning)
self.assertIsInstance(self.executor.submit(IOLoop.current).result(), IOLoop)
# Clean up to silence leak warnings. Always use asyncio since
# IOLoop doesn't (currently) close the underlying loop.
self.executor.submit(lambda: asyncio.get_event_loop().close()).result() # type: ignore

asyncio.set_event_loop_policy(AnyThreadEventLoopPolicy())
self.assertIsInstance(self.executor.submit(IOLoop.current).result(), IOLoop)
self.executor.submit(lambda: asyncio.get_event_loop().close()).result() # type: ignore
14 changes: 0 additions & 14 deletions tornado/test/ioloop_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -450,15 +450,6 @@ def tearDown(self):
if self.io_loop is not None:
self.io_loop.close()

def test_default_current(self):
self.io_loop = IOLoop()
# The first IOLoop with default arguments is made current.
self.assertIs(self.io_loop, IOLoop.current())
# A second IOLoop can be created but is not made current.
io_loop2 = IOLoop()
self.assertIs(self.io_loop, IOLoop.current())
io_loop2.close()

def test_non_current(self):
self.io_loop = IOLoop(make_current=False)
# The new IOLoop is not initially made current.
Expand All @@ -481,11 +472,6 @@ def f():
def test_force_current(self):
self.io_loop = IOLoop(make_current=True)
self.assertIs(self.io_loop, IOLoop.current())
with self.assertRaises(RuntimeError):
# A second make_current=True construction cannot succeed.
IOLoop(make_current=True)
# current() was not affected by the failed construction.
self.assertIs(self.io_loop, IOLoop.current())


class TestIOLoopCurrentAsync(AsyncTestCase):
Expand Down
29 changes: 0 additions & 29 deletions tornado/test/testing_test.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from tornado import gen, ioloop
from tornado.httpserver import HTTPServer
from tornado.locks import Event
from tornado.test.util import ignore_deprecation
from tornado.testing import AsyncHTTPTestCase, AsyncTestCase, bind_unused_port, gen_test
from tornado.web import Application
import asyncio
Expand Down Expand Up @@ -342,33 +341,5 @@ async def test(self):
self.finished = True


class GetNewIOLoopTest(AsyncTestCase):
def get_new_ioloop(self):
# Use the current loop instead of creating a new one here.
return ioloop.IOLoop.current()

def setUp(self):
# This simulates the effect of an asyncio test harness like
# pytest-asyncio.
with ignore_deprecation():
try:
self.orig_loop = asyncio.get_event_loop()
except RuntimeError:
self.orig_loop = None # type: ignore[assignment]
self.new_loop = asyncio.new_event_loop()
asyncio.set_event_loop(self.new_loop)
super().setUp()

def tearDown(self):
super().tearDown()
# AsyncTestCase must not affect the existing asyncio loop.
self.assertFalse(asyncio.get_event_loop().is_closed())
asyncio.set_event_loop(self.orig_loop)
self.new_loop.close()

def test_loop(self):
self.assertIs(self.io_loop.asyncio_loop, self.new_loop) # type: ignore


if __name__ == "__main__":
unittest.main()
58 changes: 3 additions & 55 deletions tornado/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,17 +162,6 @@ def test_http_fetch(self):
response = self.wait()
# Test contents of response
self.assertIn("FriendFeed", response.body)
.. deprecated:: 6.2
AsyncTestCase and AsyncHTTPTestCase are deprecated due to changes
in future versions of Python (after 3.10). The interfaces used
in this class are incompatible with the deprecation and intended
removal of certain methods related to the idea of a "current"
event loop while no event loop is actually running. Use
`unittest.IsolatedAsyncioTestCase` instead. Note that this class
does not emit DeprecationWarnings until better migration guidance
can be provided.
"""

def __init__(self, methodName: str = "runTest") -> None:
Expand Down Expand Up @@ -201,41 +190,8 @@ def setUp(self) -> None:
module=r"tornado\..*",
)
super().setUp()
# NOTE: this code attempts to navigate deprecation warnings introduced
# in Python 3.10. The idea of an implicit current event loop is
# deprecated in that version, with the intention that tests like this
# explicitly create a new event loop and run on it. However, other
# packages such as pytest-asyncio (as of version 0.16.0) still rely on
# the implicit current event loop and we want to be compatible with them
# (even when run on 3.10, but not, of course, on the future version of
# python that removes the get/set_event_loop methods completely).
#
# Deprecation warnings were introduced inconsistently:
# asyncio.get_event_loop warns, but
# asyncio.get_event_loop_policy().get_event_loop does not. Similarly,
# none of the set_event_loop methods warn, although comments on
# https://bugs.python.org/issue39529 indicate that they are also
# intended for future removal.
#
# Therefore, we first attempt to access the event loop with the
# (non-warning) policy method, and if it fails, fall back to creating a
# new event loop. We do not have effective test coverage of the
# new event loop case; this will have to be watched when/if
# get_event_loop is actually removed.
self.should_close_asyncio_loop = False
try:
self.asyncio_loop = asyncio.get_event_loop_policy().get_event_loop()
except Exception:
self.asyncio_loop = asyncio.new_event_loop()
self.should_close_asyncio_loop = True

async def get_loop() -> IOLoop:
return self.get_new_ioloop()

self.io_loop = self.asyncio_loop.run_until_complete(get_loop())
with warnings.catch_warnings():
warnings.simplefilter("ignore", DeprecationWarning)
self.io_loop.make_current()
self.io_loop = self.get_new_ioloop()
asyncio.set_event_loop(self.io_loop.asyncio_loop) # type: ignore[attr-defined]

def tearDown(self) -> None:
# Native coroutines tend to produce warnings if they're not
Expand Down Expand Up @@ -270,17 +226,13 @@ def tearDown(self) -> None:

# Clean up Subprocess, so it can be used again with a new ioloop.
Subprocess.uninitialize()
with warnings.catch_warnings():
warnings.simplefilter("ignore", DeprecationWarning)
self.io_loop.clear_current()
asyncio.set_event_loop(None)
if not isinstance(self.io_loop, _NON_OWNED_IOLOOPS):
# Try to clean up any file descriptors left open in the ioloop.
# This avoids leaks, especially when tests are run repeatedly
# in the same process with autoreload (because curl does not
# set FD_CLOEXEC on its file descriptors)
self.io_loop.close(all_fds=True)
if self.should_close_asyncio_loop:
self.asyncio_loop.close()
super().tearDown()
# In case an exception escaped or the StackContext caught an exception
# when there wasn't a wait() to re-raise it, do so here.
Expand Down Expand Up @@ -435,10 +387,6 @@ def test_homepage(self):
like ``http_client.fetch()``, into a synchronous operation. If you need
to do other asynchronous operations in tests, you'll probably need to use
``stop()`` and ``wait()`` yourself.
.. deprecated:: 6.2
`AsyncTestCase` and `AsyncHTTPTestCase` are deprecated due to changes
in Python 3.10; see comments on `AsyncTestCase` for more details.
"""

def setUp(self) -> None:
Expand Down

0 comments on commit d0132b5

Please sign in to comment.