Create AsyncIOLoop from existing asyncio event loop

[adamrothman]

Creating a new AsyncIOLoop always creates a new underlying asyncio event loop. I think this makes sense in most cases but it would be very helpful (especially for testing, read on) to be able to pass an existing asyncio event loop for Tornado to wrap.

We use pytest with the pytest-asyncio plugin to run our test suite for several Tornado applications. After upgrading one of these apps to Tornado 5 and improving our test hygiene to use a new event loop for each test, our test suite broke.

Our unit tests are simple because they don't interact with or depend on tornado.testing.AsyncTestCase at all. Our integration tests do (because they make requests against the application), however, and this is where I ran into trouble. When I ran the tests, I would get a ton of errors due to Tasks getting Futures attached to a different loop, like this:

RuntimeError: Task <Task pending coro=<test_get_targets() running at /Users/adam/Other/src/pusheen/tests/integration/application_api_test.py:279> cb=[IOLoop.add_future..() at /Users/adam/Other/src/pusheen/.tox/verbose/lib/python3.6/site-packages/tornado/ioloop.py:721]> got Future <Task pending coro=<AioBaseClient._make_api_call() running at /Users/adam/Other/src/pusheen/.tox/verbose/lib/python3.6/site-packages/aiobotocore/client.py:65>> attached to a different loop

pytest-asyncio creates a new event loop for each async test that it runs (which is a good thing). Previously we were overriding this behavior to always return the "main" loop via asyncio.get_event_loop() and explicitly not close the loop between tests. This worked under Tornado 4 because we were using AsyncIOMainLoop, and Tornado 4 did not close the underlying asyncio event loop in such a configuration. But this override has always felt kind of gross to me, so I resolved to remove it. After all, there's no reason it should be necessary; we are careful to pass the testing event loop to classes that require it so they don't grab the main loop by default. Plus, Tornado 5 does close the underlying loop when closing AsyncIOMainLoop.

But making this change borked our tests.

We have some setup code that is shared between the unit and integration tests. This setup is handled by pytest, so a number of async singletons (e.g. DB connectors) get configured to use the event loop created by pytest-asyncio for each test.

tornado.testing.AsyncTestCase also creates a loop for each test – an IOLoop – by calling get_new_ioloop as part of setUp. This is still a good thing – but turned out to be the cause of my sadness. During the integration tests, our application logic is run by Tornado's loop, but per the above, the async singletons use pytest-asyncio's loop. When async code running on one loop tries to interact with async code on the other, people weep in the streets.

Being able to override AsyncTestCase.get_new_ioloop goes most of the way towards solving this issue. It's not difficult to expose pytest-asyncio's event loop to this function – the problem is that I can't use this existing asyncio event loop to instantiate the Tornado IOLoop get_new_ioloop is expected to return.

For now I'm working around this limitation by overriding pytest-asyncio's default behavior to first create an AsyncIOLoop and then using the underlying .asyncio_loop. This feels wrong, though. pytest shouldn't need to know about what's happening "downstream" as long as it implements a compatible test interface like unittest.TestCase.

Mine is just one (long-winded, sorry) case but I can imagine there are other situations in which users would want to provide an existing event loop to Tornado. What do you think @bdarnell?

[bdarnell]

Shouldn't def get_new_ioloop(self): return AsyncIOMainLoop() do what you want here and pick up the loop defined by pytest-asyncio? Or does it break down because Tornado also wants to close the event loop?

I'm definitely open to suggestions here. I'm not at all familiar with testing practices in the asyncio world (and only barely familiar with pytest).

Part of the problem is that AsyncTestCase does two different things: it sets up a clean-slate environment for the tests (the new IOLoop), and it provides utilities for the tests to use at runtime (setting up the HTTPServer, etc). One way to look at this problem is that the former part of AsyncTestCase is in conflict with pytest-asyncio, and we should decouple them so that you can use the other parts of AsyncTestCase while leaving the event loop setup to pytest-asyncio.

[adamrothman]

That does work for the first test, but all subsequent tests fail. AsyncTestCase.tearDown closes its associated IOLoop – which it's always done – but now that that actually closes the underlying asyncio loop, it all grinds to a halt.

As a note – that is how we were had it set up pre-Tornado 5. But it required overriding pytest's default behavior (which is to create a new loop for each test via asyncio.new_event_loop(), then close it) to instead grab the main loop with asyncio.get_event_loop(), and leave it open between tests. Without this, Tornado and pytest end up with different loops.

I think the simplest solution would be to add something like AsyncIOLoop.from_event_loop(). I like that AsyncTestCase can stand on its own without additional configuration in the most common use case. The existing get_new_ioloop gets us most of the way to where we need to be – all that's missing is a way to utilize an existing asyncio loop.

I was trying to figure out how something like that might work but I got hung up on the Configurable/initialize pattern. Maybe the existing loop could be passed to __init__ and then used in initialize?

[bdarnell]

I think the simplest solution would be to add something like AsyncIOLoop.from_event_loop().

But this is exactly what IOLoop.current() and AsyncIOMainLoop() already do. The problem isn't that we need a way to create an IOLoop corresponding to the current asyncio event loop, the problem is that AsyncTestCase assumes that it is supposed to close the object that is returned from get_new_ioloop().

When IOLoop.instance() and IOLoop.current() worked differently, we had a special case in AsyncTestCase in which the test would not close the IOLoop if it was the same as IOLoop.instance(). I think we need to reintroduce a version of this check in which we only close the IOLoop if a new asyncio event loop was created.

[adamrothman]

That seems reasonable.

I did some more playing around, and I was able to simplify our setup quite a bit. It turns out that pytest-asyncio's event loop fixture doesn't call asyncio.set_event_loop with the loop it creates. Once I added this, returning IOLoop.current() from get_new_ioloop() did exactly the right thing.

And this even works without f47d639, since it doesn't depend on AsyncIOMainLoop or AsyncIOLoop.

[adamrothman]

@bdarnell Just FWIW, I am in the process of upgrading another, more complex service from Tornado 4 to 5. This one uses a lot of async test fixtures that do their own teardown. The teardown fails on 5.0.1 but everything works as expected with your fix from #2327. Thanks!

[bdarnell]

@adamrothman In case you're still following this issue: This interaction between AsyncTestCase and pytest-asyncio has become problematic with changes made in Python 3.10 and 3.12 (See #3223 (comment)). We're probably going to have to effectively revert #2327. I notice that now that the standard library has its own counterpart to Tornado's AsyncTestCase (IsolatedAsyncioTestCase), they never did anything like we did here to support pytest-asyncio. Is there a different solution now?

I've barely ever used pytest so I'm not sure I understand the issue here (our test case here is artificial; we don't have an actual pytest-asyncio example to work with). If you want to lobby for the continued existence of this feature, could you provide a minimal complete example where the issue applies?

[adamrothman]

Thanks for following up! The projects I was working on that prompted me to open this issue are long dead and I don’t write much Python anymore since switching to Go. Feel free to pursue whatever solution makes the most sense for Tornado and it’s users.