Pytest-Tornasync triggering Deprecation Warnings with Tornado 6.2

[blink1073]

Description

Our prerelease builds are failing due to deprecation warnings:

_________________ ERROR at setup of test_list_running_servers __________________
@pytest.fixture
defio_loop():
"""
    Create new io loop for each test, and tear it down after.
    """
>       loop = tornado.ioloop.IOLoop()
/opt/hostedtoolcache/Python/3.9.13/x64/lib/python3.9/site-packages/pytest_tornasync/plugin.py:64: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/opt/hostedtoolcache/Python/3.9.13/x64/lib/python3.9/site-packages/tornado/util.py:276: in __new__
    instance.initialize(*args, **init_kwargs)
/opt/hostedtoolcache/Python/3.9.13/x64/lib/python3.9/site-packages/tornado/platform/asyncio.py:328: in initialize
super().initialize(loop, **kwargs)
/opt/hostedtoolcache/Python/3.9.13/x64/lib/python3.9/site-packages/tornado/platform/asyncio.py:136: in initialize
super().initialize(**kwargs)
/opt/hostedtoolcache/Python/3.9.13/x64/lib/python3.9/site-packages/tornado/ioloop.py:344: in initialize
self.make_current()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <tornado.platform.asyncio.AsyncIOLoop object at 0x7fb01bd02ee0>
defmake_current(self) -> None:
>       warnings.warn(
"make_current is deprecated; start the event loop first",
DeprecationWarning,
        )
E       DeprecationWarning: make_current is deprecated; start the event loop first
/opt/hostedtoolcache/Python/3.9.13/x64/lib/python3.9/site-packages/tornado/platform/asyncio.py:341: DeprecationWarning

Reproduce

Run the test suite with tornado 6.2 beta.

Expected behavior

Test suite passes

Proposal

We may need to use the approach recommended by tornado, that looks like the following (based on one of their own unit tests):

class MyTestCase(AsyncTestCase):
    @gen_test
    async def test_http_fetch(self):
      pass

[minrk]

To be clear, I believe it is 100% compatible. The use of deprecated APIs that work fine isn't really a compatibility problem, it's a possible future compatibility problem. It's the strict warnings filters in this repo that turn deprecations into errors that cause the failure.

I think deprecations into failures would ideally be treated like we already treat prerelease dependencies in ipykernel - a low priority, low pressure failure that doesn't affect most of the test runs.

This can also be worked around by defining an io_loop fixture in conftest that avoids calling make_current

[blink1073]

FWIW I did try to override the io_loop fixture, but the test suite timed out, which leads me to believe that io_loop was not applied everywhere. I disagree that it is only a possible problem, as the asyncio maintainers have said that in 3.12 get_event_loop will alias to get_running_loop, causing a runtime error. I think we either need to find a way to make the io_loop fixture work or use the testing approach used by tornado itself.

[minrk]

I agree that it needs to be addressed, I just disagree about the priority level of it and time scale. Nothing is broken and won't be for quite some time. It doesn't make sense to me to block all other work to fix something that's not broken.

It's also just factually incorrect that this is incompatible with tornado 6.2. It's presumably incompatible with tornado 7, and 6.2 is informing us what will change with plenty of time to deal with it, under no significant time pressure (at least a year).

[blink1073]

Fair enough, I updated the title. If we want to get CI passing we can add a targeted ignore and open an issue to track.

[minrk]

Having looked at it a bit more, there's un-overrideable logic in pytest-tornasync that uses the deprecated IOLoop.current() with no running loop.

I'd recommend switching to pytest-asyncio as a runner for the async tests, and either vendor the relevant fixtures from pytest-tornasync (just these <100 lines), or switch back to the less intrusive pytest-tornado where the async test runner is optional.

The main reason for pytest-tornasync's creation doesn't exist anymore: async def tests work without marks with pytest-asyncio, and you don't need to use IOLoop.run_sync since it's the same as running with the underlying asyncio loop now.

[minrk]

There are changes in our code necessary to avoid handles on the non-running loop (the APIs deprecated in Python 3.10 and tornado 6.2). So far, I've only found our couple of PeriodicCallbacks, which must be called from inside the event loop.

