Skip to content

Commit

Permalink
Add support for renaming kernelspecs on the fly.
Browse files Browse the repository at this point in the history
This change adds support for kernelspec managers to be configured with
renaming patterns that they will apply to the kernelspecs they serve.

That, in turn, will be used a future change to allow multiple kernel
spec managers to be used simultaneously without their kernel spec names
colliding.

This functionality is provided using a mixin that can be inherited by a
subclass of KernelSpecManager in order to add this renaming support.

Additionally, we provide canned subclasses of both KernelSpecManager
and GatewayKernelSpecManager with this new renaming feature built into
them.

To use the new renaming feature with a remote Kernel Gateway, the user
would add the following snippet to their Jupyter config:

```
c.ServerApp.kernel_spec_manager_class = 'jupyter_server.gateway.managers.GatewayRenamingKernelSpecManager'
```

... meanwhile, an example of using the renaming functionality with local
kernelspecs, can be achieved by the user adding the following snippet to
their config:

```
c.ServerApp.kernel_spec_manager_class = 'jupyter_server.services.kernelspecs.renaming.RenamingKernelSpecManager'
```

This change also fixes a pre-existing issue with the GatewayMappingKernelManager class
where the `default_kernel_name` value was not set until *after* the first request to
`/api/kernelspecs`, resulting in the first such call getting the wrong default
kernel name (even though subsequent calls would have gotten the correct one).

I confirmed that this issue with the default kernel name was already present
in the codebase on the main branch before this commit.
  • Loading branch information
ojarjur committed May 5, 2023
1 parent 3ba9ac9 commit 3f2424c
Show file tree
Hide file tree
Showing 8 changed files with 589 additions and 186 deletions.
6 changes: 6 additions & 0 deletions docs/source/api/jupyter_server.services.kernelspecs.rst
Expand Up @@ -10,6 +10,12 @@ Submodules
:undoc-members:
:show-inheritance:


.. automodule:: jupyter_server.services.kernelspecs.renaming
:members:
:undoc-members:
:show-inheritance:

Module contents
---------------

Expand Down
57 changes: 49 additions & 8 deletions jupyter_server/gateway/managers.py
Expand Up @@ -16,17 +16,18 @@
from jupyter_client.clientabc import KernelClientABC
from jupyter_client.kernelspec import KernelSpecManager
from jupyter_client.managerabc import KernelManagerABC
from jupyter_core.utils import ensure_async
from jupyter_core.utils import ensure_async, run_sync
from tornado import web
from tornado.escape import json_decode, json_encode, url_escape, utf8
from traitlets import DottedObjectName, Instance, Type, default
from traitlets import DottedObjectName, Instance, Type, Unicode, default, observe

from .._tz import UTC, utcnow
from ..services.kernels.kernelmanager import (
AsyncMappingKernelManager,
ServerKernelManager,
emit_kernel_action_event,
)
from ..services.kernelspecs.renaming import RenamingKernelSpecManagerMixin, normalize_kernel_name
from ..services.sessions.sessionmanager import SessionManager
from ..utils import url_path_join
from .gateway_client import GatewayClient, gateway_request
Expand Down Expand Up @@ -60,7 +61,8 @@ def remove_kernel(self, kernel_id):
except KeyError:
pass

