Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix "DeprecationWarning: There is no current event loop" in test suite #2772

Closed
pquentin opened this issue Nov 8, 2022 · 7 comments · Fixed by #2777
Closed

Fix "DeprecationWarning: There is no current event loop" in test suite #2772

pquentin opened this issue Nov 8, 2022 · 7 comments · Fixed by #2777

Comments

@pquentin
Copy link
Member

pquentin commented Nov 8, 2022

Starting with Python 3.10, using asyncio.get_event_loop() in a synchronous context is deprecated in favor of asyncio.run and will stop working eventually. This is why we get a warning when calling tornado.ioloop.IOLoop.current() in our test suite: it calls asyncio.get_event_loop() under the hood.

Tornado has always encouraged users to do what we do: setup the server synchronously, and then start it in a background thread. This was quite cool as we did not have to worry about async at all. To address the Python 3.10 deprecation, Tornado 6.2 now reluctantly encourages calling asyncio.run() and doing the setup in async code. However, just making run_tornado_app async won't be enough in our case because our synchronous test code needs to know the port number of the server. And retrieving that value from the background thread is likely to be annoying.

What we can do instead is partially setup the server synchronously (and thus get the port number) and then finalize setup in async. It does require some synchronization but I now have a working proof-of-concept that passes even with -Werror:

import asyncio
import threading

import tornado.web

import urllib3


class MainHandler(tornado.web.RequestHandler):
    def get(self):
        self.write("Hello, world")


class TornadoServerThread(threading.Thread):
    def __init__(self):
        super().__init__()
        app = tornado.web.Application([(r"/", MainHandler)])

        self.server = tornado.httpserver.HTTPServer(app)
        self.sockets = tornado.netutil.bind_sockets(None, address="localhost")
        self.port = self.sockets[0].getsockname()[1]

        self.started = threading.Event()
        self.stopped = asyncio.Event()

    async def main(self):
        self._loop = asyncio.get_event_loop()
        self.server.add_sockets(self.sockets)
        self.started.set()
        await self.stopped.wait()

    def run(self):
        print("run")
        asyncio.run(self.main())

    def stop(self):
        print("stop")
        self._loop.call_soon_threadsafe(self.stopped.set)


t = TornadoServerThread()
t.start()
t.started.wait()
print(t.port)
print(urllib3.request("GET", f"http://localhost:{t.port}").data)
t.stop()

(I initially tried to use IOLoop(make_current=False) as suggested in tornadoweb/tornado#3156 but did not manage to make it work without warnings.)

This issue is about taking this proof-of-concept (or coming with a better one) and use it in urllib3's test suite. We should then verify that running with Python 3.10 and Tornado 6.2 does not raise any deprecation warning.

@graingert
Copy link
Contributor

Will need to be something like this instead:

import asyncio
import threading

import tornado.web

import urllib3


class MainHandler(tornado.web.RequestHandler):
    def get(self):
        self.write("Hello, world")


class TornadoServerThread(threading.Thread):
    def __init__(self):
        super().__init__()
        self.started = threading.Event()

    async def main(self):
        app = tornado.web.Application([(r"/", MainHandler)])

        self.server = tornado.httpserver.HTTPServer(app)
        self.sockets = tornado.netutil.bind_sockets(None, address="localhost")
        self.port = self.sockets[0].getsockname()[1]
        self._loop = asyncio.get_running_loop()
        self.server.add_sockets(self.sockets)
        self.started.set()
        self.stopped = asyncio.Event()
        await self.stopped.wait()

    def run(self):
        print("run")
        asyncio.run(self.main())

    def stop(self):
        print("stop")
        self._loop.call_soon_threadsafe(self.stopped.set)
        self.join()

@pquentin
Copy link
Member Author

pquentin commented Nov 9, 2022

@graingert Right putting more code in the main function is likely more elegant. (I would still initialize self.stopped in the constructor though.) But why do you say it needs to be like this?

@graingert
Copy link
Contributor

asyncio.Event() calls get_event_loop prior to 3.10

@pquentin
Copy link
Member Author

pquentin commented Nov 9, 2022

But that's OK, the warnings only happen with Python 3.10+

@graingert
Copy link
Contributor

But you want the code to work on all python versions

@pquentin
Copy link
Member Author

pquentin commented Nov 9, 2022

Oh, I get it now! On older versions we would have two different loops which will break. Thanks for the help and patience. 👍

@graingert
Copy link
Contributor

@pquentin I have a PR #2777 to do this change

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants