diff --git a/jupyter_server/base/handlers.py b/jupyter_server/base/handlers.py index 9515cdef2a..990d49363a 100644 --- a/jupyter_server/base/handlers.py +++ b/jupyter_server/base/handlers.py @@ -735,7 +735,7 @@ def write_error(self, status_code, **kwargs): reply["message"] = "Unhandled error" reply["reason"] = None reply["traceback"] = "".join(traceback.format_exception(*exc_info)) - self.log.warning("wrote error: %r", reply["message"]) + self.log.warning("wrote error: %r", reply["message"], exc_info=True) self.finish(json.dumps(reply)) def get_login_url(self): diff --git a/jupyter_server/gateway/managers.py b/jupyter_server/gateway/managers.py index 7943080a27..a186cd7a4a 100644 --- a/jupyter_server/gateway/managers.py +++ b/jupyter_server/gateway/managers.py @@ -57,7 +57,7 @@ def remove_kernel(self, kernel_id): except KeyError: pass - async def start_kernel(self, kernel_id=None, path=None, **kwargs): + async def start_kernel(self, *, kernel_id=None, path=None, **kwargs): """Start a kernel for a session and return its kernel_id. Parameters @@ -323,7 +323,7 @@ class GatewaySessionManager(SessionManager): kernel_manager = Instance("jupyter_server.gateway.managers.GatewayMappingKernelManager") - async def kernel_culled(self, kernel_id: str) -> bool: + async def kernel_culled(self, kernel_id: str) -> bool: # typing: ignore """Checks if the kernel is still considered alive and returns true if it's not found.""" km: Optional[GatewayKernelManager] = None try: diff --git a/jupyter_server/services/contents/fileio.py b/jupyter_server/services/contents/fileio.py index 66c28a1f64..b4f05c9bea 100644 --- a/jupyter_server/services/contents/fileio.py +++ b/jupyter_server/services/contents/fileio.py @@ -16,7 +16,7 @@ from traitlets import Bool from traitlets.config import Configurable -from jupyter_server.utils import to_api_path, to_os_path +from jupyter_server.utils import ApiPath, to_api_path, to_os_path def replace_file(src, dst): @@ -257,7 +257,7 @@ def _get_os_path(self, path): # to_os_path is not safe if path starts with a drive, since os.path.join discards first part if os.path.splitdrive(path)[0]: raise HTTPError(404, "%s is not a relative API path" % path) - os_path = to_os_path(path, root) + os_path = to_os_path(ApiPath(path), root) if not (os.path.abspath(os_path) + os.path.sep).startswith(root): raise HTTPError(404, "%s is outside root contents directory" % path) return os_path diff --git a/jupyter_server/services/kernels/kernelmanager.py b/jupyter_server/services/kernels/kernelmanager.py index 9806521569..9748643e73 100644 --- a/jupyter_server/services/kernels/kernelmanager.py +++ b/jupyter_server/services/kernels/kernelmanager.py @@ -7,10 +7,13 @@ # Distributed under the terms of the Modified BSD License. import asyncio import os +import typing as t import warnings from collections import defaultdict from datetime import datetime, timedelta from functools import partial +from typing import Dict as DictType +from typing import Optional from jupyter_client.ioloop.manager import AsyncIOLoopKernelManager from jupyter_client.multikernelmanager import AsyncMultiKernelManager, MultiKernelManager @@ -36,7 +39,7 @@ from jupyter_server._tz import isoformat, utcnow from jupyter_server.prometheus.metrics import KERNEL_CURRENTLY_RUNNING_TOTAL -from jupyter_server.utils import import_item, to_os_path +from jupyter_server.utils import ApiPath, import_item, to_os_path class MappingKernelManager(MultiKernelManager): @@ -56,7 +59,7 @@ def _default_kernel_manager_class(self): _kernel_connections = Dict() - _kernel_ports = Dict() + _kernel_ports: DictType[str, t.List[int]] = Dict() # type: ignore _culler_callback = None @@ -196,12 +199,16 @@ async def _remove_kernel_when_ready(self, kernel_id, kernel_awaitable): self._kernel_connections.pop(kernel_id, None) self._kernel_ports.pop(kernel_id, None) - async def _async_start_kernel(self, kernel_id=None, path=None, **kwargs): + # TODO DEC 2022: Revise the type-ignore once the signatures have been changed upstream + # https://github.com/jupyter/jupyter_client/pull/905 + async def _async_start_kernel( # type:ignore[override] + self, *, kernel_id: Optional[str] = None, path: Optional[ApiPath] = None, **kwargs: str + ) -> str: """Start a kernel for a session and return its kernel_id. Parameters ---------- - kernel_id : uuid + kernel_id : uuid (str) The uuid to associate the new kernel with. If this is not None, this kernel will be persistent whenever it is requested. @@ -216,6 +223,7 @@ async def _async_start_kernel(self, kernel_id=None, path=None, **kwargs): if path is not None: kwargs["cwd"] = self.cwd_for_path(path, env=kwargs.get("env", {})) if kernel_id is not None: + assert kernel_id is not None, "Never Fail, but necessary for mypy " kwargs["kernel_id"] = kernel_id kernel_id = await self.pinned_superclass._async_start_kernel(self, **kwargs) self._kernel_connections[kernel_id] = 0 @@ -242,9 +250,12 @@ async def _async_start_kernel(self, kernel_id=None, path=None, **kwargs): # Initialize culling if not already if not self._initialized_culler: self.initialize_culler() - + assert kernel_id is not None return kernel_id + # see https://github.com/jupyter-server/jupyter_server/issues/1165 + # this assignment is technically incorrect, but might need a change of API + # in jupyter_client. start_kernel = _async_start_kernel async def _finish_kernel_start(self, kernel_id): @@ -299,6 +310,8 @@ def _get_changed_ports(self, kernel_id): """ # Get current ports and return comparison with ports captured at startup. km = self.get_kernel(kernel_id) + assert isinstance(km.ports, list) + assert isinstance(self._kernel_ports[kernel_id], list) if km.ports != self._kernel_ports[kernel_id]: return km.ports return None diff --git a/jupyter_server/services/sessions/handlers.py b/jupyter_server/services/sessions/handlers.py index 49572f93d0..1b042b152d 100644 --- a/jupyter_server/services/sessions/handlers.py +++ b/jupyter_server/services/sessions/handlers.py @@ -52,12 +52,17 @@ async def post(self): if model is None: raise web.HTTPError(400, "No JSON data provided") - if "notebook" in model and "path" in model["notebook"]: + if "notebook" in model: self.log.warning("Sessions API changed, see updated swagger docs") - model["path"] = model["notebook"]["path"] model["type"] = "notebook" + if "name" in model["notebook"]: + model["path"] = model["notebook"]["name"] + elif "path" in model["notebook"]: + model["path"] = model["notebook"]["path"] try: + # There is a high chance here that `path` is not a path but + # a unique session id path = model["path"] except KeyError as e: raise web.HTTPError(400, "Missing field in JSON data: path") from e diff --git a/jupyter_server/services/sessions/sessionmanager.py b/jupyter_server/services/sessions/sessionmanager.py index fa64646364..2b64434a89 100644 --- a/jupyter_server/services/sessions/sessionmanager.py +++ b/jupyter_server/services/sessions/sessionmanager.py @@ -1,9 +1,14 @@ """A base class session manager.""" + # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. import os import pathlib import uuid +from typing import Any, Dict, List, NewType, Optional, Union + +KernelName = NewType("KernelName", str) +ModelName = NewType("ModelName", str) try: import sqlite3 @@ -12,7 +17,6 @@ from pysqlite2 import dbapi2 as sqlite3 # type:ignore[no-redef] from dataclasses import dataclass, fields -from typing import Union from jupyter_core.utils import ensure_async from tornado import web @@ -39,8 +43,8 @@ class KernelSessionRecord: associated with them. """ - session_id: Union[None, str] = None - kernel_id: Union[None, str] = None + session_id: Optional[str] = None + kernel_id: Optional[str] = None def __eq__(self, other: object) -> bool: """Whether a record equals another.""" @@ -98,7 +102,9 @@ class KernelSessionRecordList: it will be appended. """ - def __init__(self, *records): + _records: List[KernelSessionRecord] + + def __init__(self, *records: KernelSessionRecord): """Initialize a record list.""" self._records = [] for record in records: @@ -252,14 +258,26 @@ async def session_exists(self, path): exists = True return exists - def new_session_id(self): + def new_session_id(self) -> str: """Create a uuid for a new session""" return str(uuid.uuid4()) async def create_session( - self, path=None, name=None, type=None, kernel_name=None, kernel_id=None - ): - """Creates a session and returns its model""" + self, + path: Optional[str] = None, + name: Optional[ModelName] = None, + type: Optional[str] = None, + kernel_name: Optional[KernelName] = None, + kernel_id: Optional[str] = None, + ) -> Dict[str, Any]: + """Creates a session and returns its model + + name: ModelName(str) + Usually the model name, like the filename associated with current + kernel. + + + """ session_id = self.new_session_id() record = KernelSessionRecord(session_id=session_id) self._pending_sessions.update(record) @@ -277,15 +295,52 @@ async def create_session( self._pending_sessions.remove(record) return result - def get_kernel_env(self, path): - """Return the environment variables that need to be set in the kernel""" + def get_kernel_env( + self, path: Optional[str], name: Optional[ModelName] = None + ) -> Dict[str, str]: + """Return the environment variables that need to be set in the kernel + + path : str + the url path for the given session. + name: ModelName(str), optional + Here the name is likely to be the name of the associated file + with the current kernel at startup time. + + + """ + if name is not None: + cwd = self.kernel_manager.cwd_for_path(path) + path = os.path.join(cwd, name) + assert isinstance(path, str) return {**os.environ, "JPY_SESSION_NAME": path} - async def start_kernel_for_session(self, session_id, path, name, type, kernel_name): - """Start a new kernel for a given session.""" + async def start_kernel_for_session( + self, + session_id: str, + path: Optional[str], + name: Optional[ModelName], + type: Optional[str], + kernel_name: Optional[KernelName], + ) -> str: + """Start a new kernel for a given session. + + session_id : str + uuid for the session; this method must be given a session_id + path : str + the path for the given session - seem to be a session id sometime. + name : str + Usually the model name, like the filename associated with current + kernel. + type : str + the type of the session + kernel_name : str + the name of the kernel specification to use. The default kernel name will be used if not provided. + + """ # allow contents manager to specify kernels cwd kernel_path = await ensure_async(self.contents_manager.get_kernel_path(path=path)) - kernel_env = self.get_kernel_env(path) + + kernel_env = self.get_kernel_env(path, name) kernel_id = await self.kernel_manager.start_kernel( path=kernel_path, kernel_name=kernel_name, @@ -306,9 +361,9 @@ async def save_session(self, session_id, path=None, name=None, type=None, kernel uuid for the session; this method must be given a session_id path : str the path for the given session - name: str + name : str the name of the session - type: string + type : str the type of the session kernel_id : str a uuid for the kernel associated with this session @@ -405,13 +460,13 @@ async def update_session(self, session_id, **kwargs): query = "UPDATE session SET %s WHERE session_id=?" % (", ".join(sets)) self.cursor.execute(query, list(kwargs.values()) + [session_id]) - def kernel_culled(self, kernel_id): + async def kernel_culled(self, kernel_id: str) -> bool: """Checks if the kernel is still considered alive and returns true if its not found.""" return kernel_id not in self.kernel_manager async def row_to_model(self, row, tolerate_culled=False): """Takes sqlite database session row and turns it into a dictionary""" - kernel_culled = await ensure_async(self.kernel_culled(row["kernel_id"])) + kernel_culled: bool = await ensure_async(self.kernel_culled(row["kernel_id"])) if kernel_culled: # The kernel was culled or died without deleting the session. # We can't use delete_session here because that tries to find diff --git a/jupyter_server/utils.py b/jupyter_server/utils.py index b378bd7e93..7e1ba2f2af 100644 --- a/jupyter_server/utils.py +++ b/jupyter_server/utils.py @@ -8,6 +8,7 @@ import sys import warnings from contextlib import contextmanager +from typing import NewType from urllib.parse import ( SplitResult, quote, @@ -17,7 +18,7 @@ urlsplit, urlunsplit, ) -from urllib.request import pathname2url # noqa +from urllib.request import pathname2url # noqa: F401 from _frozen_importlib_external import _NamespacePath from jupyter_core.utils import ensure_async @@ -25,6 +26,8 @@ from tornado.httpclient import AsyncHTTPClient, HTTPClient, HTTPRequest from tornado.netutil import Resolver +ApiPath = NewType("ApiPath", str) + def url_path_join(*pieces): """Join components of url into a relative url @@ -109,19 +112,19 @@ def samefile_simple(path, other_path): return path.lower() == other_path.lower() and path_stat == other_path_stat -def to_os_path(path, root=""): +def to_os_path(path: ApiPath, root: str = "") -> str: """Convert an API path to a filesystem path If given, root will be prepended to the path. root must be a filesystem path already. """ - parts = path.strip("/").split("/") + parts = str(path).strip("/").split("/") parts = [p for p in parts if p != ""] # remove duplicate splits - path = os.path.join(root, *parts) - return os.path.normpath(path) + path_ = os.path.join(root, *parts) + return os.path.normpath(path_) -def to_api_path(os_path, root=""): +def to_api_path(os_path: str, root: str = "") -> ApiPath: """Convert a filesystem path to an API path If given, root will be removed from the path. @@ -132,7 +135,7 @@ def to_api_path(os_path, root=""): parts = os_path.strip(os.path.sep).split(os.path.sep) parts = [p for p in parts if p != ""] # remove duplicate splits path = "/".join(parts) - return path + return ApiPath(path) def check_version(v, check): diff --git a/tests/services/sessions/test_manager.py b/tests/services/sessions/test_manager.py index 48d5761746..e7f919a98a 100644 --- a/tests/services/sessions/test_manager.py +++ b/tests/services/sessions/test_manager.py @@ -8,6 +8,7 @@ from jupyter_server.services.contents.manager import ContentsManager from jupyter_server.services.kernels.kernelmanager import MappingKernelManager from jupyter_server.services.sessions.sessionmanager import ( + KernelName, KernelSessionRecord, KernelSessionRecordConflict, KernelSessionRecordList, @@ -37,7 +38,7 @@ def __init__(self, *args, **kwargs): def _new_id(self): return next(self.id_letters) - async def start_kernel(self, kernel_id=None, path=None, kernel_name="python", **kwargs): + async def start_kernel(self, *, kernel_id=None, path=None, kernel_name="python", **kwargs): kernel_id = kernel_id or self._new_id() k = self._kernels[kernel_id] = DummyKernel(kernel_name=kernel_name) self._kernel_connections[kernel_id] = 0 @@ -50,7 +51,7 @@ async def shutdown_kernel(self, kernel_id, now=False): class SlowStartingKernelsMKM(MockMKM): - async def start_kernel(self, kernel_id=None, path=None, kernel_name="python", **kwargs): + async def start_kernel(self, *, kernel_id=None, path=None, kernel_name="python", **kwargs): await asyncio.sleep(1.0) return await super().start_kernel( kernel_id=kernel_id, path=path, kernel_name=kernel_name, **kwargs @@ -419,7 +420,7 @@ async def test_good_database_filepath(jp_runtime_dir): ) await session_manager.create_session( - path="/path/to/test.ipynb", kernel_name="python", type="notebook" + path="/path/to/test.ipynb", kernel_name=KernelName("python"), type="notebook" ) # Assert that the database file exists assert empty_file.exists() @@ -451,7 +452,7 @@ async def test_session_persistence(jp_runtime_dir): ) session = await session_manager.create_session( - path="/path/to/test.ipynb", kernel_name="python", type="notebook" + path="/path/to/test.ipynb", kernel_name=KernelName("python"), type="notebook" ) # Assert that the database file exists @@ -482,7 +483,7 @@ async def test_pending_kernel(): ) # Create a session with a slow starting kernel fut = session_manager.create_session( - path="/path/to/test.ipynb", kernel_name="python", type="notebook" + path="/path/to/test.ipynb", kernel_name=KernelName("python"), type="notebook" ) task = asyncio.create_task(fut) await asyncio.sleep(0.1) @@ -506,10 +507,10 @@ async def test_pending_kernel(): # Test multiple, parallel pending kernels fut1 = session_manager.create_session( - path="/path/to/test.ipynb", kernel_name="python", type="notebook" + path="/path/to/test.ipynb", kernel_name=KernelName("python"), type="notebook" ) fut2 = session_manager.create_session( - path="/path/to/test.ipynb", kernel_name="python", type="notebook" + path="/path/to/test.ipynb", kernel_name=KernelName("python"), type="notebook" ) task1 = asyncio.create_task(fut1) await asyncio.sleep(0.1) diff --git a/tests/utils.py b/tests/utils.py index 4eabcdceaa..0ca9c008f2 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,4 +1,5 @@ import json +from typing import NewType from tornado.httpclient import HTTPClientError from tornado.web import HTTPError @@ -10,6 +11,8 @@ "display_name": "Test kernel", } +ApiPath = NewType("ApiPath", str) + def mkdir(tmp_path, *parts): path = tmp_path.joinpath(*parts)