From 1b9ca233a6a89aabadc0faf7b8947f27ee5e2f18 Mon Sep 17 00:00:00 2001 From: GitHub Action <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 25 Oct 2021 06:24:36 -0500 Subject: [PATCH] wip --- jupyter_server/pytest_plugin.py | 7 +- .../services/kernels/kernelmanager.py | 2 - jupyter_server/services/sessions/handlers.py | 6 +- .../tests/services/kernels/test_api.py | 25 +++++-- .../tests/services/kernels/test_cull.py | 26 ++++++- .../tests/services/sessions/test_api.py | 68 ++++++++++++++++++- 6 files changed, 121 insertions(+), 13 deletions(-) diff --git a/jupyter_server/pytest_plugin.py b/jupyter_server/pytest_plugin.py index 66093ff53b..dfbef483ee 100644 --- a/jupyter_server/pytest_plugin.py +++ b/jupyter_server/pytest_plugin.py @@ -416,13 +416,16 @@ def client_fetch(*parts, headers=None, params=None, **kwargs): @pytest.fixture def jp_kernelspecs(jp_data_dir): """Configures some sample kernelspecs in the Jupyter data directory.""" - spec_names = ["sample", "sample 2"] + spec_names = ["sample", "sample 2", "bad"] for name in spec_names: sample_kernel_dir = jp_data_dir.joinpath("kernels", name) sample_kernel_dir.mkdir(parents=True) # Create kernel json file sample_kernel_file = sample_kernel_dir.joinpath("kernel.json") - sample_kernel_file.write_text(json.dumps(sample_kernel_json)) + kernel_json = sample_kernel_json.copy() + if name == "bad": + kernel_json["argv"] = ["non_existent_path"] + sample_kernel_file.write_text(json.dumps(kernel_json)) # Create resources text sample_kernel_resources = sample_kernel_dir.joinpath("resource.txt") sample_kernel_resources.write_text(some_resource) diff --git a/jupyter_server/services/kernels/kernelmanager.py b/jupyter_server/services/kernels/kernelmanager.py index 6d05f3f00f..638108911b 100644 --- a/jupyter_server/services/kernels/kernelmanager.py +++ b/jupyter_server/services/kernels/kernelmanager.py @@ -52,8 +52,6 @@ def _default_kernel_manager_class(self): kernel_argv = List(Unicode()) - use_pending_kernels = True - root_dir = Unicode(config=True) _kernel_connections = Dict() diff --git a/jupyter_server/services/sessions/handlers.py b/jupyter_server/services/sessions/handlers.py index 5eba2e660b..b29cb7583f 100644 --- a/jupyter_server/services/sessions/handlers.py +++ b/jupyter_server/services/sessions/handlers.py @@ -4,6 +4,7 @@ """ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. +import asyncio import json try: @@ -146,7 +147,10 @@ async def patch(self, session_id): if model["kernel"]["id"] != before["kernel"]["id"]: # kernel_id changed because we got a new kernel # shutdown the old one - await ensure_async(km.shutdown_kernel(before["kernel"]["id"])) + fut = asyncio.ensure_future(ensure_async(km.shutdown_kernel(before["kernel"]["id"]))) + # If we are not using pending kernels, wait for the kernel to shut down + if not getattr(km, "use_pending_kernels"): + await fut self.finish(json.dumps(model, default=json_default)) @web.authenticated diff --git a/jupyter_server/tests/services/kernels/test_api.py b/jupyter_server/tests/services/kernels/test_api.py index 945343fd62..d6564a5767 100644 --- a/jupyter_server/tests/services/kernels/test_api.py +++ b/jupyter_server/tests/services/kernels/test_api.py @@ -4,6 +4,7 @@ import pytest import tornado from jupyter_client.kernelspec import NATIVE_KERNEL_NAME +from tornado.httpclient import HTTPClientError from ...utils import expected_http_error from jupyter_server.utils import url_path_join @@ -150,12 +151,28 @@ async def test_kernel_handler(jp_fetch, jp_cleanup_subprocesses, jp_serverapp, u await jp_cleanup_subprocesses() -@pytest.mark.parametrize("use_pending_kernels", (False, True)) async def test_kernel_handler_startup_error( - jp_fetch, jp_cleanup_subprocesses, jp_serverapp, use_pending_kernels + jp_fetch, jp_cleanup_subprocesses, jp_serverapp, jp_kernelspecs +): + jp_serverapp.kernel_manager.use_pending_kernels = False + # Create a kernel + with pytest.raises(HTTPClientError): + await jp_fetch("api", "kernels", method="POST", body=json.dumps({"name": "bad"})) + + +async def test_kernel_handler_startup_error_pending( + jp_fetch, jp_ws_fetch, jp_cleanup_subprocesses, jp_serverapp, jp_kernelspecs ): - pass - # TODO + if not hasattr(jp_serverapp.kernel_manager, "use_pending_kernels"): + return + + jp_serverapp.kernel_manager.use_pending_kernels = True + # Create a kernel + r = await jp_fetch("api", "kernels", method="POST", body=json.dumps({"name": "bad"})) + kid = json.loads(r.body.decode())["id"] + + with pytest.raises(HTTPClientError): + await jp_ws_fetch("api", "kernels", kid, "channels") async def test_connection( diff --git a/jupyter_server/tests/services/kernels/test_cull.py b/jupyter_server/tests/services/kernels/test_cull.py index f61884251f..a2c83bd84b 100644 --- a/jupyter_server/tests/services/kernels/test_cull.py +++ b/jupyter_server/tests/services/kernels/test_cull.py @@ -34,7 +34,7 @@ def jp_server_config(): ) -async def test_culling(jp_fetch, jp_ws_fetch, jp_cleanup_subprocesses): +async def test_cull_idle(jp_fetch, jp_ws_fetch, jp_cleanup_subprocesses): r = await jp_fetch("api", "kernels", method="POST", allow_nonstandard_methods=True) kernel = json.loads(r.body.decode()) kid = kernel["id"] @@ -53,6 +53,30 @@ async def test_culling(jp_fetch, jp_ws_fetch, jp_cleanup_subprocesses): await jp_cleanup_subprocesses() +async def test_cull_dead( + jp_fetch, jp_ws_fetch, jp_serverapp, jp_cleanup_subprocesses, jp_kernelspecs +): + if not hasattr(jp_serverapp.kernel_manager, "use_pending_kernels"): + return + + jp_serverapp.kernel_manager.use_pending_kernels = True + jp_serverapp.kernel_manager.default_kernel_name = "bad" + r = await jp_fetch("api", "kernels", method="POST", allow_nonstandard_methods=True) + kernel = json.loads(r.body.decode()) + kid = kernel["id"] + + # Open a websocket connection. + with pytest.raises(HTTPClientError): + await jp_ws_fetch("api", "kernels", kid, "channels") + + r = await jp_fetch("api", "kernels", kid, method="GET") + model = json.loads(r.body.decode()) + assert model["connections"] == 0 + culled = await get_cull_status(kid, jp_fetch) # connected, should not be culled + assert culled + await jp_cleanup_subprocesses() + + async def get_cull_status(kid, jp_fetch): frequency = 0.5 culled = False diff --git a/jupyter_server/tests/services/sessions/test_api.py b/jupyter_server/tests/services/sessions/test_api.py index 4258cec832..f4d963ff41 100644 --- a/jupyter_server/tests/services/sessions/test_api.py +++ b/jupyter_server/tests/services/sessions/test_api.py @@ -7,6 +7,7 @@ from jupyter_client.ioloop import AsyncIOLoopKernelManager from nbformat import writes from nbformat.v4 import new_notebook +from tornado.httpclient import HTTPClientError from traitlets import default from ...utils import expected_http_error @@ -68,7 +69,7 @@ async def list(self): async def get(self, id): return await self._req(id, method="GET") - async def create(self, path, type="notebook", kernel_name="python", kernel_id=None): + async def create(self, path, type="notebook", kernel_name=None, kernel_id=None): body = {"path": path, "type": type, "kernel": {"name": kernel_name, "id": kernel_id}} return await self._req(method="POST", body=body) @@ -197,6 +198,60 @@ async def test_create( await jp_cleanup_subprocesses() +async def test_create_bad( + session_client, jp_base_url, jp_cleanup_subprocesses, jp_serverapp, jp_kernelspecs +): + jp_serverapp.kernel_manager.use_pending_kernels = False + + # Make sure no sessions exist. + jp_serverapp.kernel_manager.default_kernel_name = "bad" + resp = await session_client.list() + sessions = j(resp) + assert len(sessions) == 0 + + # Create a session. + with pytest.raises(HTTPClientError): + await session_client.create("foo/nb1.ipynb") + + # Need to find a better solution to this. + await session_client.cleanup() + await jp_cleanup_subprocesses() + + +async def test_create_bad_pending( + session_client, jp_base_url, jp_ws_fetch, jp_cleanup_subprocesses, jp_serverapp, jp_kernelspecs +): + if not hasattr(jp_serverapp.kernel_manager, "use_pending_kernels"): + return + + jp_serverapp.kernel_manager.use_pending_kernels = True + + # Make sure no sessions exist. + jp_serverapp.kernel_manager.default_kernel_name = "bad" + resp = await session_client.list() + sessions = j(resp) + assert len(sessions) == 0 + + # Create a session. + resp = await session_client.create("foo/nb1.ipynb") + assert resp.code == 201 + + # Open a websocket connection. + kid = j(resp)["kernel"]["id"] + with pytest.raises(HTTPClientError): + await jp_ws_fetch("api", "kernels", kid, "channels") + + # Get the updated kernel state + resp = await session_client.list() + session = j(resp)[0] + assert session["kernel"]["execution_state"] == "dead" + assert "non_existent_path" in session["kernel"]["reason"] + + # Need to find a better solution to this. + await session_client.cleanup() + await jp_cleanup_subprocesses() + + @pytest.mark.parametrize("use_pending_kernels", (False, True)) async def test_create_file_session( session_client, jp_cleanup_subprocesses, jp_serverapp, use_pending_kernels @@ -393,7 +448,11 @@ async def test_modify_kernel_name( kernel_list = j(resp) after["kernel"].pop("last_activity") [k.pop("last_activity") for k in kernel_list] - assert kernel_list == [after["kernel"]] + if not use_pending_kernels: + assert kernel_list == [after["kernel"]] + else: + assert len(kernel_list) == 2 + # Need to find a better solution to this. await session_client.cleanup() await jp_cleanup_subprocesses() @@ -427,7 +486,10 @@ async def test_modify_kernel_id( kernel.pop("last_activity") [k.pop("last_activity") for k in kernel_list] - assert kernel_list == [kernel] + if not use_pending_kernels: + assert kernel_list == [kernel] + else: + assert len(kernel_list) == 2 # Need to find a better solution to this. await session_client.cleanup()