One simple way to do some of that would be to change the outermost launch_instance or something to call asyncio.run(). That would ensure that absolutely everywhere in our code, the event loop is always running. That's a bit of pain with our separate 'initialize' and 'start' steps, because 'start' normally runs the event loop itself.

[frenzymadness]

I've found this issue when trying to understand why jupyter-server depends on pytest-tornasync. FYI the pytest-tornasync seems to be abandoned for years. I'm trying to package all dependencies of Jupyter into Fedora Linux and the status of tornasync makes is much harder. Is there any chance you'll remove that dependency and switch to something newer/better?

[Zsailer]

@frenzymadness can you help us understand exactly what the issue is when packaging on Fedora?

A couple of things to note...

pytest-tornasync hasn't cut a release in awhile, true, but I believe this is because the plugin was essentially "feature complete" from its start. It has a very narrow scope—managing the tornado async event loop between tests.

pytest-tornasync isn't a dependency of jupyter_server—it's only a dependency for jupyter_server's unit tests. It shouldn't be required unless you're installing the Jupyter Server's test dependencies explicitly.

Coincidently, @blink1073 is in the process of moving our fixtures entirely out of the jupyter_server package into the (newly revived) pytest-jupyter package. Currently, pytest-jupyter still uses pytest-tornasync, but the isolation of these dependencies are a bit more clean.

Also, I should mention, we have recently discussed manually copying out the fixtures from pytest-tornasync and adding them directly to pytest-jupyter (adding proper attribution/credit/lincensing to pytest-tornasync), so we could evolve these fixtures further. The issues you're seeing might be further motivation to finish that work.

Before we go down this road, it would help us to understand your issue more clearly.

[frenzymadness]

@Zsailer sure thing, thanks for your interest. The first thing to note is that when building RPM packages, it's best practice to run its unittests which means that I should package also test dependencies. As a maintainer of an RPM package in a Linux distro, I'm responsible for it being buildable and installable, and having an active upstream development is always good because when a problem appears (for example, in Fedora, we've already started testing Python packages with Python 3.12) I have to either drop the package or fix it. And dropping a package is harder when other packages already depend on it. And when I see a project like tornasync, I hesitate to package it as an RPM because it seems to be inactive in general with some specific issues making my packaging work harder, namely:

• taking sources from PyPI does not work because there are missing files in tests. A PR to fix this exists for more than 2 years but nobody merged it. I can take sources from github but there are missing tags so it's hard to say which commit represents the last release on PyPI. Again, the issue for missing tags exists for more than a year with no response.
• tests configuration in tox does not contain python 3.8+, pytest 5+ and tornado 6. It seems that the project works also with newer versions but there is no active testing from upstream. Also, using Travis means that there are paying for it (unlikely) or that the CI does not work.

To be 100% honest. We have tried to package JupyterLab into Fedora multiple times but the effort failed because the package is too complex and requires too many dependencies. Because it's possible to install it from PyPI into a virtual environment, we kept only Notebook in the distro so we have at least something directly available to users. But now, Notebook 7 is also based on the same components as JupyterLab so I'm trying it again.

I've also discovered one possibly related problem. If I want to package jupyter-server, I have to also package jupyter-server-terminals because it's a runtime dependency of jupyter-server. But jupyter-server-terminals has jupyter-server[test] in its testing dependencies which creates a dependency loop. It's possible to break a dependency loop like this by packaging the first version of each package without tests but having a new package with pytest fixtures both of those packages will depend on can also break it and make it easier.

My current plan is to package server-terminals without tests and then jupyter-server without tornasync (if possible) or also without tests to skip the need to package tornasync.

[blink1073]

Given the above, I think the best course is for us to inline pytest-tornasync in pytest-jupyter. I'll do that today.

[blink1073]

https://github.com/jupyter-server/pytest-jupyter/releases/tag/v0.5.0

[frenzymadness]

Thanks for this! There is still a dependency loop between jupyter-server and pytest-jupyter but at least I don't have to package the tornasync package. By the way, jupyter-server-terminals also depends on tornasync which is something you might want to change.

[blink1073]

Thanks, yes, there are some follow up tasks to move some libraries onto pytest-jupyter.

[blink1073]

https://github.com/jupyter-server/jupyter_server_terminals/releases/tag/v0.4.2