async def start_kernel(self, *, kernel_id=None, path=None, **kwargs):
@normalize_kernel_name
async def start_kernel(self, *, kernel_id=None, path=None, renamed_kernel=None, **kwargs):
"""Start a kernel for a session and return its kernel_id.
Parameters
Expand All @@ -80,6 +82,10 @@ async def start_kernel(self, *, kernel_id=None, path=None, **kwargs):

km = self.kernel_manager_factory(parent=self, log=self.log)
await km.start_kernel(kernel_id=kernel_id, **kwargs)
if renamed_kernel is not None:
km.kernel_name = renamed_kernel
if km.kernel:
km.kernel["name"] = km.kernel_name
kernel_id = km.kernel_id
self._kernels[kernel_id] = km
# Initialize culling if not already
Expand Down Expand Up @@ -210,6 +216,27 @@ async def cull_kernels(self):
class GatewayKernelSpecManager(KernelSpecManager):
"""A gateway kernel spec manager."""

default_kernel_name = Unicode(allow_none=True)

# Use a hidden trait for the default kernel name we get from the remote.
#
# This is automatically copied to the corresponding public trait.
#
# We use two layers of trait so that sub classes can modify the public
# trait without confusing the logic that tracks changes to the remote
# default kernel name.
_remote_default_kernel_name = Unicode(allow_none=True)

@default("default_kernel_name")
def _default_default_kernel_name(self):
# The default kernel name is taken from the remote gateway
run_sync(self.get_all_specs)()
return self._remote_default_kernel_name

@observe("_remote_default_kernel_name")
def _observe_remote_default_kernel_name(self, change):
self.default_kernel_name = change.new

def __init__(self, **kwargs):
"""Initialize a gateway kernel spec manager."""
super().__init__(**kwargs)
Expand Down Expand Up @@ -273,14 +300,13 @@ async def get_all_specs(self):
# If different log a warning and reset the default. However, the
# caller of this method will still return this server's value until
# the next fetch of kernelspecs - at which time they'll match.
km = self.parent.kernel_manager
remote_default_kernel_name = fetched_kspecs.get("default")
if remote_default_kernel_name != km.default_kernel_name:
if remote_default_kernel_name != self._remote_default_kernel_name:
self.log.info(
f"Default kernel name on Gateway server ({remote_default_kernel_name}) differs from "
f"Notebook server ({km.default_kernel_name}). Updating to Gateway server's value."
f"Notebook server ({self._remote_default_kernel_name}). Updating to Gateway server's value."
)
km.default_kernel_name = remote_default_kernel_name
self._remote_default_kernel_name = remote_default_kernel_name

remote_kspecs = fetched_kspecs.get("kernelspecs")
return remote_kspecs
Expand Down Expand Up @@ -345,6 +371,18 @@ async def get_kernel_spec_resource(self, kernel_name, path):
return kernel_spec_resource


class GatewayRenamingKernelSpecManager(RenamingKernelSpecManagerMixin, GatewayKernelSpecManager):
spec_name_prefix = Unicode(
"remote-", help="Prefix to be added onto the front of kernel spec names."
)

display_name_suffix = Unicode(
" (Remote)",
config=True,
help="Suffix to be added onto the end of kernel spec display names.",
)


class GatewaySessionManager(SessionManager):
"""A gateway session manager."""

Expand Down Expand Up @@ -453,6 +491,8 @@ async def refresh_model(self, model=None):
# a parent instance if, say, a server extension is using another application
# (e.g., papermill) that uses a KernelManager instance directly.
self.parent._kernel_connections[self.kernel_id] = int(model["connections"])
if self.kernel_name:
model["name"] = self.kernel_name

self.kernel = model
return model
Expand All @@ -477,7 +517,8 @@ async def start_kernel(self, **kwargs):

if kernel_id is None:
kernel_name = kwargs.get("kernel_name", "python3")
self.log.debug("Request new kernel at: %s" % self.kernels_url)
self.kernel_name = kernel_name
self.log.debug(f"Request new kernel at: {self.kernels_url} using {kernel_name}")

# Let KERNEL_USERNAME take precedent over http_user config option.
if os.environ.get("KERNEL_USERNAME") is None and GatewayClient.instance().http_user:
Expand Down
63 changes: 59 additions & 4 deletions jupyter_server/services/kernels/kernelmanager.py
Expand Up @@ -17,6 +17,7 @@
from typing import Optional

from jupyter_client.ioloop.manager import AsyncIOLoopKernelManager
from jupyter_client.kernelspec import NATIVE_KERNEL_NAME
from jupyter_client.multikernelmanager import AsyncMultiKernelManager, MultiKernelManager
from jupyter_client.session import Session
from jupyter_core.paths import exists
Expand All @@ -38,6 +39,7 @@
TraitError,
Unicode,
default,
observe,
validate,
)

Expand All @@ -46,6 +48,8 @@
from jupyter_server.prometheus.metrics import KERNEL_CURRENTLY_RUNNING_TOTAL
from jupyter_server.utils import ApiPath, import_item, to_os_path

from ..kernelspecs.renaming import normalize_kernel_name


class MappingKernelManager(MultiKernelManager):
"""A KernelManager that handles
Expand Down Expand Up @@ -206,8 +210,14 @@ async def _remove_kernel_when_ready(self, kernel_id, kernel_awaitable):

