diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 39cf8836fe..0d72a2d239 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -21,7 +21,7 @@ repos: - id: trailing-whitespace - repo: https://github.com/python-jsonschema/check-jsonschema - rev: 0.28.4 + rev: 0.28.6 hooks: - id: check-github-workflows @@ -52,7 +52,7 @@ repos: - id: rst-inline-touching-normal - repo: https://github.com/pre-commit/mirrors-mypy - rev: "v1.10.0" + rev: "v1.10.1" hooks: - id: mypy files: jupyter_server @@ -61,7 +61,7 @@ repos: ["traitlets>=5.13", "jupyter_core>=5.5", "jupyter_client>=8.5"] - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.4.7 + rev: v0.5.0 hooks: - id: ruff types_or: [python, jupyter] diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fa88c9b00..cadf2e8a93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,34 @@ All notable changes to this project will be documented in this file. +## 2.14.2 + +([Full Changelog](https://github.com/jupyter-server/jupyter_server/compare/v2.14.1...b961d4eb499071c0c60e24f429c20d1e6a908a32)) + +### Bugs fixed + +- Pass session_id during Websocket connect [#1440](https://github.com/jupyter-server/jupyter_server/pull/1440) ([@gogasca](https://github.com/gogasca)) +- Do not log environment variables passed to kernels [#1437](https://github.com/jupyter-server/jupyter_server/pull/1437) ([@krassowski](https://github.com/krassowski)) + +### Maintenance and upkeep improvements + +- chore: update pre-commit hooks [#1441](https://github.com/jupyter-server/jupyter_server/pull/1441) ([@pre-commit-ci](https://github.com/pre-commit-ci)) +- chore: update pre-commit hooks [#1427](https://github.com/jupyter-server/jupyter_server/pull/1427) ([@pre-commit-ci](https://github.com/pre-commit-ci)) + +### Documentation improvements + +- Update documentation for `cookie_secret` [#1433](https://github.com/jupyter-server/jupyter_server/pull/1433) ([@krassowski](https://github.com/krassowski)) +- Add Changelog for 2.14.1 [#1430](https://github.com/jupyter-server/jupyter_server/pull/1430) ([@blink1073](https://github.com/blink1073)) +- Update simple extension examples: \_jupyter_server_extension_points [#1426](https://github.com/jupyter-server/jupyter_server/pull/1426) ([@manics](https://github.com/manics)) + +### Contributors to this release + +([GitHub contributors page for this release](https://github.com/jupyter-server/jupyter_server/graphs/contributors?from=2024-05-31&to=2024-07-12&type=c)) + +[@blink1073](https://github.com/search?q=repo%3Ajupyter-server%2Fjupyter_server+involves%3Ablink1073+updated%3A2024-05-31..2024-07-12&type=Issues) | [@gogasca](https://github.com/search?q=repo%3Ajupyter-server%2Fjupyter_server+involves%3Agogasca+updated%3A2024-05-31..2024-07-12&type=Issues) | [@krassowski](https://github.com/search?q=repo%3Ajupyter-server%2Fjupyter_server+involves%3Akrassowski+updated%3A2024-05-31..2024-07-12&type=Issues) | [@manics](https://github.com/search?q=repo%3Ajupyter-server%2Fjupyter_server+involves%3Amanics+updated%3A2024-05-31..2024-07-12&type=Issues) | [@pre-commit-ci](https://github.com/search?q=repo%3Ajupyter-server%2Fjupyter_server+involves%3Apre-commit-ci+updated%3A2024-05-31..2024-07-12&type=Issues) + + + ## 2.14.1 ([Full Changelog](https://github.com/jupyter-server/jupyter_server/compare/v2.14.0...f1379164fa209bc4bfeadf43ab0e7f473b03a0ce)) @@ -27,8 +55,6 @@ All notable changes to this project will be documented in this file. [@blink1073](https://github.com/search?q=repo%3Ajupyter-server%2Fjupyter_server+involves%3Ablink1073+updated%3A2024-04-11..2024-05-31&type=Issues) | [@lresende](https://github.com/search?q=repo%3Ajupyter-server%2Fjupyter_server+involves%3Alresende+updated%3A2024-04-11..2024-05-31&type=Issues) | [@pre-commit-ci](https://github.com/search?q=repo%3Ajupyter-server%2Fjupyter_server+involves%3Apre-commit-ci+updated%3A2024-04-11..2024-05-31&type=Issues) - - ## 2.14.0 ([Full Changelog](https://github.com/jupyter-server/jupyter_server/compare/v2.13.0...074628806d6b2ec3304d60ab5cfba1c326f67730)) diff --git a/jupyter_server/gateway/connections.py b/jupyter_server/gateway/connections.py index 8027a822cc..d4dde730fa 100644 --- a/jupyter_server/gateway/connections.py +++ b/jupyter_server/gateway/connections.py @@ -47,6 +47,8 @@ async def connect(self): url_escape(self.kernel_id), "channels", ) + if self.session_id: + ws_url += f"?session_id={url_escape(self.session_id)}" self.log.info(f"Connecting to {ws_url}") kwargs: dict[str, Any] = {} kwargs = GatewayClient.instance().load_connection_args(**kwargs) diff --git a/jupyter_server/serverapp.py b/jupyter_server/serverapp.py index ed0738976a..3dedd5634f 100644 --- a/jupyter_server/serverapp.py +++ b/jupyter_server/serverapp.py @@ -1139,8 +1139,9 @@ def _default_cookie_secret_file(self) -> str: b"", config=True, help="""The random bytes used to secure cookies. - By default this is a new random number every time you start the server. - Set it to a value in a config file to enable logins to persist across server sessions. + By default this is generated on first start of the server and persisted across server + sessions by writing the cookie secret into the `cookie_secret_file` file. + When using an executable config file you can override this to be random at each server restart. Note: Cookie secrets should be kept private, do not share config files with cookie_secret stored in plaintext (you can read the value from a file). diff --git a/jupyter_server/services/contents/handlers.py b/jupyter_server/services/contents/handlers.py index ad25a2d3f7..13e987809b 100644 --- a/jupyter_server/services/contents/handlers.py +++ b/jupyter_server/services/contents/handlers.py @@ -140,7 +140,9 @@ async def get(self, path=""): hash_str = self.get_query_argument("hash", default="0") if hash_str not in {"0", "1"}: - raise web.HTTPError(400, f"Content {hash_str!r} is invalid") + raise web.HTTPError( + 400, f"Hash argument {hash_str!r} is invalid. It must be '0' or '1'." + ) require_hash = int(hash_str) if not cm.allow_hidden and await ensure_async(cm.is_hidden(path)): diff --git a/jupyter_server/services/kernels/kernelmanager.py b/jupyter_server/services/kernels/kernelmanager.py index 5b0d09aab2..cd8a9de71f 100644 --- a/jupyter_server/services/kernels/kernelmanager.py +++ b/jupyter_server/services/kernels/kernelmanager.py @@ -232,18 +232,25 @@ async def _async_start_kernel( # type:ignore[override] kwargs["kernel_id"] = kernel_id kernel_id = await self.pinned_superclass._async_start_kernel(self, **kwargs) self._kernel_connections[kernel_id] = 0 - task = asyncio.create_task(self._finish_kernel_start(kernel_id)) - if not getattr(self, "use_pending_kernels", None): - await task - else: - self._pending_kernel_tasks[kernel_id] = task + # add busy/activity markers: kernel = self.get_kernel(kernel_id) kernel.execution_state = "starting" # type:ignore[attr-defined] kernel.reason = "" # type:ignore[attr-defined] kernel.last_activity = utcnow() # type:ignore[attr-defined] self.log.info("Kernel started: %s", kernel_id) - self.log.debug("Kernel args: %r", kwargs) + self.log.debug( + "Kernel args (excluding env): %r", {k: v for k, v in kwargs.items() if k != "env"} + ) + env = kwargs.get("env", None) + if env and isinstance(env, dict): # type:ignore[unreachable] + self.log.debug("Kernel argument 'env' passed with: %r", list(env.keys())) # type:ignore[unreachable] + + task = asyncio.create_task(self._finish_kernel_start(kernel_id)) + if not getattr(self, "use_pending_kernels", None): + await task + else: + self._pending_kernel_tasks[kernel_id] = task # Increase the metric of number of kernels running # for the relevant kernel type by 1 @@ -532,6 +539,40 @@ def _check_kernel_id(self, kernel_id): raise web.HTTPError(404, "Kernel does not exist: %s" % kernel_id) # monitoring activity: + untracked_message_types = List( + trait=Unicode(), + config=True, + default_value=[ + "comm_info_request", + "comm_info_reply", + "kernel_info_request", + "kernel_info_reply", + "shutdown_request", + "shutdown_reply", + "interrupt_request", + "interrupt_reply", + "debug_request", + "debug_reply", + "stream", + "display_data", + "update_display_data", + "execute_input", + "execute_result", + "error", + "status", + "clear_output", + "debug_event", + "input_request", + "input_reply", + ], + help="""List of kernel message types excluded from user activity tracking. + + This should be a superset of the message types sent on any channel other + than the shell channel.""", + ) + + def track_message_type(self, message_type): + return message_type not in self.untracked_message_types def start_watching_activity(self, kernel_id): """Start watching IOPub messages on a kernel for activity. @@ -552,15 +593,27 @@ def start_watching_activity(self, kernel_id): def record_activity(msg_list): """Record an IOPub message arriving from a kernel""" - self.last_kernel_activity = kernel.last_activity = utcnow() - idents, fed_msg_list = session.feed_identities(msg_list) msg = session.deserialize(fed_msg_list, content=False) msg_type = msg["header"]["msg_type"] + parent_msg_type = msg.get("parent_header", {}).get("msg_type", None) + if ( + self.track_message_type(msg_type) + or self.track_message_type(parent_msg_type) + or kernel.execution_state == "busy" + ): + self.last_kernel_activity = kernel.last_activity = utcnow() if msg_type == "status": msg = session.deserialize(fed_msg_list) - kernel.execution_state = msg["content"]["execution_state"] + execution_state = msg["content"]["execution_state"] + if self.track_message_type(parent_msg_type): + kernel.execution_state = execution_state + elif kernel.execution_state == "starting" and execution_state != "starting": + # We always normalize post-starting execution state to "idle" + # unless we know that the status is in response to one of our + # tracked message types. + kernel.execution_state = "idle" self.log.debug( "activity on %s: %s (%s)", kernel_id, diff --git a/tests/services/kernels/test_cull.py b/tests/services/kernels/test_cull.py index 50ecbf2b96..5b0b8fd9a0 100644 --- a/tests/services/kernels/test_cull.py +++ b/tests/services/kernels/test_cull.py @@ -1,7 +1,9 @@ import asyncio +import datetime import json import os import platform +import uuid import warnings import jupyter_client @@ -94,6 +96,83 @@ async def test_cull_idle(jp_fetch, jp_ws_fetch): assert culled +@pytest.mark.parametrize( + "jp_server_config", + [ + # Test the synchronous case + Config( + { + "ServerApp": { + "kernel_manager_class": "jupyter_server.services.kernels.kernelmanager.MappingKernelManager", + "MappingKernelManager": { + "cull_idle_timeout": CULL_TIMEOUT, + "cull_interval": CULL_INTERVAL, + "cull_connected": True, + }, + } + } + ), + # Test the async case + Config( + { + "ServerApp": { + "kernel_manager_class": "jupyter_server.services.kernels.kernelmanager.AsyncMappingKernelManager", + "AsyncMappingKernelManager": { + "cull_idle_timeout": CULL_TIMEOUT, + "cull_interval": CULL_INTERVAL, + "cull_connected": True, + }, + } + } + ), + ], +) +async def test_cull_connected(jp_fetch, jp_ws_fetch): + 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. + ws = await jp_ws_fetch("api", "kernels", kid, "channels") + session_id = uuid.uuid1().hex + message_id = uuid.uuid1().hex + await ws.write_message( + json.dumps( + { + "channel": "shell", + "header": { + "date": datetime.datetime.now(tz=datetime.timezone.utc).isoformat(), + "session": session_id, + "msg_id": message_id, + "msg_type": "execute_request", + "username": "", + "version": "5.2", + }, + "parent_header": {}, + "metadata": {}, + "content": { + "code": f"import time\ntime.sleep({CULL_TIMEOUT-1})", + "silent": False, + "allow_stdin": False, + "stop_on_error": True, + }, + "buffers": [], + } + ) + ) + + r = await jp_fetch("api", "kernels", kid, method="GET") + model = json.loads(r.body.decode()) + assert model["connections"] == 1 + culled = await get_cull_status( + kid, jp_fetch + ) # connected, but code cell still running. Should not be culled + assert not culled + culled = await get_cull_status(kid, jp_fetch) # still connected, but idle... should be culled + assert culled + ws.close() + + async def test_cull_idle_disable(jp_fetch, jp_ws_fetch, jp_kernelspec_with_metadata): r = await jp_fetch("api", "kernels", method="POST", allow_nonstandard_methods=True) kernel = json.loads(r.body.decode()) diff --git a/tests/services/kernels/test_execution_state.py b/tests/services/kernels/test_execution_state.py new file mode 100644 index 0000000000..50155ec76f --- /dev/null +++ b/tests/services/kernels/test_execution_state.py @@ -0,0 +1,146 @@ +import asyncio +import datetime +import json +import os +import platform +import time +import uuid +import warnings + +import jupyter_client +import pytest +from flaky import flaky +from tornado.httpclient import HTTPClientError +from traitlets.config import Config + +MAX_POLL_ATTEMPTS = 10 +POLL_INTERVAL = 1 +MINIMUM_CONSISTENT_COUNT = 4 + + +@flaky +async def test_execution_state(jp_fetch, jp_ws_fetch): + 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. + ws = await jp_ws_fetch("api", "kernels", kid, "channels") + session_id = uuid.uuid1().hex + message_id = uuid.uuid1().hex + await ws.write_message( + json.dumps( + { + "channel": "shell", + "header": { + "date": datetime.datetime.now(tz=datetime.timezone.utc).isoformat(), + "session": session_id, + "msg_id": message_id, + "msg_type": "execute_request", + "username": "", + "version": "5.2", + }, + "parent_header": {}, + "metadata": {}, + "content": { + "code": "while True:\n\tpass", + "silent": False, + "allow_stdin": False, + "stop_on_error": True, + }, + "buffers": [], + } + ) + ) + await poll_for_parent_message_status(kid, message_id, "busy", ws) + es = await get_execution_state(kid, jp_fetch) + assert es == "busy" + + message_id_2 = uuid.uuid1().hex + await ws.write_message( + json.dumps( + { + "channel": "control", + "header": { + "date": datetime.datetime.now(tz=datetime.timezone.utc).isoformat(), + "session": session_id, + "msg_id": message_id_2, + "msg_type": "debug_request", + "username": "", + "version": "5.2", + }, + "parent_header": {}, + "metadata": {}, + "content": { + "type": "request", + "command": "debugInfo", + }, + "buffers": [], + } + ) + ) + await poll_for_parent_message_status(kid, message_id_2, "idle", ws) + es = await get_execution_state(kid, jp_fetch) + + # Verify that the overall kernel status is still "busy" even though one + # "idle" response was already seen for the second execute request. + assert es == "busy" + + await jp_fetch( + "api", + "kernels", + kid, + "interrupt", + method="POST", + allow_nonstandard_methods=True, + ) + + await poll_for_parent_message_status(kid, message_id, "idle", ws) + es = await get_execution_state(kid, jp_fetch) + assert es == "idle" + ws.close() + + +async def get_execution_state(kid, jp_fetch): + # There is an inherent race condition when getting the kernel execution status + # where we might fetch the status right before an expected state change occurs. + # + # To work-around this, we don't return the status until we've been able to fetch + # it twice in a row and get the same result both times. + last_execution_states = [] + + for _ in range(MAX_POLL_ATTEMPTS): + r = await jp_fetch("api", "kernels", kid, method="GET") + model = json.loads(r.body.decode()) + execution_state = model["execution_state"] + last_execution_states.append(execution_state) + consistent_count = 0 + last_execution_state = None + for es in last_execution_states: + if es != last_execution_state: + consistent_count = 0 + last_execution_state = es + consistent_count += 1 + if consistent_count >= MINIMUM_CONSISTENT_COUNT: + return es + time.sleep(POLL_INTERVAL) + + raise AssertionError("failed to get a consistent execution state") + + +async def poll_for_parent_message_status(kid, parent_message_id, target_status, ws): + while True: + resp = await ws.read_message() + resp_json = json.loads(resp) + print(resp_json) + parent_message = resp_json.get("parent_header", {}).get("msg_id", None) + if parent_message != parent_message_id: + continue + + response_type = resp_json.get("header", {}).get("msg_type", None) + if response_type != "status": + continue + + execution_state = resp_json.get("content", {}).get("execution_state", "") + if execution_state == target_status: + return diff --git a/tests/test_gateway.py b/tests/test_gateway.py index f0033c278e..569268d833 100644 --- a/tests/test_gateway.py +++ b/tests/test_gateway.py @@ -200,6 +200,7 @@ async def mock_gateway_request(url, **kwargs): mocked_gateway = patch("jupyter_server.gateway.managers.gateway_request", mock_gateway_request) +mock_gateway_ws_url = "ws://mock-gateway-server:8889" mock_gateway_url = "http://mock-gateway-server:8889" mock_http_user = "alice" @@ -733,6 +734,41 @@ async def test_websocket_connection_closed(init_gateway, jp_serverapp, jp_fetch, pytest.fail(f"Logs contain an error: {message}") +@patch("tornado.websocket.websocket_connect", mock_websocket_connect()) +async def test_websocket_connection_with_session_id(init_gateway, jp_serverapp, jp_fetch, caplog): + # Create the session and kernel and get the kernel manager... + kernel_id = await create_kernel(jp_fetch, "kspec_foo") + km: GatewayKernelManager = jp_serverapp.kernel_manager.get_kernel(kernel_id) + + # Create the KernelWebsocketHandler... + request = HTTPServerRequest("foo", "GET") + request.connection = MagicMock() + handler = KernelWebsocketHandler(jp_serverapp.web_app, request) + # Create the GatewayWebSocketConnection and attach it to the handler... + with mocked_gateway: + conn = GatewayWebSocketConnection(parent=km, websocket_handler=handler) + handler.connection = conn + await conn.connect() + assert conn.session_id != None + expected_ws_url = ( + f"{mock_gateway_ws_url}/api/kernels/{kernel_id}/channels?session_id={conn.session_id}" + ) + assert ( + expected_ws_url in caplog.text + ), "WebSocket URL does not contain the expected session_id." + + # Processing websocket messages happens in separate coroutines and any + # errors in that process will show up in logs, but not bubble up to the + # caller. + # + # To check for these, we wait for the server to stop and then check the + # logs for errors. + await jp_serverapp._cleanup() + for _, level, message in caplog.record_tuples: + if level >= logging.ERROR: + pytest.fail(f"Logs contain an error: {message}") + + # # Test methods below... # diff --git a/tests/test_utils.py b/tests/test_utils.py index fdbfd60587..90f9dcd340 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -160,7 +160,7 @@ def test_filefind(tmp_path, filename, result): if isinstance(result, str): found = filefind(filename, [str(a), str(b)]) found_relative = Path(found).relative_to(tmp_path) - assert str(found_relative) == result + assert str(found_relative).replace(os.sep, "/") == result else: with pytest.raises(result): filefind(filename, [str(a), str(b)])