Better Handling of Asyncio (#1035)

* wip handle warnings

* switch from pytest-tornasync to pytest-asyncio

* wip

* remove unused plugin

* wip clean up manager

* more progress

* more cleanup

* fix auth tests

* more cleanup

* wip

* fix login test

* clean up clean up and examples

* lint and fix prerelease

* bump to 3.8+

* lint

* make fixtures compatible with pytest-tornasync again

* more cleanup

* autouse does not work in async fixtures without pytest-asyncio

* fix handling of io_loop

* more cleanup

* try isolating the del method

* add a way to close all open sockets

* fix typing

* more socket cleanup

* allow socket warnings for now

* skip some windows tests

* deprecated run_sync_in_loop

* bump pyupgrade

Author: blink1073

.github/workflows/integration-tests.yml

@@ -11,7 +11,7 @@ jobs:
       fail-fast: false
       matrix:
         os: [ubuntu-latest]
-        python-version: ["3.7", "3.8", "3.9", "3.10", "pypy-3.7"]
+        python-version: ["3.8", "3.9", "3.10", "3.11", "pypy-3.8"]
     steps:
       - name: Checkout
         uses: actions/checkout@v2

.github/workflows/python-tests.yml

@@ -18,30 +18,47 @@ jobs:
       fail-fast: false
       matrix:
         os: [ubuntu-latest, windows-latest, macos-latest]
-        python-version: ["3.7", "3.10"]
+        python-version: ["3.8", "3.11"]
         include:
           - os: windows-latest
             python-version: "3.9"
           - os: ubuntu-latest
             python-version: "pypy-3.8"
           - os: macos-latest
-            python-version: "3.8"
+            python-version: "3.10"
     steps:
       - name: Checkout
         uses: actions/checkout@v2
       - name: Base Setup
         uses: jupyterlab/maintainer-tools/.github/actions/base-setup@v1
       - name: Run the tests
         if: ${{ !startsWith(matrix.python-version, 'pypy') && !startsWith(matrix.os, 'windows') }}
-        run: hatch run cov:test || hatch run cov:test --lf
+        run: hatch run cov:test -W default || hatch run cov:test -W default --lf
       - name: Run the tests on pypy and windows
         if: ${{ startsWith(matrix.python-version, 'pypy') || startsWith(matrix.os, 'windows') }}
-        run: hatch run test:test || hatch run test:test --lf
+        run: hatch run test:test  -W default || hatch run test:test  -W default --lf
       - name: Coverage
         run: |
           pip install codecov
           codecov
 
+  client8:
+    runs-on: ${{ matrix.os }}
+    timeout-minutes: 20
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [ubuntu-latest, windows-latest, macos-latest]
+        python-version: ["3.10"]
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v2
+      - name: Base Setup
+        uses: jupyterlab/maintainer-tools/.github/actions/base-setup@v1
+      - run: |
+          pip install -U pre jupyter_client
+          hatch run test:test || hatch run test:test  --lf
+
   pre-commit:
     name: pre-commit
     runs-on: ubuntu-latest
@@ -94,7 +111,7 @@ jobs:
       - name: Base Setup
         uses: jupyterlab/maintainer-tools/.github/actions/base-setup@v1
         with:
-          python_version: "3.7"
+          python_version: "3.8"
       - name: Install miniumum versions
         uses: jupyterlab/maintainer-tools/.github/actions/install-minimums@v1
       - name: Run the unit tests
@@ -110,11 +127,11 @@ jobs:
       - name: Base Setup
         uses: jupyterlab/maintainer-tools/.github/actions/base-setup@v1
         with:
-          python_version: "3.11.0-beta - 3.11.0"
+          python_version: "3.11"
       - name: Install the Python dependencies
         run: |
           pip install --no-deps .
-          pip install --pre --upgrade "jupyter_server[test]"
+          pip install --pre --upgrade ".[test]"
       - name: List installed packages
         run: |
           pip freeze

.pre-commit-config.yaml

@@ -50,7 +50,7 @@ repos:
     rev: v3.2.0
     hooks:
       - id: pyupgrade
-        args: [--py37-plus]
+        args: [--py38-plus]
 
   - repo: https://github.com/PyCQA/doc8
     rev: v1.0.0

examples/simple/README.md

@@ -10,7 +10,7 @@ You need `python3` to build and run the server extensions.
 # Clone, create a conda env and install from source.
 git clone https://github.com/jupyter/jupyter_server && \
   cd examples/simple && \
-  conda create -y -n jupyter-server-example python=3.7 && \
+  conda create -y -n jupyter-server-example python=3.9 && \
   conda activate jupyter-server-example && \
   pip install -e .[test]
 ```

examples/simple/pytest.ini

@@ -1,3 +1,4 @@
 [pytest]
 # Disable any upper exclusion.
 norecursedirs =
+asyncio_mode = auto

examples/simple/setup.py

@@ -37,13 +37,13 @@ def add_data_files(path):
     version=VERSION,
     description="Jupyter Server Example",
     long_description=open("README.md").read(),
-    python_requires=">=3.7",
+    python_requires=">=3.8",
     install_requires=[
         "jupyter_server",
         "jinja2",
     ],
     extras_require={
-        "test": ["pytest"],
+        "test": ["pytest", "pytest-asyncio"],
     },
     include_package_data=True,
     cmdclass=cmdclass,

jupyter_server/__init__.py

@@ -1,8 +1,6 @@
 """The Jupyter Server"""
 import os
 import pathlib
-import subprocess
-import sys
 
 DEFAULT_STATIC_FILES_PATH = os.path.join(os.path.dirname(__file__), "static")
 DEFAULT_TEMPLATE_PATH_LIST = [
@@ -21,12 +19,3 @@
 
 def _cleanup():
     pass
-
-
-# patch subprocess on Windows for python<3.7
-# see https://bugs.python.org/issue37380
-# the fix for python3.7: https://github.com/python/cpython/pull/15706/files
-if sys.platform == "win32":
-    if sys.version_info < (3, 7):
-        subprocess._cleanup = _cleanup
-        subprocess._active = None

jupyter_server/base/zmqhandlers.py

@@ -19,7 +19,7 @@
 from jupyter_client.session import Session
 from tornado import ioloop, web
 from tornado.iostream import IOStream
-from tornado.websocket import WebSocketHandler
+from tornado.websocket import WebSocketClosedError, WebSocketHandler
 
 from .handlers import JupyterHandler
 
@@ -302,7 +302,10 @@ def _on_zmq_reply(self, stream, msg_list):
             except Exception:
                 self.log.critical("Malformed message: %r" % msg_list, exc_info=True)
             else:
-                self.write_message(msg, binary=isinstance(msg, bytes))
+                try:
+                    self.write_message(msg, binary=isinstance(msg, bytes))
+                except WebSocketClosedError as e:
+                    self.log.warning(str(e))
 
 
 class AuthenticatedZMQStreamHandler(ZMQStreamHandler, JupyterHandler):

jupyter_server/pytest_plugin.py

@@ -1,5 +1,6 @@
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
+import asyncio
 import importlib
 import io
 import json
@@ -9,11 +10,14 @@
 import sys
 import urllib.parse
 from binascii import hexlify
+from contextlib import closing
 
 import jupyter_core.paths
 import nbformat
 import pytest
 import tornado
+import tornado.testing
+from pytest_tornasync.plugin import AsyncHTTPServerClient
 from tornado.escape import url_escape
 from tornado.httpclient import HTTPClientError
 from tornado.websocket import WebSocketHandler
@@ -35,16 +39,17 @@
 ]
 
 
-import asyncio
-
-if os.name == "nt" and sys.version_info >= (3, 7):
+if os.name == "nt":
     asyncio.set_event_loop_policy(
         asyncio.WindowsSelectorEventLoopPolicy()  # type:ignore[attr-defined]
     )
 
 
 # ============ Move to Jupyter Core =============
 
+# Once the chunk below moves to Jupyter Core
+# use the fixtures directly from Jupyter Core.
+
 
 def mkdir(tmp_path, *parts):
     path = tmp_path.joinpath(*parts)
@@ -130,6 +135,55 @@ def jp_environ(
 # ================= End: Move to Jupyter core ================
 
 
+@pytest.fixture
+def asyncio_loop():
+    loop = asyncio.new_event_loop()
+    asyncio.set_event_loop(loop)
+    yield loop
+    loop.close()
+
+
+@pytest.fixture(autouse=True)
+def io_loop(asyncio_loop):
+    async def get_tornado_loop():
+        return tornado.ioloop.IOLoop.current()
+
+    return asyncio_loop.run_until_complete(get_tornado_loop())
+
+
+@pytest.fixture
+def http_server_client(http_server, io_loop):
+    """
+    Create an asynchronous HTTP client that can fetch from `http_server`.
+    """
+
+    async def get_client():
+        return AsyncHTTPServerClient(http_server=http_server)
+
+    client = io_loop.run_sync(get_client)
+    with closing(client) as context:
+        yield context
+
+
+@pytest.fixture
+def http_server(io_loop, http_server_port, jp_web_app):
+    """Start a tornado HTTP server that listens on all available interfaces."""
+
+    async def get_server():
+        server = tornado.httpserver.HTTPServer(jp_web_app)
+        server.add_socket(http_server_port[0])
+        return server
+
+    server = io_loop.run_sync(get_server)
+    yield server
+    server.stop()
+
+    if hasattr(server, "close_all_connections"):
+        io_loop.run_sync(server.close_all_connections)
+
+    http_server_port[0].close()
+
+
 @pytest.fixture
 def jp_server_config():
     """Allows tests to setup their specific configuration values."""
@@ -167,7 +221,8 @@ def jp_extension_environ(jp_env_config_path, monkeypatch):
 @pytest.fixture
 def jp_http_port(http_server_port):
     """Returns the port value from the http_server_port fixture."""
-    return http_server_port[-1]
+    yield http_server_port[-1]
+    http_server_port[0].close()
 
 
 @pytest.fixture
@@ -216,8 +271,8 @@ def jp_configurable_serverapp(
     jp_base_url,
     tmp_path,
     jp_root_dir,
-    io_loop,
     jp_logging_stream,
+    asyncio_loop,
 ):
     """Starts a Jupyter Server instance based on
     the provided configuration values.
@@ -254,8 +309,9 @@ def _configurable_serverapp(
     ):
         c = Config(config)
         c.NotebookNotary.db_file = ":memory:"
-        token = hexlify(os.urandom(4)).decode("ascii")
-        c.IdentityProvider.token = token
+        if "token" not in c.ServerApp and not c.IdentityProvider.token:
+            token = hexlify(os.urandom(4)).decode("ascii")
+            c.IdentityProvider.token = token
 
         # Allow tests to configure root_dir via a file, argv, or its
         # default (cwd) by specifying a value of None.
@@ -278,48 +334,29 @@ def _configurable_serverapp(
         app.log.propagate = True
         app.log.handlers = []
         # Initialize app without httpserver
-        app.initialize(argv=argv, new_httpserver=False)
+        if asyncio_loop.is_running():
+            app.initialize(argv=argv, new_httpserver=False)
+        else:
+
+            async def initialize_app():
+                app.initialize(argv=argv, new_httpserver=False)
+
+            asyncio_loop.run_until_complete(initialize_app())
         # Reroute all logging StreamHandlers away from stdin/stdout since pytest hijacks
         # these streams and closes them at unfortunate times.
         stream_handlers = [h for h in app.log.handlers if isinstance(h, logging.StreamHandler)]
         for handler in stream_handlers:
             handler.setStream(jp_logging_stream)
         app.log.propagate = True
         app.log.handlers = []
-        # Start app without ioloop
         app.start_app()
         return app
 
     return _configurable_serverapp
 
 
-@pytest.fixture
-def jp_ensure_app_fixture(request):
-    """Ensures that the 'app' fixture used by pytest-tornasync
-    is set to `jp_web_app`, the Tornado Web Application returned
-    by the ServerApp in Jupyter Server, provided by the jp_web_app
-    fixture in this module.
-
-    Note, this hardcodes the `app_fixture` option from
-    pytest-tornasync to `jp_web_app`. If this value is configured
-    to something other than the default, it will raise an exception.
-    """
-    app_option = request.config.getoption("app_fixture")
-    if app_option not in ["app", "jp_web_app"]:
-        raise Exception(
-            "jp_serverapp requires the `app-fixture` option "
-            "to be set to 'jp_web_app`. Try rerunning the "
-            "current tests with the option `--app-fixture "
-            "jp_web_app`."
-        )
-    elif app_option == "app":
-        # Manually set the app_fixture to `jp_web_app` if it's
-        # not set already.
-        request.config.option.app_fixture = "jp_web_app"
-
-
 @pytest.fixture(scope="function")
-def jp_serverapp(jp_ensure_app_fixture, jp_server_config, jp_argv, jp_configurable_serverapp):
+def jp_serverapp(jp_server_config, jp_argv, jp_configurable_serverapp):
     """Starts a Jupyter Server instance based on the established configuration values."""
     return jp_configurable_serverapp(config=jp_server_config, argv=jp_argv)
 
@@ -482,24 +519,13 @@ def inner(nbpath):
 
 
 @pytest.fixture(autouse=True)
-def jp_server_cleanup(io_loop):
+def jp_server_cleanup(asyncio_loop):
     yield
     app: ServerApp = ServerApp.instance()
-    loop = io_loop.asyncio_loop
-    loop.run_until_complete(app._cleanup())
+    asyncio_loop.run_until_complete(app._cleanup())
     ServerApp.clear_instance()
 
 
-@pytest.fixture
-def jp_cleanup_subprocesses(jp_serverapp):
-    """DEPRECATED: The jp_server_cleanup fixture automatically cleans up the singleton ServerApp class"""
-
-    async def _():
-        pass
-
-    return _
-
-
 @pytest.fixture
 def send_request(jp_fetch, jp_ws_fetch):
     """Send to Jupyter Server and return response code."""