# 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
@normalize_kernel_name
async def _async_start_kernel(
self,
*,
kernel_id: Optional[str] = None,
path: Optional[ApiPath] = None,
renamed_kernel: Optional[str] = None,
**kwargs: str,
) -> str:
"""Start a kernel for a session and return its kernel_id.
Expand All @@ -231,6 +241,8 @@ async def _async_start_kernel( # type:ignore[override]
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)
if renamed_kernel:
self._kernels[kernel_id].kernel_name = renamed_kernel
self._kernel_connections[kernel_id] = 0
task = asyncio.create_task(self._finish_kernel_start(kernel_id))
if not getattr(self, "use_pending_kernels", None):
Expand Down Expand Up @@ -261,7 +273,7 @@ async def _async_start_kernel( # type:ignore[override]
# 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 # type:ignore[assignment]
start_kernel = _async_start_kernel

async def _finish_kernel_start(self, kernel_id):
"""Handle a kernel that finishes starting."""
Expand Down Expand Up @@ -678,7 +690,7 @@ async def cull_kernel_if_idle(self, kernel_id):

# AsyncMappingKernelManager inherits as much as possible from MappingKernelManager,
# overriding only what is different.
class AsyncMappingKernelManager(MappingKernelManager, AsyncMultiKernelManager): # type:ignore[misc]
class AsyncMappingKernelManager(MappingKernelManager, AsyncMultiKernelManager):
"""An asynchronous mapping kernel manager."""

@default("kernel_manager_class")
Expand All @@ -700,13 +712,56 @@ def _validate_kernel_manager_class(self, proposal):
)
return km_class_value

@default("default_kernel_name")
def _default_default_kernel_name(self):
if (
hasattr(self.kernel_spec_manager, "default_kernel_name")
and self.kernel_spec_manager.default_kernel_name
):
return self.kernel_spec_manager.default_kernel_name
return NATIVE_KERNEL_NAME

@observe("default_kernel_name")
def _observe_default_kernel_name(self, change):
if (
hasattr(self.kernel_spec_manager, "default_kernel_name")
and self.kernel_spec_manager.default_kernel_name
):
# If the kernel spec manager defines a default kernel name, treat that
# one as authoritative.
kernel_name = change.new
if kernel_name == self.kernel_spec_manager.default_kernel_name:
return
self.log.debug(
f"The MultiKernelManager default kernel name '{kernel_name}'"
" differs from the KernelSpecManager default kernel name"
f" '{self.kernel_spec_manager.default_kernel_name}'..."
" Using the kernel spec manager's default name."
)
self.default_kernel_name = self.kernel_spec_manager.default_kernel_name

def _on_kernel_spec_manager_default_kernel_name_changed(self, change):
# Sync the kernel-spec-manager's trait to the multi-kernel-manager's trait.
kernel_name = change.new
if kernel_name is None:
return
self.log.debug(f"KernelSpecManager default kernel name changed: {kernel_name}")
self.default_kernel_name = kernel_name

def __init__(self, **kwargs):
"""Initialize an async mapping kernel manager."""
self.pinned_superclass = MultiKernelManager
self._pending_kernel_tasks = {}
self.pinned_superclass.__init__(self, **kwargs)
self.last_kernel_activity = utcnow()

if hasattr(self.kernel_spec_manager, "default_kernel_name"):
self.kernel_spec_manager.observe(
self._on_kernel_spec_manager_default_kernel_name_changed, "default_kernel_name"
)
if not self.kernel_spec_manager.default_kernel_name:
self.kernel_spec_manager.default_kernel_name = self.default_kernel_name


def emit_kernel_action_event(success_msg: str = ""): # type: ignore
"""Decorate kernel action methods to
Expand Down
2 changes: 1 addition & 1 deletion jupyter_server/services/kernelspecs/handlers.py
Expand Up @@ -64,10 +64,10 @@ async def get(self):
"""Get the list of kernel specs."""
ksm = self.kernel_spec_manager
km = self.kernel_manager
kspecs = await ensure_async(ksm.get_all_specs())
model = {}
model["default"] = km.default_kernel_name
model["kernelspecs"] = specs = {}
kspecs = await ensure_async(ksm.get_all_specs())
for kernel_name, kernel_info in kspecs.items():
try:
if is_kernelspec_model(kernel_info):
Expand Down

0 comments on commit 3f2424c

Please sign in to comment.