From d4643abac2ef081961aa2b4cf13e6342a7ff4952 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Mon, 24 Apr 2023 11:56:47 -0700 Subject: [PATCH 01/16] Add caching to kernelspec management (using watchdog) --- jupyter_server/base/handlers.py | 4 + jupyter_server/kernelspecs/handlers.py | 4 +- jupyter_server/serverapp.py | 21 ++ .../services/kernelspecs/handlers.py | 8 +- .../services/kernelspecs/kernelspec_cache.py | 302 ++++++++++++++++++ pyproject.toml | 1 + .../kernelspecs/test_kernelspec_cache.py | 195 +++++++++++ 7 files changed, 529 insertions(+), 6 deletions(-) create mode 100644 jupyter_server/services/kernelspecs/kernelspec_cache.py create mode 100644 tests/services/kernelspecs/test_kernelspec_cache.py diff --git a/jupyter_server/base/handlers.py b/jupyter_server/base/handlers.py index 061eea672a..96f37d3281 100644 --- a/jupyter_server/base/handlers.py +++ b/jupyter_server/base/handlers.py @@ -342,6 +342,10 @@ def terminal_manager(self): def kernel_spec_manager(self): return self.settings["kernel_spec_manager"] + @property + def kernel_spec_cache(self): + return self.settings["kernel_spec_cache"] + @property def config_manager(self): return self.settings["config_manager"] diff --git a/jupyter_server/kernelspecs/handlers.py b/jupyter_server/kernelspecs/handlers.py index 611a6f3a9a..0b10928daa 100644 --- a/jupyter_server/kernelspecs/handlers.py +++ b/jupyter_server/kernelspecs/handlers.py @@ -24,11 +24,11 @@ def initialize(self): @authorized async def get(self, kernel_name, path, include_body=True): """Get a kernelspec resource.""" - ksm = self.kernel_spec_manager + ksc = self.kernel_spec_cache if path.lower().endswith(".png"): self.set_header("Cache-Control", f"max-age={60*60*24*30}") try: - kspec = await ensure_async(ksm.get_kernel_spec(kernel_name)) + kspec = await ensure_async(ksc.get_kernel_spec(kernel_name)) self.root = kspec.resource_dir except KeyError as e: raise web.HTTPError(404, "Kernel spec %s not found" % kernel_name) from e diff --git a/jupyter_server/serverapp.py b/jupyter_server/serverapp.py index 48c60b7498..b4241b9981 100644 --- a/jupyter_server/serverapp.py +++ b/jupyter_server/serverapp.py @@ -107,6 +107,7 @@ AsyncMappingKernelManager, MappingKernelManager, ) +from jupyter_server.services.kernelspecs.kernelspec_cache import KernelSpecCache from jupyter_server.services.sessions.sessionmanager import SessionManager from jupyter_server.utils import ( check_pid, @@ -234,6 +235,7 @@ def __init__( authorizer=None, identity_provider=None, kernel_websocket_connection_class=None, + kernel_spec_cache=None, ): """Initialize a server web application.""" if identity_provider is None: @@ -271,6 +273,7 @@ def __init__( authorizer=authorizer, identity_provider=identity_provider, kernel_websocket_connection_class=kernel_websocket_connection_class, + kernel_spec_cache=kernel_spec_cache, ) handlers = self.init_handlers(default_services, settings) @@ -295,6 +298,7 @@ def init_settings( authorizer=None, identity_provider=None, kernel_websocket_connection_class=None, + kernel_spec_cache=None, ): """Initialize settings for the web application.""" _template_path = settings_overrides.get( @@ -372,6 +376,7 @@ def init_settings( "contents_manager": contents_manager, "session_manager": session_manager, "kernel_spec_manager": kernel_spec_manager, + "kernel_spec_cache": kernel_spec_cache, "config_manager": config_manager, "authorizer": authorizer, "identity_provider": identity_provider, @@ -1504,6 +1509,16 @@ def _default_session_manager_class(self): return "jupyter_server.gateway.managers.GatewaySessionManager" return SessionManager + kernel_spec_cache_class = Type( + default_value=KernelSpecCache, + klass=KernelSpecCache, + config=True, + help=""" + The kernel spec cache class to use. Must be a subclass + of `jupyter_server.services.kernelspecs.kernelspec_cache.KernelSpecCache`. + """, + ) + kernel_websocket_connection_class = Type( default_value=ZMQChannelsWebsocketConnection, klass=BaseKernelWebsocketConnection, @@ -1886,6 +1901,11 @@ def init_configurables(self): kernel_manager=self.kernel_manager, contents_manager=self.contents_manager, ) + self.kernel_spec_cache = self.kernel_spec_cache_class( + parent=self, + log=self.log, + kernel_spec_manager=self.kernel_spec_manager, + ) self.config_manager = self.config_manager_class( parent=self, log=self.log, @@ -2044,6 +2064,7 @@ def init_webapp(self): authorizer=self.authorizer, identity_provider=self.identity_provider, kernel_websocket_connection_class=self.kernel_websocket_connection_class, + kernel_spec_cache=self.kernel_spec_cache, ) if self.certfile: self.ssl_options["certfile"] = self.certfile diff --git a/jupyter_server/services/kernelspecs/handlers.py b/jupyter_server/services/kernelspecs/handlers.py index e1ed186fa7..2a01846b55 100644 --- a/jupyter_server/services/kernelspecs/handlers.py +++ b/jupyter_server/services/kernelspecs/handlers.py @@ -62,12 +62,12 @@ class MainKernelSpecHandler(KernelSpecsAPIHandler): @authorized async def get(self): """Get the list of kernel specs.""" - ksm = self.kernel_spec_manager + ksc = self.kernel_spec_cache km = self.kernel_manager model = {} model["default"] = km.default_kernel_name model["kernelspecs"] = specs = {} - kspecs = await ensure_async(ksm.get_all_specs()) + kspecs = await ensure_async(ksc.get_all_specs()) for kernel_name, kernel_info in kspecs.items(): try: if is_kernelspec_model(kernel_info): @@ -94,10 +94,10 @@ class KernelSpecHandler(KernelSpecsAPIHandler): @authorized async def get(self, kernel_name): """Get a kernel spec model.""" - ksm = self.kernel_spec_manager + ksc = self.kernel_spec_cache kernel_name = url_unescape(kernel_name) try: - spec = await ensure_async(ksm.get_kernel_spec(kernel_name)) + spec = await ensure_async(ksc.get_kernel_spec(kernel_name)) except KeyError as e: raise web.HTTPError(404, "Kernel spec %s not found" % kernel_name) from e if is_kernelspec_model(spec): diff --git a/jupyter_server/services/kernelspecs/kernelspec_cache.py b/jupyter_server/services/kernelspecs/kernelspec_cache.py new file mode 100644 index 0000000000..d4e507aea1 --- /dev/null +++ b/jupyter_server/services/kernelspecs/kernelspec_cache.py @@ -0,0 +1,302 @@ +"""Cache handling for kernel specs.""" +# Copyright (c) Jupyter Development Team. +# Distributed under the terms of the Modified BSD License. + + +import os +from typing import Dict, Optional, Union + +from jupyter_client.kernelspec import KernelSpec +from traitlets.config import SingletonConfigurable +from traitlets.traitlets import CBool, default +from watchdog.events import FileMovedEvent, FileSystemEventHandler +from watchdog.observers import Observer + +from jupyter_server.utils import ensure_async + +# Simplify the typing. Cache items are essentially dictionaries of strings +# to either strings or dictionaries. The items themselves are indexed by +# the kernel_name (case-insensitive). +CacheItemType = Dict[str, Union[str, Dict]] + + +class KernelSpecCache(SingletonConfigurable): + """The primary (singleton) instance for managing KernelSpecs. + + This class contains the configured KernelSpecManager instance upon + which it uses to populate the cache (when enabled) or as a pass-thru + (when disabled). + + Note that the KernelSpecManager returns different formats from methods + get_all_specs() and get_kernel_spec(). The format in which cache entries + are stored is that of the get_all_specs() results. As a result, some + conversion between formats is necessary, depending on which method is called. + """ + + cache_enabled_env = "JUPYTER_KERNELSPEC_CACHE_ENABLED" + cache_enabled = CBool( + config=True, + help="""Enable Kernel Specification caching. (JUPYTER_KERNELSPEC_CACHE_ENABLED env var)""", + ) + + @default("cache_enabled") + def _cache_enabled_default(self): + return os.getenv(self.cache_enabled_env, "true").lower() in ("true", "1") + + def __init__(self, kernel_spec_manager, **kwargs) -> None: + """Initialize the cache.""" + super().__init__(**kwargs) + self.kernel_spec_manager = kernel_spec_manager + self._initialize() + + async def get_kernel_spec(self, kernel_name: str) -> KernelSpec: + """Get the named kernel specification. + + This method is equivalent to calling KernelSpecManager.get_kernel_spec(). If + caching is enabled, it will pull the item from the cache. If no item is + returned (as will be the case if caching is disabled) it will defer to the + currently configured KernelSpecManager. If an item is returned (and caching + is enabled), it will be added to the cache. + """ + kernelspec = self.get_item(kernel_name) + if not kernelspec: + kernelspec = await ensure_async(self.kernel_spec_manager.get_kernel_spec(kernel_name)) + if kernelspec: + self.put_item(kernel_name, kernelspec) + return kernelspec + + async def get_all_specs(self) -> Dict[str, CacheItemType]: + """Get all available kernel specifications. + + This method is equivalent to calling KernelSpecManager.get_all_specs(). If + caching is enabled, it will pull all items from the cache. If no items are + returned (as will be the case if caching is disabled) it will defer to the + currently configured KernelSpecManager. If items are returned (and caching + is enabled), they will be added to the cache. + + Note that the return type of this method is not a dictionary or list of + KernelSpec instances, but rather a dictionary of kernel-name to kernel-info + dictionaries are returned - as is the case with the respective return values + of the KernelSpecManager methods. + """ + kernelspecs = self.get_all_items() + if not kernelspecs: + kernelspecs = await ensure_async(self.kernel_spec_manager.get_all_specs()) + if kernelspecs: + self.put_all_items(kernelspecs) + return kernelspecs + + # Cache-related methods + def get_item(self, kernel_name: str) -> Optional[KernelSpec]: + """Retrieves a named kernel specification from the cache. + + If cache is disabled or the item is not in the cache, None is returned; + otherwise, a KernelSpec instance of the item is returned. + """ + kernelspec = None + if self.cache_enabled: + cache_item = self.cache_items.get(kernel_name.lower()) + if cache_item: # Convert to KernelSpec + # In certain conditions, like when the kernelspec is fetched prior to its removal from the cache, + # we can encounter a FileNotFoundError. In those cases, treat as a cache miss as well. + try: + kernelspec = KernelSpecCache.cache_item_to_kernel_spec(cache_item) + except FileNotFoundError: + pass + if not kernelspec: + self.cache_misses += 1 + self.log.debug( + "Cache miss ({misses}) for kernelspec: {kernel_name}".format( + misses=self.cache_misses, kernel_name=kernel_name + ) + ) + return kernelspec + + def get_all_items(self) -> Dict[str, CacheItemType]: + """Retrieves all kernel specification from the cache. + + If cache is disabled or no items are in the cache, an empty dictionary is returned; + otherwise, a dictionary of kernel-name to specifications (kernel infos) are returned. + """ + items = {} + if self.cache_enabled: + for kernel_name in self.cache_items: + cache_item = self.cache_items.get(kernel_name) + items[kernel_name] = cache_item + if not items: + self.cache_misses += 1 + return items + + def put_item(self, kernel_name: str, cache_item: Union[KernelSpec, CacheItemType]) -> None: + """Adds or updates a kernel specification in the cache. + + This method can take either a KernelSpec (if called directly from the `get_kernel_spec()` + method, or a CacheItemItem (if called from a cache-related method) as that is the type + in which the cache items are stored. + + If it determines the cache entry corresponds to a currently unwatched directory, + that directory will be added to list of observed directories and scheduled accordingly. + """ + if self.cache_enabled: + self.log.info(f"KernelSpecCache: adding/updating kernelspec: {kernel_name}") + if type(cache_item) is KernelSpec: + cache_item = KernelSpecCache.kernel_spec_to_cache_item(cache_item) + + resource_dir = cache_item["resource_dir"] + self.cache_items[kernel_name.lower()] = cache_item + observed_dir = os.path.dirname(resource_dir) + if observed_dir not in self.observed_dirs: + # New directory to watch, schedule it... + self.log.debug( + "KernelSpecCache: observing directory: {observed_dir}".format( + observed_dir=observed_dir + ) + ) + self.observed_dirs.add(observed_dir) + self.observer.schedule(KernelSpecChangeHandler(self), observed_dir, recursive=True) + + def put_all_items(self, kernelspecs: Dict[str, CacheItemType]) -> None: + """Adds or updates a dictionary of kernel specification in the cache.""" + for kernel_name, cache_item in kernelspecs.items(): + self.put_item(kernel_name, cache_item) + + def remove_item(self, kernel_name: str) -> Optional[CacheItemType]: + """Removes the cache item corresponding to kernel_name from the cache.""" + cache_item = None + if self.cache_enabled and kernel_name.lower() in self.cache_items: + cache_item = self.cache_items.pop(kernel_name.lower()) + self.log.info(f"KernelSpecCache: removed kernelspec: {kernel_name}") + return cache_item + + def _initialize(self): + """Initializes the cache and starts the observer.""" + + # The kernelspec cache consists of a dictionary mapping the kernel name to the actual + # kernelspec data (CacheItemType). + self.cache_items = {} # Maps kernel name to kernelspec + self.observed_dirs = set() # Tracks which directories are being watched + self.cache_misses = 0 + + # Seed the cache and start the observer + if self.cache_enabled: + self.observer = Observer() + kernelspecs = self.kernel_spec_manager.get_all_specs() + self.put_all_items(kernelspecs) + # Following adds, see if any of the manager's kernel dirs are not observed and add them + for kernel_dir in self.kernel_spec_manager.kernel_dirs: + if kernel_dir not in self.observed_dirs: + if os.path.exists(kernel_dir): + self.log.info( + "KernelSpecCache: observing directory: {kernel_dir}".format( + kernel_dir=kernel_dir + ) + ) + self.observed_dirs.add(kernel_dir) + self.observer.schedule( + KernelSpecChangeHandler(self), kernel_dir, recursive=True + ) + else: + self.log.warning( + "KernelSpecCache: kernel_dir '{kernel_dir}' does not exist" + " and will not be observed.".format(kernel_dir=kernel_dir) + ) + self.observer.start() + + @staticmethod + def kernel_spec_to_cache_item(kernelspec: KernelSpec) -> CacheItemType: + """Converts a KernelSpec instance to a CacheItemType for storage into the cache.""" + cache_item = {"spec": kernelspec.to_dict(), "resource_dir": kernelspec.resource_dir} + return cache_item + + @staticmethod + def cache_item_to_kernel_spec(cache_item: CacheItemType) -> KernelSpec: + """Converts a CacheItemType to a KernelSpec instance for user consumption.""" + kernel_spec = KernelSpec(resource_dir=cache_item["resource_dir"], **cache_item["spec"]) + return kernel_spec + + +class KernelSpecChangeHandler(FileSystemEventHandler): + """Watchdog handler that filters on specific files deemed representative of a kernel specification.""" + + # Events related to these files trigger the management of the KernelSpec cache. Should we find + # other files qualify as indicators of a kernel specification's state (like perhaps detached parameter + # files in the future) should be added to this list - at which time it should become configurable. + watched_files = ["kernel.json"] + + def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs): + """Initialize the handler.""" + super().__init__(**kwargs) + self.kernel_spec_cache = kernel_spec_cache + self.log = kernel_spec_cache.log + + def dispatch(self, event): + """Dispatches events pertaining to kernelspecs to the appropriate methods. + + + The primary purpose of this method is to ensure the action is occurring against + the a file in the list of watched files and adds some additional attributes to + the event instance to make the actual event handling method easier. + :param event: + The event object representing the file system event. + :type event: + :class:`FileSystemEvent` + """ + if os.path.basename(event.src_path) in self.watched_files: + src_resource_dir = os.path.dirname(event.src_path) + event.src_resource_dir = src_resource_dir + event.src_kernel_name = os.path.basename(src_resource_dir) + if type(event) is FileMovedEvent: + dest_resource_dir = os.path.dirname(event.dest_path) + event.dest_resource_dir = dest_resource_dir + event.dest_kernel_name = os.path.basename(dest_resource_dir) + + super().dispatch(event) + + def on_created(self, event): + """Fires when a watched file is created. + + This will trigger a call to the configured KernelSpecManager to fetch the instance + associated with the created file, which is then added to the cache. + """ + kernel_name = event.src_kernel_name + try: + kernelspec = self.kernel_spec_cache.kernel_spec_manager.get_kernel_spec(kernel_name) + self.kernel_spec_cache.put_item(kernel_name, kernelspec) + except Exception as e: + self.log.warning( + "The following exception occurred creating cache entry for: {src_resource_dir} " + "- continuing... ({e})".format(src_resource_dir=event.src_resource_dir, e=e) + ) + + def on_deleted(self, event): + """Fires when a watched file is deleted, triggering a removal of the corresponding item from the cache.""" + kernel_name = event.src_kernel_name + self.kernel_spec_cache.remove_item(kernel_name) + + def on_modified(self, event): + """Fires when a watched file is modified. + + This will trigger a call to the configured KernelSpecManager to fetch the instance + associated with the modified file, which is then replaced in the cache. + """ + kernel_name = event.src_kernel_name + try: + kernelspec = self.kernel_spec_cache.kernel_spec_manager.get_kernel_spec(kernel_name) + self.kernel_spec_cache.put_item(kernel_name, kernelspec) + except Exception as e: + self.log.warning( + "The following exception occurred updating cache entry for: {src_resource_dir} " + "- continuing... ({e})".format(src_resource_dir=event.src_resource_dir, e=e) + ) + + def on_moved(self, event): + """Fires when a watched file is moved. + + This will trigger the update of the existing cached item, replacing its resource_dir entry + with that of the new destination. + """ + src_kernel_name = event.src_kernel_name + dest_kernel_name = event.dest_kernel_name + cache_item = self.kernel_spec_cache.remove_item(src_kernel_name) + cache_item["resource_dir"] = event.dest_resource_dir + self.kernel_spec_cache.put_item(dest_kernel_name, cache_item) diff --git a/pyproject.toml b/pyproject.toml index 9b7371f831..b094fb7efc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,6 +44,7 @@ dependencies = [ "tornado>=6.2.0", "traitlets>=5.6.0", "websocket-client", + "watchdog", "jupyter_events>=0.6.0", "overrides" ] diff --git a/tests/services/kernelspecs/test_kernelspec_cache.py b/tests/services/kernelspecs/test_kernelspec_cache.py new file mode 100644 index 0000000000..740da16099 --- /dev/null +++ b/tests/services/kernelspecs/test_kernelspec_cache.py @@ -0,0 +1,195 @@ +# Copyright (c) Jupyter Development Team. +# Distributed under the terms of the Modified BSD License. +"""Tests for KernelSpecCache.""" + +import asyncio +import json +import os +import shutil +import sys + +import jupyter_core.paths +import pytest +from jupyter_client.kernelspec import KernelSpecManager, NoSuchKernel + +from jupyter_server.services.kernelspecs.kernelspec_cache import KernelSpecCache + + +# BEGIN - Remove once transition to jupyter_server occurs +def mkdir(tmp_path, *parts): + path = tmp_path.joinpath(*parts) + if not path.exists(): + path.mkdir(parents=True) + return path + + +home_dir = pytest.fixture(lambda tmp_path: mkdir(tmp_path, "home")) +data_dir = pytest.fixture(lambda tmp_path: mkdir(tmp_path, "data")) +config_dir = pytest.fixture(lambda tmp_path: mkdir(tmp_path, "config")) +runtime_dir = pytest.fixture(lambda tmp_path: mkdir(tmp_path, "runtime")) +system_jupyter_path = pytest.fixture(lambda tmp_path: mkdir(tmp_path, "share", "jupyter")) +env_jupyter_path = pytest.fixture(lambda tmp_path: mkdir(tmp_path, "env", "share", "jupyter")) +system_config_path = pytest.fixture(lambda tmp_path: mkdir(tmp_path, "etc", "jupyter")) +env_config_path = pytest.fixture(lambda tmp_path: mkdir(tmp_path, "env", "etc", "jupyter")) + + +@pytest.fixture +def environ( + monkeypatch, + tmp_path, + home_dir, + data_dir, + config_dir, + runtime_dir, + system_jupyter_path, + system_config_path, + env_jupyter_path, + env_config_path, +): + monkeypatch.setenv("HOME", str(home_dir)) + monkeypatch.setenv("PYTHONPATH", os.pathsep.join(sys.path)) + monkeypatch.setenv("JUPYTER_NO_CONFIG", "1") + monkeypatch.setenv("JUPYTER_CONFIG_DIR", str(config_dir)) + monkeypatch.setenv("JUPYTER_DATA_DIR", str(data_dir)) + monkeypatch.setenv("JUPYTER_RUNTIME_DIR", str(runtime_dir)) + monkeypatch.setattr(jupyter_core.paths, "SYSTEM_JUPYTER_PATH", [str(system_jupyter_path)]) + monkeypatch.setattr(jupyter_core.paths, "ENV_JUPYTER_PATH", [str(env_jupyter_path)]) + monkeypatch.setattr(jupyter_core.paths, "SYSTEM_CONFIG_PATH", [str(system_config_path)]) + monkeypatch.setattr(jupyter_core.paths, "ENV_CONFIG_PATH", [str(env_config_path)]) + + +# END - Remove once transition to jupyter_server occurs + + +kernelspec_json = { + "argv": ["cat", "{connection_file}"], + "display_name": "Test kernel: {kernel_name}", +} + + +def _install_kernelspec(kernels_dir, kernel_name): + """install a sample kernel in a kernels directory""" + kernelspec_dir = os.path.join(kernels_dir, kernel_name) + os.makedirs(kernelspec_dir) + json_file = os.path.join(kernelspec_dir, "kernel.json") + named_json = kernelspec_json.copy() + named_json["display_name"] = named_json["display_name"].format(kernel_name=kernel_name) + with open(json_file, "w") as f: + json.dump(named_json, f) + return kernelspec_dir + + +def _modify_kernelspec(kernelspec_dir, kernel_name): + json_file = os.path.join(kernelspec_dir, "kernel.json") + kernel_json = kernelspec_json.copy() + kernel_json["display_name"] = f"{kernel_name} modified!" + with open(json_file, "w") as f: + json.dump(kernel_json, f) + + +kernelspec_location = pytest.fixture(lambda data_dir: mkdir(data_dir, "kernels")) +other_kernelspec_location = pytest.fixture( + lambda env_jupyter_path: mkdir(env_jupyter_path, "kernels") +) + + +@pytest.fixture +def setup_kernelspecs(environ, kernelspec_location): + # Only populate factory info + _install_kernelspec(str(kernelspec_location), "test1") + _install_kernelspec(str(kernelspec_location), "test2") + _install_kernelspec(str(kernelspec_location), "test3") + + +@pytest.fixture +def kernel_spec_manager(environ, setup_kernelspecs): + yield KernelSpecManager(ensure_native_kernel=False) + + +@pytest.fixture +def kernel_spec_cache(is_enabled, kernel_spec_manager): + kspec_cache = KernelSpecCache.instance( + kernel_spec_manager=kernel_spec_manager, cache_enabled=is_enabled + ) + yield kspec_cache + kspec_cache = None + KernelSpecCache.clear_instance() + + +@pytest.fixture(params=[False, True]) # Add types as needed +def is_enabled(request): + return request.param + + +async def tests_get_all_specs(kernel_spec_cache): + kspecs = await kernel_spec_cache.get_all_specs() + assert len(kspecs) == 3 + + +async def tests_get_named_spec(kernel_spec_cache): + kspec = await kernel_spec_cache.get_kernel_spec("test2") + assert kspec.display_name == "Test kernel: test2" + + +async def tests_get_modified_spec(kernel_spec_cache): + kspec = await kernel_spec_cache.get_kernel_spec("test2") + assert kspec.display_name == "Test kernel: test2" + + # Modify entry + _modify_kernelspec(kspec.resource_dir, "test2") + await asyncio.sleep(0.5) # sleep for a half-second to allow cache to update item + kspec = await kernel_spec_cache.get_kernel_spec("test2") + assert kspec.display_name == "test2 modified!" + + +async def tests_add_spec(kernel_spec_cache, kernelspec_location, other_kernelspec_location): + assert len(kernel_spec_cache.observed_dirs) == (1 if kernel_spec_cache.cache_enabled else 0) + assert ( + str(kernelspec_location) in kernel_spec_cache.observed_dirs + if kernel_spec_cache.cache_enabled + else True + ) + + _install_kernelspec(str(other_kernelspec_location), "added") + kspec = await kernel_spec_cache.get_kernel_spec("added") + + # Ensure new location has been added to observed_dirs + assert len(kernel_spec_cache.observed_dirs) == (2 if kernel_spec_cache.cache_enabled else 0) + assert ( + str(other_kernelspec_location) in kernel_spec_cache.observed_dirs + if kernel_spec_cache.cache_enabled + else True + ) + + assert kspec.display_name == "Test kernel: added" + assert kernel_spec_cache.cache_misses == (1 if kernel_spec_cache.cache_enabled else 0) + + # Add another to an existing observed directory, no cache miss here + _install_kernelspec(str(kernelspec_location), "added2") + await asyncio.sleep( + 0.5 + ) # sleep for a half-second to allow cache to add item (no cache miss in this case) + kspec = await kernel_spec_cache.get_kernel_spec("added2") + + assert kspec.display_name == "Test kernel: added2" + assert kernel_spec_cache.cache_misses == (1 if kernel_spec_cache.cache_enabled else 0) + + +async def tests_remove_spec(kernel_spec_cache): + kspec = await kernel_spec_cache.get_kernel_spec("test2") + assert kspec.display_name == "Test kernel: test2" + + assert kernel_spec_cache.cache_misses == 0 + shutil.rmtree(kspec.resource_dir) + await asyncio.sleep(0.5) # sleep for a half-second to allow cache to remove item + with pytest.raises(NoSuchKernel): + await kernel_spec_cache.get_kernel_spec("test2") + + assert kernel_spec_cache.cache_misses == (1 if kernel_spec_cache.cache_enabled else 0) + + +async def tests_get_missing(kernel_spec_cache): + with pytest.raises(NoSuchKernel): + await kernel_spec_cache.get_kernel_spec("missing") + + assert kernel_spec_cache.cache_misses == (1 if kernel_spec_cache.cache_enabled else 0) From 337f7d1f72ce50cfbd4aa2e51692632379efc815 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Mon, 8 May 2023 15:46:43 -0700 Subject: [PATCH 02/16] Prepare for using monitor classes --- .../services/kernelspecs/kernelspec_cache.py | 286 ++++++++++-------- .../kernelspecs/test_kernelspec_cache.py | 24 +- 2 files changed, 169 insertions(+), 141 deletions(-) diff --git a/jupyter_server/services/kernelspecs/kernelspec_cache.py b/jupyter_server/services/kernelspecs/kernelspec_cache.py index d4e507aea1..8d9e02df28 100644 --- a/jupyter_server/services/kernelspecs/kernelspec_cache.py +++ b/jupyter_server/services/kernelspecs/kernelspec_cache.py @@ -4,13 +4,13 @@ import os +from abc import ABC, abstractmethod from typing import Dict, Optional, Union from jupyter_client.kernelspec import KernelSpec +from overrides import overrides from traitlets.config import SingletonConfigurable -from traitlets.traitlets import CBool, default -from watchdog.events import FileMovedEvent, FileSystemEventHandler -from watchdog.observers import Observer +from traitlets.traitlets import CBool, Instance, Type, default from jupyter_server.utils import ensure_async @@ -43,11 +43,27 @@ class KernelSpecCache(SingletonConfigurable): def _cache_enabled_default(self): return os.getenv(self.cache_enabled_env, "true").lower() in ("true", "1") + kernel_spec_manager = Instance("jupyter_client.kernelspec.KernelSpecManager") + + monitor_class = Type( + klass="jupyter_server.services.kernelspecs.kernelspec_cache.KernelSpecMonitorBase", + help="""The monitor class to use to capture kernelspecs.""", + ).tag(config=True) + + @default("monitor_class") + def _monitor_class_default(self): + return "jupyter_server.services.kernelspecs.kernelspec_cache.KernelSpecWatchdogMonitor" + + # The kernelspec cache consists of a dictionary mapping the kernel name to the actual + # kernelspec data (CacheItemType). + cache_items: Dict = {} + cache_misses: int = 0 + def __init__(self, kernel_spec_manager, **kwargs) -> None: """Initialize the cache.""" super().__init__(**kwargs) self.kernel_spec_manager = kernel_spec_manager - self._initialize() + self.kernel_spec_monitor = KernelSpecMonitorBase.create_instance(self) async def get_kernel_spec(self, kernel_name: str) -> KernelSpec: """Get the named kernel specification. @@ -133,27 +149,12 @@ def put_item(self, kernel_name: str, cache_item: Union[KernelSpec, CacheItemType This method can take either a KernelSpec (if called directly from the `get_kernel_spec()` method, or a CacheItemItem (if called from a cache-related method) as that is the type in which the cache items are stored. - - If it determines the cache entry corresponds to a currently unwatched directory, - that directory will be added to list of observed directories and scheduled accordingly. """ if self.cache_enabled: self.log.info(f"KernelSpecCache: adding/updating kernelspec: {kernel_name}") if type(cache_item) is KernelSpec: cache_item = KernelSpecCache.kernel_spec_to_cache_item(cache_item) - - resource_dir = cache_item["resource_dir"] self.cache_items[kernel_name.lower()] = cache_item - observed_dir = os.path.dirname(resource_dir) - if observed_dir not in self.observed_dirs: - # New directory to watch, schedule it... - self.log.debug( - "KernelSpecCache: observing directory: {observed_dir}".format( - observed_dir=observed_dir - ) - ) - self.observed_dirs.add(observed_dir) - self.observer.schedule(KernelSpecChangeHandler(self), observed_dir, recursive=True) def put_all_items(self, kernelspecs: Dict[str, CacheItemType]) -> None: """Adds or updates a dictionary of kernel specification in the cache.""" @@ -168,20 +169,64 @@ def remove_item(self, kernel_name: str) -> Optional[CacheItemType]: self.log.info(f"KernelSpecCache: removed kernelspec: {kernel_name}") return cache_item - def _initialize(self): - """Initializes the cache and starts the observer.""" + @staticmethod + def kernel_spec_to_cache_item(kernelspec: KernelSpec) -> CacheItemType: + """Converts a KernelSpec instance to a CacheItemType for storage into the cache.""" + cache_item = {"spec": kernelspec.to_dict(), "resource_dir": kernelspec.resource_dir} + return cache_item - # The kernelspec cache consists of a dictionary mapping the kernel name to the actual - # kernelspec data (CacheItemType). - self.cache_items = {} # Maps kernel name to kernelspec + @staticmethod + def cache_item_to_kernel_spec(cache_item: CacheItemType) -> KernelSpec: + """Converts a CacheItemType to a KernelSpec instance for user consumption.""" + kernel_spec = KernelSpec(resource_dir=cache_item["resource_dir"], **cache_item["spec"]) + return kernel_spec + + +class KernelSpecMonitorBase(ABC): + @classmethod + def create_instance( + cls, kernel_spec_cache: KernelSpecCache, **kwargs + ) -> "KernelSpecMonitorBase": + """Creates an instance of the monitor class configured on the KernelSpecCache instance.""" + monitor_instance = kernel_spec_cache.monitor_class(kernel_spec_cache, **kwargs) + monitor_instance.initialize() + return monitor_instance + + @abstractmethod + def initialize(self) -> None: + """Initializes the monitor.""" + pass + + @abstractmethod + def destroy(self) -> None: + """Destroys the monitor.""" + pass + + +class KernelSpecWatchdogMonitor(KernelSpecMonitorBase): + """Watchdog handler that filters on specific files deemed representative of a kernel specification.""" + + from watchdog.events import FileSystemEventHandler + + def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs): + """Initialize the handler.""" + super().__init__(**kwargs) + self.kernel_spec_cache = kernel_spec_cache + self.kernel_spec_manager = self.kernel_spec_cache.kernel_spec_manager + self.log = kernel_spec_cache.log self.observed_dirs = set() # Tracks which directories are being watched - self.cache_misses = 0 + self.observer = None + + @overrides + def initialize(self): + """Initializes the cache and starts the observer.""" + from watchdog.observers import Observer # Seed the cache and start the observer - if self.cache_enabled: + if self.kernel_spec_cache.cache_enabled: self.observer = Observer() kernelspecs = self.kernel_spec_manager.get_all_specs() - self.put_all_items(kernelspecs) + self.kernel_spec_cache.put_all_items(kernelspecs) # Following adds, see if any of the manager's kernel dirs are not observed and add them for kernel_dir in self.kernel_spec_manager.kernel_dirs: if kernel_dir not in self.observed_dirs: @@ -193,7 +238,9 @@ def _initialize(self): ) self.observed_dirs.add(kernel_dir) self.observer.schedule( - KernelSpecChangeHandler(self), kernel_dir, recursive=True + KernelSpecWatchdogMonitor.WatchDogHandler(self), + kernel_dir, + recursive=True, ) else: self.log.warning( @@ -202,101 +249,92 @@ def _initialize(self): ) self.observer.start() - @staticmethod - def kernel_spec_to_cache_item(kernelspec: KernelSpec) -> CacheItemType: - """Converts a KernelSpec instance to a CacheItemType for storage into the cache.""" - cache_item = {"spec": kernelspec.to_dict(), "resource_dir": kernelspec.resource_dir} - return cache_item - - @staticmethod - def cache_item_to_kernel_spec(cache_item: CacheItemType) -> KernelSpec: - """Converts a CacheItemType to a KernelSpec instance for user consumption.""" - kernel_spec = KernelSpec(resource_dir=cache_item["resource_dir"], **cache_item["spec"]) - return kernel_spec - - -class KernelSpecChangeHandler(FileSystemEventHandler): - """Watchdog handler that filters on specific files deemed representative of a kernel specification.""" - - # Events related to these files trigger the management of the KernelSpec cache. Should we find - # other files qualify as indicators of a kernel specification's state (like perhaps detached parameter - # files in the future) should be added to this list - at which time it should become configurable. - watched_files = ["kernel.json"] - - def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs): - """Initialize the handler.""" - super().__init__(**kwargs) - self.kernel_spec_cache = kernel_spec_cache - self.log = kernel_spec_cache.log - - def dispatch(self, event): - """Dispatches events pertaining to kernelspecs to the appropriate methods. + @overrides + def destroy(self) -> None: + self.observer = None + + class WatchDogHandler(FileSystemEventHandler): + # Events related to these files trigger the management of the KernelSpec cache. Should we find + # other files qualify as indicators of a kernel specification's state (like perhaps detached parameter + # files in the future) should be added to this list - at which time it should become configurable. + watched_files = ["kernel.json"] + + def __init__(self, monitor: "KernelSpecWatchdogMonitor", **kwargs): + """Initialize the handler.""" + super().__init__(**kwargs) + self.kernel_spec_cache = monitor.kernel_spec_cache + self.log = monitor.kernel_spec_cache.log + + def dispatch(self, event): + """Dispatches events pertaining to kernelspecs to the appropriate methods. + + + The primary purpose of this method is to ensure the action is occurring against + the a file in the list of watched files and adds some additional attributes to + the event instance to make the actual event handling method easier. + :param event: + The event object representing the file system event. + :type event: + :class:`FileSystemEvent` + """ + from watchdog.events import FileMovedEvent + + if os.path.basename(event.src_path) in self.watched_files: + src_resource_dir = os.path.dirname(event.src_path) + event.src_resource_dir = src_resource_dir + event.src_kernel_name = os.path.basename(src_resource_dir) + if type(event) is FileMovedEvent: + dest_resource_dir = os.path.dirname(event.dest_path) + event.dest_resource_dir = dest_resource_dir + event.dest_kernel_name = os.path.basename(dest_resource_dir) + + super().dispatch(event) + + def on_created(self, event): + """Fires when a watched file is created. + + This will trigger a call to the configured KernelSpecManager to fetch the instance + associated with the created file, which is then added to the cache. + """ + kernel_name = event.src_kernel_name + try: + kernelspec = self.kernel_spec_cache.kernel_spec_manager.get_kernel_spec(kernel_name) + self.kernel_spec_cache.put_item(kernel_name, kernelspec) + except Exception as e: + self.log.warning( + "The following exception occurred creating cache entry for: {src_resource_dir} " + "- continuing... ({e})".format(src_resource_dir=event.src_resource_dir, e=e) + ) + def on_deleted(self, event): + """Fires when a watched file is deleted, triggering a removal of the corresponding item from the cache.""" + kernel_name = event.src_kernel_name + self.kernel_spec_cache.remove_item(kernel_name) + + def on_modified(self, event): + """Fires when a watched file is modified. + + This will trigger a call to the configured KernelSpecManager to fetch the instance + associated with the modified file, which is then replaced in the cache. + """ + kernel_name = event.src_kernel_name + try: + kernelspec = self.kernel_spec_cache.kernel_spec_manager.get_kernel_spec(kernel_name) + self.kernel_spec_cache.put_item(kernel_name, kernelspec) + except Exception as e: + self.log.warning( + "The following exception occurred updating cache entry for: {src_resource_dir} " + "- continuing... ({e})".format(src_resource_dir=event.src_resource_dir, e=e) + ) - The primary purpose of this method is to ensure the action is occurring against - the a file in the list of watched files and adds some additional attributes to - the event instance to make the actual event handling method easier. - :param event: - The event object representing the file system event. - :type event: - :class:`FileSystemEvent` - """ - if os.path.basename(event.src_path) in self.watched_files: - src_resource_dir = os.path.dirname(event.src_path) - event.src_resource_dir = src_resource_dir - event.src_kernel_name = os.path.basename(src_resource_dir) - if type(event) is FileMovedEvent: - dest_resource_dir = os.path.dirname(event.dest_path) - event.dest_resource_dir = dest_resource_dir - event.dest_kernel_name = os.path.basename(dest_resource_dir) - - super().dispatch(event) - - def on_created(self, event): - """Fires when a watched file is created. - - This will trigger a call to the configured KernelSpecManager to fetch the instance - associated with the created file, which is then added to the cache. - """ - kernel_name = event.src_kernel_name - try: - kernelspec = self.kernel_spec_cache.kernel_spec_manager.get_kernel_spec(kernel_name) - self.kernel_spec_cache.put_item(kernel_name, kernelspec) - except Exception as e: - self.log.warning( - "The following exception occurred creating cache entry for: {src_resource_dir} " - "- continuing... ({e})".format(src_resource_dir=event.src_resource_dir, e=e) - ) - - def on_deleted(self, event): - """Fires when a watched file is deleted, triggering a removal of the corresponding item from the cache.""" - kernel_name = event.src_kernel_name - self.kernel_spec_cache.remove_item(kernel_name) - - def on_modified(self, event): - """Fires when a watched file is modified. - - This will trigger a call to the configured KernelSpecManager to fetch the instance - associated with the modified file, which is then replaced in the cache. - """ - kernel_name = event.src_kernel_name - try: - kernelspec = self.kernel_spec_cache.kernel_spec_manager.get_kernel_spec(kernel_name) - self.kernel_spec_cache.put_item(kernel_name, kernelspec) - except Exception as e: - self.log.warning( - "The following exception occurred updating cache entry for: {src_resource_dir} " - "- continuing... ({e})".format(src_resource_dir=event.src_resource_dir, e=e) - ) - - def on_moved(self, event): - """Fires when a watched file is moved. - - This will trigger the update of the existing cached item, replacing its resource_dir entry - with that of the new destination. - """ - src_kernel_name = event.src_kernel_name - dest_kernel_name = event.dest_kernel_name - cache_item = self.kernel_spec_cache.remove_item(src_kernel_name) - cache_item["resource_dir"] = event.dest_resource_dir - self.kernel_spec_cache.put_item(dest_kernel_name, cache_item) + def on_moved(self, event): + """Fires when a watched file is moved. + + This will trigger the update of the existing cached item, replacing its resource_dir entry + with that of the new destination. + """ + src_kernel_name = event.src_kernel_name + dest_kernel_name = event.dest_kernel_name + cache_item = self.kernel_spec_cache.remove_item(src_kernel_name) + cache_item["resource_dir"] = event.dest_resource_dir + self.kernel_spec_cache.put_item(dest_kernel_name, cache_item) diff --git a/tests/services/kernelspecs/test_kernelspec_cache.py b/tests/services/kernelspecs/test_kernelspec_cache.py index 740da16099..603b8839ab 100644 --- a/tests/services/kernelspecs/test_kernelspec_cache.py +++ b/tests/services/kernelspecs/test_kernelspec_cache.py @@ -143,26 +143,16 @@ async def tests_get_modified_spec(kernel_spec_cache): async def tests_add_spec(kernel_spec_cache, kernelspec_location, other_kernelspec_location): - assert len(kernel_spec_cache.observed_dirs) == (1 if kernel_spec_cache.cache_enabled else 0) - assert ( - str(kernelspec_location) in kernel_spec_cache.observed_dirs - if kernel_spec_cache.cache_enabled - else True - ) + with pytest.raises(NoSuchKernel): + await kernel_spec_cache.get_kernel_spec("added") # this will increment cache_miss _install_kernelspec(str(other_kernelspec_location), "added") + # this will increment cache_miss prior to load kspec = await kernel_spec_cache.get_kernel_spec("added") - # Ensure new location has been added to observed_dirs - assert len(kernel_spec_cache.observed_dirs) == (2 if kernel_spec_cache.cache_enabled else 0) - assert ( - str(other_kernelspec_location) in kernel_spec_cache.observed_dirs - if kernel_spec_cache.cache_enabled - else True - ) - assert kspec.display_name == "Test kernel: added" - assert kernel_spec_cache.cache_misses == (1 if kernel_spec_cache.cache_enabled else 0) + # Cache misses should be 2, one for prior to adding the spec, the other after discovering its addition + assert kernel_spec_cache.cache_misses == (2 if kernel_spec_cache.cache_enabled else 0) # Add another to an existing observed directory, no cache miss here _install_kernelspec(str(kernelspec_location), "added2") @@ -172,7 +162,7 @@ async def tests_add_spec(kernel_spec_cache, kernelspec_location, other_kernelspe kspec = await kernel_spec_cache.get_kernel_spec("added2") assert kspec.display_name == "Test kernel: added2" - assert kernel_spec_cache.cache_misses == (1 if kernel_spec_cache.cache_enabled else 0) + assert kernel_spec_cache.cache_misses == (2 if kernel_spec_cache.cache_enabled else 0) async def tests_remove_spec(kernel_spec_cache): @@ -181,7 +171,7 @@ async def tests_remove_spec(kernel_spec_cache): assert kernel_spec_cache.cache_misses == 0 shutil.rmtree(kspec.resource_dir) - await asyncio.sleep(0.5) # sleep for a half-second to allow cache to remove item + await asyncio.sleep(1.5) # sleep for a half-second to allow cache to remove item with pytest.raises(NoSuchKernel): await kernel_spec_cache.get_kernel_spec("test2") From 8b6bdd173e482457f785ca456b331204c837a553 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Tue, 9 May 2023 08:49:05 -0700 Subject: [PATCH 03/16] Derive KernelSpecCache from LoggingConfigurable, not SingletonConfigurable --- jupyter_server/services/kernelspecs/kernelspec_cache.py | 4 ++-- tests/services/kernelspecs/test_kernelspec_cache.py | 6 +----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/jupyter_server/services/kernelspecs/kernelspec_cache.py b/jupyter_server/services/kernelspecs/kernelspec_cache.py index 8d9e02df28..a6def14f4c 100644 --- a/jupyter_server/services/kernelspecs/kernelspec_cache.py +++ b/jupyter_server/services/kernelspecs/kernelspec_cache.py @@ -9,7 +9,7 @@ from jupyter_client.kernelspec import KernelSpec from overrides import overrides -from traitlets.config import SingletonConfigurable +from traitlets.config import LoggingConfigurable from traitlets.traitlets import CBool, Instance, Type, default from jupyter_server.utils import ensure_async @@ -20,7 +20,7 @@ CacheItemType = Dict[str, Union[str, Dict]] -class KernelSpecCache(SingletonConfigurable): +class KernelSpecCache(LoggingConfigurable): """The primary (singleton) instance for managing KernelSpecs. This class contains the configured KernelSpecManager instance upon diff --git a/tests/services/kernelspecs/test_kernelspec_cache.py b/tests/services/kernelspecs/test_kernelspec_cache.py index 603b8839ab..bdcebcfcd5 100644 --- a/tests/services/kernelspecs/test_kernelspec_cache.py +++ b/tests/services/kernelspecs/test_kernelspec_cache.py @@ -108,12 +108,8 @@ def kernel_spec_manager(environ, setup_kernelspecs): @pytest.fixture def kernel_spec_cache(is_enabled, kernel_spec_manager): - kspec_cache = KernelSpecCache.instance( - kernel_spec_manager=kernel_spec_manager, cache_enabled=is_enabled - ) + kspec_cache = KernelSpecCache(kernel_spec_manager=kernel_spec_manager, cache_enabled=is_enabled) yield kspec_cache - kspec_cache = None - KernelSpecCache.clear_instance() @pytest.fixture(params=[False, True]) # Add types as needed From 2c9cfc5b705da43da8bfe5343e5d4164b8adaa28 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Tue, 9 May 2023 13:38:15 -0700 Subject: [PATCH 04/16] Convert monitors to be driven by entry_points --- .../services/kernelspecs/kernelspec_cache.py | 202 +++++------------- .../services/kernelspecs/monitors/__init__.py | 1 + .../kernelspecs/monitors/watchdog_monitor.py | 145 +++++++++++++ pyproject.toml | 3 + .../kernelspecs/test_kernelspec_cache.py | 2 +- 5 files changed, 201 insertions(+), 152 deletions(-) create mode 100644 jupyter_server/services/kernelspecs/monitors/__init__.py create mode 100644 jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py diff --git a/jupyter_server/services/kernelspecs/kernelspec_cache.py b/jupyter_server/services/kernelspecs/kernelspec_cache.py index a6def14f4c..f4324a0a27 100644 --- a/jupyter_server/services/kernelspecs/kernelspec_cache.py +++ b/jupyter_server/services/kernelspecs/kernelspec_cache.py @@ -4,13 +4,19 @@ import os -from abc import ABC, abstractmethod +import sys +from abc import ABC, ABCMeta, abstractmethod from typing import Dict, Optional, Union +# See compatibility note on `group` keyword in https://docs.python.org/3/library/importlib.metadata.html#entry-points +if sys.version_info < (3, 10): # pragma: no cover + from importlib_metadata import EntryPoint, entry_points +else: # pragma: no cover + from importlib.metadata import EntryPoint, entry_points + from jupyter_client.kernelspec import KernelSpec -from overrides import overrides from traitlets.config import LoggingConfigurable -from traitlets.traitlets import CBool, Instance, Type, default +from traitlets.traitlets import CBool, Instance, Unicode, default from jupyter_server.utils import ensure_async @@ -45,14 +51,13 @@ def _cache_enabled_default(self): kernel_spec_manager = Instance("jupyter_client.kernelspec.KernelSpecManager") - monitor_class = Type( - klass="jupyter_server.services.kernelspecs.kernelspec_cache.KernelSpecMonitorBase", - help="""The monitor class to use to capture kernelspecs.""", + monitor_entry_point = Unicode( + help="""The monitor entry_point to use to capture kernelspecs updates.""", ).tag(config=True) - @default("monitor_class") - def _monitor_class_default(self): - return "jupyter_server.services.kernelspecs.kernelspec_cache.KernelSpecWatchdogMonitor" + @default("monitor_entry_point") + def _monitor_entry_point_default(self): + return "watchdog-monitor" # The kernelspec cache consists of a dictionary mapping the kernel name to the actual # kernelspec data (CacheItemType). @@ -63,7 +68,9 @@ def __init__(self, kernel_spec_manager, **kwargs) -> None: """Initialize the cache.""" super().__init__(**kwargs) self.kernel_spec_manager = kernel_spec_manager - self.kernel_spec_monitor = KernelSpecMonitorBase.create_instance(self) + self.kernel_spec_monitor = None + if self.cache_enabled: + self.kernel_spec_monitor = KernelSpecMonitorBase.create_instance(self) async def get_kernel_spec(self, kernel_name: str) -> KernelSpec: """Get the named kernel specification. @@ -182,15 +189,45 @@ def cache_item_to_kernel_spec(cache_item: CacheItemType) -> KernelSpec: return kernel_spec -class KernelSpecMonitorBase(ABC): +class KernelSpecMonitorMeta(ABCMeta, type(LoggingConfigurable)): # type: ignore + pass + + +class KernelSpecMonitorBase( # type:ignore[misc] + ABC, LoggingConfigurable, metaclass=KernelSpecMonitorMeta +): + GROUP_NAME = "jupyter_server.kernelspec_monitors" + @classmethod def create_instance( cls, kernel_spec_cache: KernelSpecCache, **kwargs ) -> "KernelSpecMonitorBase": """Creates an instance of the monitor class configured on the KernelSpecCache instance.""" - monitor_instance = kernel_spec_cache.monitor_class(kernel_spec_cache, **kwargs) - monitor_instance.initialize() - return monitor_instance + + entry_point_name = kernel_spec_cache.monitor_entry_point + eps = entry_points(group=KernelSpecMonitorBase.GROUP_NAME, name=entry_point_name) + if eps: + ep: EntryPoint = eps[entry_point_name] + monitor_class = ep.load() + monitor_instance: KernelSpecMonitorBase = monitor_class( + kernel_spec_cache=kernel_spec_cache, **kwargs + ) + if not isinstance(monitor_instance, KernelSpecMonitorBase): + msg = ( + f"Entrypoint '{kernel_spec_cache.monitor_entry_point}' of " + f"group '{KernelSpecMonitorBase.GROUP_NAME}' is not a " + f"subclass of '{KernelSpecMonitorBase.__name__}'" + ) + raise RuntimeError(msg) + + monitor_instance.initialize() + return monitor_instance + else: + msg = ( + f"Entrypoint '{kernel_spec_cache.monitor_entry_point}' of " + f"group '{KernelSpecMonitorBase.GROUP_NAME}' cannot be located." + ) + raise RuntimeError(msg) @abstractmethod def initialize(self) -> None: @@ -201,140 +238,3 @@ def initialize(self) -> None: def destroy(self) -> None: """Destroys the monitor.""" pass - - -class KernelSpecWatchdogMonitor(KernelSpecMonitorBase): - """Watchdog handler that filters on specific files deemed representative of a kernel specification.""" - - from watchdog.events import FileSystemEventHandler - - def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs): - """Initialize the handler.""" - super().__init__(**kwargs) - self.kernel_spec_cache = kernel_spec_cache - self.kernel_spec_manager = self.kernel_spec_cache.kernel_spec_manager - self.log = kernel_spec_cache.log - self.observed_dirs = set() # Tracks which directories are being watched - self.observer = None - - @overrides - def initialize(self): - """Initializes the cache and starts the observer.""" - from watchdog.observers import Observer - - # Seed the cache and start the observer - if self.kernel_spec_cache.cache_enabled: - self.observer = Observer() - kernelspecs = self.kernel_spec_manager.get_all_specs() - self.kernel_spec_cache.put_all_items(kernelspecs) - # Following adds, see if any of the manager's kernel dirs are not observed and add them - for kernel_dir in self.kernel_spec_manager.kernel_dirs: - if kernel_dir not in self.observed_dirs: - if os.path.exists(kernel_dir): - self.log.info( - "KernelSpecCache: observing directory: {kernel_dir}".format( - kernel_dir=kernel_dir - ) - ) - self.observed_dirs.add(kernel_dir) - self.observer.schedule( - KernelSpecWatchdogMonitor.WatchDogHandler(self), - kernel_dir, - recursive=True, - ) - else: - self.log.warning( - "KernelSpecCache: kernel_dir '{kernel_dir}' does not exist" - " and will not be observed.".format(kernel_dir=kernel_dir) - ) - self.observer.start() - - @overrides - def destroy(self) -> None: - self.observer = None - - class WatchDogHandler(FileSystemEventHandler): - # Events related to these files trigger the management of the KernelSpec cache. Should we find - # other files qualify as indicators of a kernel specification's state (like perhaps detached parameter - # files in the future) should be added to this list - at which time it should become configurable. - watched_files = ["kernel.json"] - - def __init__(self, monitor: "KernelSpecWatchdogMonitor", **kwargs): - """Initialize the handler.""" - super().__init__(**kwargs) - self.kernel_spec_cache = monitor.kernel_spec_cache - self.log = monitor.kernel_spec_cache.log - - def dispatch(self, event): - """Dispatches events pertaining to kernelspecs to the appropriate methods. - - - The primary purpose of this method is to ensure the action is occurring against - the a file in the list of watched files and adds some additional attributes to - the event instance to make the actual event handling method easier. - :param event: - The event object representing the file system event. - :type event: - :class:`FileSystemEvent` - """ - from watchdog.events import FileMovedEvent - - if os.path.basename(event.src_path) in self.watched_files: - src_resource_dir = os.path.dirname(event.src_path) - event.src_resource_dir = src_resource_dir - event.src_kernel_name = os.path.basename(src_resource_dir) - if type(event) is FileMovedEvent: - dest_resource_dir = os.path.dirname(event.dest_path) - event.dest_resource_dir = dest_resource_dir - event.dest_kernel_name = os.path.basename(dest_resource_dir) - - super().dispatch(event) - - def on_created(self, event): - """Fires when a watched file is created. - - This will trigger a call to the configured KernelSpecManager to fetch the instance - associated with the created file, which is then added to the cache. - """ - kernel_name = event.src_kernel_name - try: - kernelspec = self.kernel_spec_cache.kernel_spec_manager.get_kernel_spec(kernel_name) - self.kernel_spec_cache.put_item(kernel_name, kernelspec) - except Exception as e: - self.log.warning( - "The following exception occurred creating cache entry for: {src_resource_dir} " - "- continuing... ({e})".format(src_resource_dir=event.src_resource_dir, e=e) - ) - - def on_deleted(self, event): - """Fires when a watched file is deleted, triggering a removal of the corresponding item from the cache.""" - kernel_name = event.src_kernel_name - self.kernel_spec_cache.remove_item(kernel_name) - - def on_modified(self, event): - """Fires when a watched file is modified. - - This will trigger a call to the configured KernelSpecManager to fetch the instance - associated with the modified file, which is then replaced in the cache. - """ - kernel_name = event.src_kernel_name - try: - kernelspec = self.kernel_spec_cache.kernel_spec_manager.get_kernel_spec(kernel_name) - self.kernel_spec_cache.put_item(kernel_name, kernelspec) - except Exception as e: - self.log.warning( - "The following exception occurred updating cache entry for: {src_resource_dir} " - "- continuing... ({e})".format(src_resource_dir=event.src_resource_dir, e=e) - ) - - def on_moved(self, event): - """Fires when a watched file is moved. - - This will trigger the update of the existing cached item, replacing its resource_dir entry - with that of the new destination. - """ - src_kernel_name = event.src_kernel_name - dest_kernel_name = event.dest_kernel_name - cache_item = self.kernel_spec_cache.remove_item(src_kernel_name) - cache_item["resource_dir"] = event.dest_resource_dir - self.kernel_spec_cache.put_item(dest_kernel_name, cache_item) diff --git a/jupyter_server/services/kernelspecs/monitors/__init__.py b/jupyter_server/services/kernelspecs/monitors/__init__.py new file mode 100644 index 0000000000..637d4d2cf1 --- /dev/null +++ b/jupyter_server/services/kernelspecs/monitors/__init__.py @@ -0,0 +1 @@ +from .watchdog_monitor import KernelSpecWatchdogMonitor # noqa diff --git a/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py b/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py new file mode 100644 index 0000000000..b1fc34c2fa --- /dev/null +++ b/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py @@ -0,0 +1,145 @@ +"""KernelSpec watchdog monitor used by KernelspecCache.""" +# Copyright (c) Jupyter Development Team. +# Distributed under the terms of the Modified BSD License. +import os + +from overrides import overrides + +from ..kernelspec_cache import KernelSpecCache, KernelSpecMonitorBase + + +class KernelSpecWatchdogMonitor(KernelSpecMonitorBase): + """Watchdog handler that filters on specific files deemed representative of a kernel specification.""" + + from watchdog.events import FileSystemEventHandler + + def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs): + """Initialize the handler.""" + super().__init__(**kwargs) + self.kernel_spec_cache = kernel_spec_cache + self.kernel_spec_manager = self.kernel_spec_cache.kernel_spec_manager + self.log = kernel_spec_cache.log + self.observed_dirs = set() # Tracks which directories are being watched + self.observer = None + + @overrides + def initialize(self): + """Initializes the cache and starts the observer.""" + from watchdog.observers import Observer + + # Seed the cache and start the observer + if self.kernel_spec_cache.cache_enabled: + self.observer = Observer() + kernelspecs = self.kernel_spec_manager.get_all_specs() + self.kernel_spec_cache.put_all_items(kernelspecs) + # Following adds, see if any of the manager's kernel dirs are not observed and add them + for kernel_dir in self.kernel_spec_manager.kernel_dirs: + if kernel_dir not in self.observed_dirs: + if os.path.exists(kernel_dir): + self.log.info( + "KernelSpecCache: observing directory: {kernel_dir}".format( + kernel_dir=kernel_dir + ) + ) + self.observed_dirs.add(kernel_dir) + self.observer.schedule( + KernelSpecWatchdogMonitor.WatchDogHandler(self), + kernel_dir, + recursive=True, + ) + else: + self.log.warning( + "KernelSpecCache: kernel_dir '{kernel_dir}' does not exist" + " and will not be observed.".format(kernel_dir=kernel_dir) + ) + self.observer.start() + + @overrides + def destroy(self) -> None: + self.observer = None + + class WatchDogHandler(FileSystemEventHandler): + # Events related to these files trigger the management of the KernelSpec cache. Should we find + # other files qualify as indicators of a kernel specification's state (like perhaps detached parameter + # files in the future) should be added to this list - at which time it should become configurable. + watched_files = ["kernel.json"] + + def __init__(self, monitor: "KernelSpecWatchdogMonitor", **kwargs): + """Initialize the handler.""" + super().__init__(**kwargs) + self.kernel_spec_cache = monitor.kernel_spec_cache + self.log = monitor.kernel_spec_cache.log + + def dispatch(self, event): + """Dispatches events pertaining to kernelspecs to the appropriate methods. + + + The primary purpose of this method is to ensure the action is occurring against + the a file in the list of watched files and adds some additional attributes to + the event instance to make the actual event handling method easier. + :param event: + The event object representing the file system event. + :type event: + :class:`FileSystemEvent` + """ + from watchdog.events import FileMovedEvent + + if os.path.basename(event.src_path) in self.watched_files: + src_resource_dir = os.path.dirname(event.src_path) + event.src_resource_dir = src_resource_dir + event.src_kernel_name = os.path.basename(src_resource_dir) + if type(event) is FileMovedEvent: + dest_resource_dir = os.path.dirname(event.dest_path) + event.dest_resource_dir = dest_resource_dir + event.dest_kernel_name = os.path.basename(dest_resource_dir) + + super().dispatch(event) + + def on_created(self, event): + """Fires when a watched file is created. + + This will trigger a call to the configured KernelSpecManager to fetch the instance + associated with the created file, which is then added to the cache. + """ + kernel_name = event.src_kernel_name + try: + kernelspec = self.kernel_spec_cache.kernel_spec_manager.get_kernel_spec(kernel_name) + self.kernel_spec_cache.put_item(kernel_name, kernelspec) + except Exception as e: + self.log.warning( + "The following exception occurred creating cache entry for: {src_resource_dir} " + "- continuing... ({e})".format(src_resource_dir=event.src_resource_dir, e=e) + ) + + def on_deleted(self, event): + """Fires when a watched file is deleted, triggering a removal of the corresponding item from the cache.""" + kernel_name = event.src_kernel_name + self.kernel_spec_cache.remove_item(kernel_name) + + def on_modified(self, event): + """Fires when a watched file is modified. + + This will trigger a call to the configured KernelSpecManager to fetch the instance + associated with the modified file, which is then replaced in the cache. + """ + kernel_name = event.src_kernel_name + try: + kernelspec = self.kernel_spec_cache.kernel_spec_manager.get_kernel_spec(kernel_name) + self.kernel_spec_cache.put_item(kernel_name, kernelspec) + except Exception as e: + self.log.warning( + "The following exception occurred updating cache entry for: {src_resource_dir} " + "- continuing... ({e})".format(src_resource_dir=event.src_resource_dir, e=e) + ) + + def on_moved(self, event): + """Fires when a watched file is moved. + + This will trigger the update of the existing cached item, replacing its resource_dir entry + with that of the new destination. + """ + src_kernel_name = event.src_kernel_name + dest_kernel_name = event.dest_kernel_name + cache_item = self.kernel_spec_cache.remove_item(src_kernel_name) + cache_item["resource_dir"] = event.dest_resource_dir + self.kernel_spec_cache.put_item(dest_kernel_name, cache_item) diff --git a/pyproject.toml b/pyproject.toml index b094fb7efc..3f9cd25be9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -94,6 +94,9 @@ docs = [ [project.scripts] jupyter-server = "jupyter_server.serverapp:main" +[project.entry-points."jupyter_server.kernelspec_monitors"] +watchdog-monitor = "jupyter_server.services.kernelspecs.monitors:KernelSpecWatchdogMonitor" + [tool.hatch.envs.docs] features = ["docs"] [tool.hatch.envs.docs.scripts] diff --git a/tests/services/kernelspecs/test_kernelspec_cache.py b/tests/services/kernelspecs/test_kernelspec_cache.py index bdcebcfcd5..6655f53b3c 100644 --- a/tests/services/kernelspecs/test_kernelspec_cache.py +++ b/tests/services/kernelspecs/test_kernelspec_cache.py @@ -167,7 +167,7 @@ async def tests_remove_spec(kernel_spec_cache): assert kernel_spec_cache.cache_misses == 0 shutil.rmtree(kspec.resource_dir) - await asyncio.sleep(1.5) # sleep for a half-second to allow cache to remove item + await asyncio.sleep(0.5) # sleep for a half-second to allow cache to remove item with pytest.raises(NoSuchKernel): await kernel_spec_cache.get_kernel_spec("test2") From aae48c7eae04be300a9ecd22b286290ce4d48ba7 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Tue, 9 May 2023 16:20:28 -0700 Subject: [PATCH 05/16] Add KernelSpecPollingMonitor --- .../services/kernelspecs/kernelspec_cache.py | 17 +++-- .../services/kernelspecs/monitors/__init__.py | 1 + .../kernelspecs/monitors/polling_monitor.py | 62 +++++++++++++++++++ .../kernelspecs/monitors/watchdog_monitor.py | 5 +- pyproject.toml | 1 + .../kernelspecs/test_kernelspec_cache.py | 44 ++++++++++--- 6 files changed, 114 insertions(+), 16 deletions(-) create mode 100644 jupyter_server/services/kernelspecs/monitors/polling_monitor.py diff --git a/jupyter_server/services/kernelspecs/kernelspec_cache.py b/jupyter_server/services/kernelspecs/kernelspec_cache.py index f4324a0a27..0f55b19961 100644 --- a/jupyter_server/services/kernelspecs/kernelspec_cache.py +++ b/jupyter_server/services/kernelspecs/kernelspec_cache.py @@ -57,7 +57,7 @@ def _cache_enabled_default(self): @default("monitor_entry_point") def _monitor_entry_point_default(self): - return "watchdog-monitor" + return "polling-monitor" # The kernelspec cache consists of a dictionary mapping the kernel name to the actual # kernelspec data (CacheItemType). @@ -70,7 +70,7 @@ def __init__(self, kernel_spec_manager, **kwargs) -> None: self.kernel_spec_manager = kernel_spec_manager self.kernel_spec_monitor = None if self.cache_enabled: - self.kernel_spec_monitor = KernelSpecMonitorBase.create_instance(self) + self.kernel_spec_monitor = KernelSpecMonitorBase.create_instance(parent=self, **kwargs) async def get_kernel_spec(self, kernel_name: str) -> KernelSpec: """Get the named kernel specification. @@ -176,6 +176,12 @@ def remove_item(self, kernel_name: str) -> Optional[CacheItemType]: self.log.info(f"KernelSpecCache: removed kernelspec: {kernel_name}") return cache_item + def remove_all_items(self) -> None: + """Removes all items from the cache.""" + if self.cache_enabled: + self.cache_items.clear() + self.log.info("KernelSpecCache: all items removed from cache") + @staticmethod def kernel_spec_to_cache_item(kernelspec: KernelSpec) -> CacheItemType: """Converts a KernelSpec instance to a CacheItemType for storage into the cache.""" @@ -199,18 +205,17 @@ class KernelSpecMonitorBase( # type:ignore[misc] GROUP_NAME = "jupyter_server.kernelspec_monitors" @classmethod - def create_instance( - cls, kernel_spec_cache: KernelSpecCache, **kwargs - ) -> "KernelSpecMonitorBase": + def create_instance(cls, **kwargs) -> "KernelSpecMonitorBase": """Creates an instance of the monitor class configured on the KernelSpecCache instance.""" + kernel_spec_cache = kwargs["parent"] entry_point_name = kernel_spec_cache.monitor_entry_point eps = entry_points(group=KernelSpecMonitorBase.GROUP_NAME, name=entry_point_name) if eps: ep: EntryPoint = eps[entry_point_name] monitor_class = ep.load() monitor_instance: KernelSpecMonitorBase = monitor_class( - kernel_spec_cache=kernel_spec_cache, **kwargs + parent=kernel_spec_cache, config=kwargs.get("config") ) if not isinstance(monitor_instance, KernelSpecMonitorBase): msg = ( diff --git a/jupyter_server/services/kernelspecs/monitors/__init__.py b/jupyter_server/services/kernelspecs/monitors/__init__.py index 637d4d2cf1..54ae7aaafe 100644 --- a/jupyter_server/services/kernelspecs/monitors/__init__.py +++ b/jupyter_server/services/kernelspecs/monitors/__init__.py @@ -1 +1,2 @@ from .watchdog_monitor import KernelSpecWatchdogMonitor # noqa +from .polling_monitor import KernelSpecPollingMonitor # noqa diff --git a/jupyter_server/services/kernelspecs/monitors/polling_monitor.py b/jupyter_server/services/kernelspecs/monitors/polling_monitor.py new file mode 100644 index 0000000000..6a5272e5a4 --- /dev/null +++ b/jupyter_server/services/kernelspecs/monitors/polling_monitor.py @@ -0,0 +1,62 @@ +"""KernelSpec watchdog monitor used by KernelspecCache.""" +# Copyright (c) Jupyter Development Team. +# Distributed under the terms of the Modified BSD License. + +from overrides import overrides +from traitlets.traitlets import Float + +from ..kernelspec_cache import KernelSpecCache, KernelSpecMonitorBase + + +class KernelSpecPollingMonitor(KernelSpecMonitorBase): + """Polling monitor that uses a periodic poll period to reload the kernelspec cache.""" + + interval = Float( + default_value=30.0, + config=True, + help="""The interval (in seconds) at which kernelspecs are updated in the cache.""", + ) + + _pcallback = None + + def __init__(self, **kwargs): + """Initialize the handler.""" + super().__init__(**kwargs) + self.kernel_spec_cache: KernelSpecCache = kwargs["parent"] + self.kernel_spec_manager = self.kernel_spec_cache.kernel_spec_manager + self.log.info(f"Starting {self.__class__.__name__} with interval: {self.interval} ...") + + @overrides + def initialize(self): + """Initializes the cache and starts the registers the periodic poller.""" + + # Seed the cache and start the observer + if self.kernel_spec_cache.cache_enabled: + self.poll() + self.start() + + @overrides + def destroy(self) -> None: + self.stop() + + def poll(self): + self.kernel_spec_cache.remove_all_items() + kernelspecs = self.kernel_spec_manager.get_all_specs() + self.kernel_spec_cache.put_all_items(kernelspecs) + + def start(self): + """Start the polling of the kernel.""" + if self._pcallback is None: + from tornado.ioloop import PeriodicCallback + + self._pcallback = PeriodicCallback( + self.poll, + 1000 * self.interval, + ) + self._pcallback.start() + + def stop(self): + """Stop the kernel polling.""" + if self._pcallback is not None: + self._pcallback.stop() + self._pcallback = None diff --git a/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py b/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py index b1fc34c2fa..b8ab1424b1 100644 --- a/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py +++ b/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py @@ -13,12 +13,11 @@ class KernelSpecWatchdogMonitor(KernelSpecMonitorBase): from watchdog.events import FileSystemEventHandler - def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs): + def __init__(self, **kwargs): """Initialize the handler.""" super().__init__(**kwargs) - self.kernel_spec_cache = kernel_spec_cache + self.kernel_spec_cache: KernelSpecCache = kwargs["parent"] self.kernel_spec_manager = self.kernel_spec_cache.kernel_spec_manager - self.log = kernel_spec_cache.log self.observed_dirs = set() # Tracks which directories are being watched self.observer = None diff --git a/pyproject.toml b/pyproject.toml index 3f9cd25be9..9b2d3bc422 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -96,6 +96,7 @@ jupyter-server = "jupyter_server.serverapp:main" [project.entry-points."jupyter_server.kernelspec_monitors"] watchdog-monitor = "jupyter_server.services.kernelspecs.monitors:KernelSpecWatchdogMonitor" +polling-monitor = "jupyter_server.services.kernelspecs.monitors:KernelSpecPollingMonitor" [tool.hatch.envs.docs] features = ["docs"] diff --git a/tests/services/kernelspecs/test_kernelspec_cache.py b/tests/services/kernelspecs/test_kernelspec_cache.py index 6655f53b3c..339051b97f 100644 --- a/tests/services/kernelspecs/test_kernelspec_cache.py +++ b/tests/services/kernelspecs/test_kernelspec_cache.py @@ -11,6 +11,7 @@ import jupyter_core.paths import pytest from jupyter_client.kernelspec import KernelSpecManager, NoSuchKernel +from traitlets.config import Config from jupyter_server.services.kernelspecs.kernelspec_cache import KernelSpecCache @@ -106,10 +107,39 @@ def kernel_spec_manager(environ, setup_kernelspecs): yield KernelSpecManager(ensure_native_kernel=False) -@pytest.fixture -def kernel_spec_cache(is_enabled, kernel_spec_manager): - kspec_cache = KernelSpecCache(kernel_spec_manager=kernel_spec_manager, cache_enabled=is_enabled) +MONITORS = ["watchdog-monitor", "polling-monitor"] + + +@pytest.fixture(params=MONITORS) +def kernel_spec_cache(request, is_enabled, kernel_spec_manager): + config = None + if request.param == "polling-monitor": + config = Config( + { + "KernelSpecCache": { + "KernelSpecPollingMonitor": { + "interval": 1.0, + } + } + } + ) + + kspec_cache = KernelSpecCache( + kernel_spec_manager=kernel_spec_manager, + cache_enabled=is_enabled, + monitor_entry_point=request.param, + config=config, + ) yield kspec_cache + kspec_cache = None + + +def get_delay_factor(kernel_spec_cache: KernelSpecCache): + if kernel_spec_cache.cache_enabled: + if kernel_spec_cache.monitor_entry_point == "polling-monitor": + return 2.0 + return 1.0 + return 0.5 @pytest.fixture(params=[False, True]) # Add types as needed @@ -133,7 +163,7 @@ async def tests_get_modified_spec(kernel_spec_cache): # Modify entry _modify_kernelspec(kspec.resource_dir, "test2") - await asyncio.sleep(0.5) # sleep for a half-second to allow cache to update item + await asyncio.sleep(get_delay_factor(kernel_spec_cache)) # sleep to allow cache to update item kspec = await kernel_spec_cache.get_kernel_spec("test2") assert kspec.display_name == "test2 modified!" @@ -153,8 +183,8 @@ async def tests_add_spec(kernel_spec_cache, kernelspec_location, other_kernelspe # Add another to an existing observed directory, no cache miss here _install_kernelspec(str(kernelspec_location), "added2") await asyncio.sleep( - 0.5 - ) # sleep for a half-second to allow cache to add item (no cache miss in this case) + get_delay_factor(kernel_spec_cache) + ) # sleep to allow cache to add item (no cache miss in this case) kspec = await kernel_spec_cache.get_kernel_spec("added2") assert kspec.display_name == "Test kernel: added2" @@ -167,7 +197,7 @@ async def tests_remove_spec(kernel_spec_cache): assert kernel_spec_cache.cache_misses == 0 shutil.rmtree(kspec.resource_dir) - await asyncio.sleep(0.5) # sleep for a half-second to allow cache to remove item + await asyncio.sleep(get_delay_factor(kernel_spec_cache)) # sleep to allow cache to remove item with pytest.raises(NoSuchKernel): await kernel_spec_cache.get_kernel_spec("test2") From cc513d4434263aa73b61079d241ea9cae5615fa4 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Tue, 9 May 2023 17:32:14 -0700 Subject: [PATCH 06/16] Cleanup tests, use server's fixtures --- .../kernelspecs/test_kernelspec_cache.py | 73 +++---------------- 1 file changed, 12 insertions(+), 61 deletions(-) diff --git a/tests/services/kernelspecs/test_kernelspec_cache.py b/tests/services/kernelspecs/test_kernelspec_cache.py index 339051b97f..9448a9ffd3 100644 --- a/tests/services/kernelspecs/test_kernelspec_cache.py +++ b/tests/services/kernelspecs/test_kernelspec_cache.py @@ -6,62 +6,14 @@ import json import os import shutil -import sys -import jupyter_core.paths import pytest from jupyter_client.kernelspec import KernelSpecManager, NoSuchKernel +from pytest_jupyter.utils import mkdir from traitlets.config import Config from jupyter_server.services.kernelspecs.kernelspec_cache import KernelSpecCache - -# BEGIN - Remove once transition to jupyter_server occurs -def mkdir(tmp_path, *parts): - path = tmp_path.joinpath(*parts) - if not path.exists(): - path.mkdir(parents=True) - return path - - -home_dir = pytest.fixture(lambda tmp_path: mkdir(tmp_path, "home")) -data_dir = pytest.fixture(lambda tmp_path: mkdir(tmp_path, "data")) -config_dir = pytest.fixture(lambda tmp_path: mkdir(tmp_path, "config")) -runtime_dir = pytest.fixture(lambda tmp_path: mkdir(tmp_path, "runtime")) -system_jupyter_path = pytest.fixture(lambda tmp_path: mkdir(tmp_path, "share", "jupyter")) -env_jupyter_path = pytest.fixture(lambda tmp_path: mkdir(tmp_path, "env", "share", "jupyter")) -system_config_path = pytest.fixture(lambda tmp_path: mkdir(tmp_path, "etc", "jupyter")) -env_config_path = pytest.fixture(lambda tmp_path: mkdir(tmp_path, "env", "etc", "jupyter")) - - -@pytest.fixture -def environ( - monkeypatch, - tmp_path, - home_dir, - data_dir, - config_dir, - runtime_dir, - system_jupyter_path, - system_config_path, - env_jupyter_path, - env_config_path, -): - monkeypatch.setenv("HOME", str(home_dir)) - monkeypatch.setenv("PYTHONPATH", os.pathsep.join(sys.path)) - monkeypatch.setenv("JUPYTER_NO_CONFIG", "1") - monkeypatch.setenv("JUPYTER_CONFIG_DIR", str(config_dir)) - monkeypatch.setenv("JUPYTER_DATA_DIR", str(data_dir)) - monkeypatch.setenv("JUPYTER_RUNTIME_DIR", str(runtime_dir)) - monkeypatch.setattr(jupyter_core.paths, "SYSTEM_JUPYTER_PATH", [str(system_jupyter_path)]) - monkeypatch.setattr(jupyter_core.paths, "ENV_JUPYTER_PATH", [str(env_jupyter_path)]) - monkeypatch.setattr(jupyter_core.paths, "SYSTEM_CONFIG_PATH", [str(system_config_path)]) - monkeypatch.setattr(jupyter_core.paths, "ENV_CONFIG_PATH", [str(env_config_path)]) - - -# END - Remove once transition to jupyter_server occurs - - kernelspec_json = { "argv": ["cat", "{connection_file}"], "display_name": "Test kernel: {kernel_name}", @@ -88,14 +40,14 @@ def _modify_kernelspec(kernelspec_dir, kernel_name): json.dump(kernel_json, f) -kernelspec_location = pytest.fixture(lambda data_dir: mkdir(data_dir, "kernels")) +kernelspec_location = pytest.fixture(lambda jp_data_dir: mkdir(jp_data_dir, "kernels")) other_kernelspec_location = pytest.fixture( - lambda env_jupyter_path: mkdir(env_jupyter_path, "kernels") + lambda jp_env_jupyter_path: mkdir(jp_env_jupyter_path, "kernels") ) @pytest.fixture -def setup_kernelspecs(environ, kernelspec_location): +def setup_kernelspecs(jp_environ, kernelspec_location): # Only populate factory info _install_kernelspec(str(kernelspec_location), "test1") _install_kernelspec(str(kernelspec_location), "test2") @@ -103,7 +55,7 @@ def setup_kernelspecs(environ, kernelspec_location): @pytest.fixture -def kernel_spec_manager(environ, setup_kernelspecs): +def kernel_spec_manager(jp_environ, setup_kernelspecs): yield KernelSpecManager(ensure_native_kernel=False) @@ -131,7 +83,6 @@ def kernel_spec_cache(request, is_enabled, kernel_spec_manager): config=config, ) yield kspec_cache - kspec_cache = None def get_delay_factor(kernel_spec_cache: KernelSpecCache): @@ -147,17 +98,17 @@ def is_enabled(request): return request.param -async def tests_get_all_specs(kernel_spec_cache): +async def test_get_all_specs(kernel_spec_cache): kspecs = await kernel_spec_cache.get_all_specs() - assert len(kspecs) == 3 + assert len(kspecs) == 4 # The 3 we create, plus the echo kernel that jupyter_core adds -async def tests_get_named_spec(kernel_spec_cache): +async def test_get_named_spec(kernel_spec_cache): kspec = await kernel_spec_cache.get_kernel_spec("test2") assert kspec.display_name == "Test kernel: test2" -async def tests_get_modified_spec(kernel_spec_cache): +async def test_get_modified_spec(kernel_spec_cache): kspec = await kernel_spec_cache.get_kernel_spec("test2") assert kspec.display_name == "Test kernel: test2" @@ -168,7 +119,7 @@ async def tests_get_modified_spec(kernel_spec_cache): assert kspec.display_name == "test2 modified!" -async def tests_add_spec(kernel_spec_cache, kernelspec_location, other_kernelspec_location): +async def test_add_spec(kernel_spec_cache, kernelspec_location, other_kernelspec_location): with pytest.raises(NoSuchKernel): await kernel_spec_cache.get_kernel_spec("added") # this will increment cache_miss @@ -191,7 +142,7 @@ async def tests_add_spec(kernel_spec_cache, kernelspec_location, other_kernelspe assert kernel_spec_cache.cache_misses == (2 if kernel_spec_cache.cache_enabled else 0) -async def tests_remove_spec(kernel_spec_cache): +async def test_remove_spec(kernel_spec_cache): kspec = await kernel_spec_cache.get_kernel_spec("test2") assert kspec.display_name == "Test kernel: test2" @@ -204,7 +155,7 @@ async def tests_remove_spec(kernel_spec_cache): assert kernel_spec_cache.cache_misses == (1 if kernel_spec_cache.cache_enabled else 0) -async def tests_get_missing(kernel_spec_cache): +async def test_get_missing(kernel_spec_cache): with pytest.raises(NoSuchKernel): await kernel_spec_cache.get_kernel_spec("missing") From 5b96176da2b863e9cc8288c81ad2dc33889df115 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Wed, 10 May 2023 08:54:16 -0700 Subject: [PATCH 07/16] Fix handling of parent, create optional dependency on watchdog --- .../services/kernelspecs/kernelspec_cache.py | 17 ++++---- .../kernelspecs/monitors/polling_monitor.py | 4 +- .../kernelspecs/monitors/watchdog_monitor.py | 4 +- pyproject.toml | 6 ++- .../kernelspecs/test_kernelspec_cache.py | 42 +++++++++---------- 5 files changed, 37 insertions(+), 36 deletions(-) diff --git a/jupyter_server/services/kernelspecs/kernelspec_cache.py b/jupyter_server/services/kernelspecs/kernelspec_cache.py index 0f55b19961..09900b7668 100644 --- a/jupyter_server/services/kernelspecs/kernelspec_cache.py +++ b/jupyter_server/services/kernelspecs/kernelspec_cache.py @@ -47,7 +47,7 @@ class KernelSpecCache(LoggingConfigurable): @default("cache_enabled") def _cache_enabled_default(self): - return os.getenv(self.cache_enabled_env, "true").lower() in ("true", "1") + return os.getenv(self.cache_enabled_env, "false").lower() in ("true", "1") kernel_spec_manager = Instance("jupyter_client.kernelspec.KernelSpecManager") @@ -70,7 +70,10 @@ def __init__(self, kernel_spec_manager, **kwargs) -> None: self.kernel_spec_manager = kernel_spec_manager self.kernel_spec_monitor = None if self.cache_enabled: - self.kernel_spec_monitor = KernelSpecMonitorBase.create_instance(parent=self, **kwargs) + # Remove configurable traits that have no bearing on monitors + kwargs.pop("cache_enabled", None) + kwargs.pop("monitor_entry_point", None) + self.kernel_spec_monitor = KernelSpecMonitorBase.create_instance(self, **kwargs) async def get_kernel_spec(self, kernel_name: str) -> KernelSpec: """Get the named kernel specification. @@ -205,18 +208,18 @@ class KernelSpecMonitorBase( # type:ignore[misc] GROUP_NAME = "jupyter_server.kernelspec_monitors" @classmethod - def create_instance(cls, **kwargs) -> "KernelSpecMonitorBase": + def create_instance( + cls, kernel_spec_cache: KernelSpecCache, **kwargs + ) -> "KernelSpecMonitorBase": """Creates an instance of the monitor class configured on the KernelSpecCache instance.""" - kernel_spec_cache = kwargs["parent"] + kernel_spec_cache = kernel_spec_cache entry_point_name = kernel_spec_cache.monitor_entry_point eps = entry_points(group=KernelSpecMonitorBase.GROUP_NAME, name=entry_point_name) if eps: ep: EntryPoint = eps[entry_point_name] monitor_class = ep.load() - monitor_instance: KernelSpecMonitorBase = monitor_class( - parent=kernel_spec_cache, config=kwargs.get("config") - ) + monitor_instance: KernelSpecMonitorBase = monitor_class(kernel_spec_cache, **kwargs) if not isinstance(monitor_instance, KernelSpecMonitorBase): msg = ( f"Entrypoint '{kernel_spec_cache.monitor_entry_point}' of " diff --git a/jupyter_server/services/kernelspecs/monitors/polling_monitor.py b/jupyter_server/services/kernelspecs/monitors/polling_monitor.py index 6a5272e5a4..c9017ea29c 100644 --- a/jupyter_server/services/kernelspecs/monitors/polling_monitor.py +++ b/jupyter_server/services/kernelspecs/monitors/polling_monitor.py @@ -19,10 +19,10 @@ class KernelSpecPollingMonitor(KernelSpecMonitorBase): _pcallback = None - def __init__(self, **kwargs): + def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs): """Initialize the handler.""" super().__init__(**kwargs) - self.kernel_spec_cache: KernelSpecCache = kwargs["parent"] + self.kernel_spec_cache: KernelSpecCache = kernel_spec_cache self.kernel_spec_manager = self.kernel_spec_cache.kernel_spec_manager self.log.info(f"Starting {self.__class__.__name__} with interval: {self.interval} ...") diff --git a/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py b/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py index b8ab1424b1..a596850e98 100644 --- a/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py +++ b/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py @@ -13,10 +13,10 @@ class KernelSpecWatchdogMonitor(KernelSpecMonitorBase): from watchdog.events import FileSystemEventHandler - def __init__(self, **kwargs): + def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs): """Initialize the handler.""" super().__init__(**kwargs) - self.kernel_spec_cache: KernelSpecCache = kwargs["parent"] + self.kernel_spec_cache: KernelSpecCache = kernel_spec_cache self.kernel_spec_manager = self.kernel_spec_cache.kernel_spec_manager self.observed_dirs = set() # Tracks which directories are being watched self.observer = None diff --git a/pyproject.toml b/pyproject.toml index adf280b91a..51dbfc59a7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,7 +44,6 @@ dependencies = [ "tornado>=6.2.0", "traitlets>=5.6.0", "websocket-client", - "watchdog", "jupyter_events>=0.6.0", "overrides" ] @@ -90,13 +89,16 @@ docs = [ # missing typing_extensions "typing_extensions" ] +watchdog-monitor = [ + "watchdog" +] [project.scripts] jupyter-server = "jupyter_server.serverapp:main" [project.entry-points."jupyter_server.kernelspec_monitors"] -watchdog-monitor = "jupyter_server.services.kernelspecs.monitors:KernelSpecWatchdogMonitor" polling-monitor = "jupyter_server.services.kernelspecs.monitors:KernelSpecPollingMonitor" +watchdog-monitor = "jupyter_server.services.kernelspecs.monitors:KernelSpecWatchdogMonitor" [tool.hatch.envs.docs] features = ["docs"] diff --git a/tests/services/kernelspecs/test_kernelspec_cache.py b/tests/services/kernelspecs/test_kernelspec_cache.py index 9448a9ffd3..dee6404776 100644 --- a/tests/services/kernelspecs/test_kernelspec_cache.py +++ b/tests/services/kernelspecs/test_kernelspec_cache.py @@ -8,7 +8,7 @@ import shutil import pytest -from jupyter_client.kernelspec import KernelSpecManager, NoSuchKernel +from jupyter_client.kernelspec import NoSuchKernel from pytest_jupyter.utils import mkdir from traitlets.config import Config @@ -54,35 +54,31 @@ def setup_kernelspecs(jp_environ, kernelspec_location): _install_kernelspec(str(kernelspec_location), "test3") -@pytest.fixture -def kernel_spec_manager(jp_environ, setup_kernelspecs): - yield KernelSpecManager(ensure_native_kernel=False) - - MONITORS = ["watchdog-monitor", "polling-monitor"] @pytest.fixture(params=MONITORS) -def kernel_spec_cache(request, is_enabled, kernel_spec_manager): - config = None - if request.param == "polling-monitor": - config = Config( - { +def kernel_spec_cache( + jp_environ, setup_kernelspecs, request, is_enabled, jp_configurable_serverapp +): + config = Config( + { + "ServerApp": { + "KernelSpecManager": { + "ensure_native_kernel": False, + }, "KernelSpecCache": { - "KernelSpecPollingMonitor": { - "interval": 1.0, - } - } + "cache_enabled": is_enabled, + "monitor_entry_point": request.param, + }, + "KernelSpecPollingMonitor": { + "interval": 1.0 if request.param == "polling-monitor" else 30.0, + }, } - ) - - kspec_cache = KernelSpecCache( - kernel_spec_manager=kernel_spec_manager, - cache_enabled=is_enabled, - monitor_entry_point=request.param, - config=config, + } ) - yield kspec_cache + app = jp_configurable_serverapp(config=config) + yield app.kernel_spec_cache def get_delay_factor(kernel_spec_cache: KernelSpecCache): From 27339f44e53d7c9d1ff488039a611a54861e4994 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Wed, 10 May 2023 09:01:11 -0700 Subject: [PATCH 08/16] Rename trait monitor_entry_point to monitor_name --- .../services/kernelspecs/kernelspec_cache.py | 16 ++++++++-------- .../kernelspecs/test_kernelspec_cache.py | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/jupyter_server/services/kernelspecs/kernelspec_cache.py b/jupyter_server/services/kernelspecs/kernelspec_cache.py index 09900b7668..03f6b2189f 100644 --- a/jupyter_server/services/kernelspecs/kernelspec_cache.py +++ b/jupyter_server/services/kernelspecs/kernelspec_cache.py @@ -51,12 +51,12 @@ def _cache_enabled_default(self): kernel_spec_manager = Instance("jupyter_client.kernelspec.KernelSpecManager") - monitor_entry_point = Unicode( - help="""The monitor entry_point to use to capture kernelspecs updates.""", + monitor_name = Unicode( + help="""The name of the entry_point used to monitor changes to kernelspecs.""", ).tag(config=True) - @default("monitor_entry_point") - def _monitor_entry_point_default(self): + @default("monitor_name") + def _monitor_name_default(self): return "polling-monitor" # The kernelspec cache consists of a dictionary mapping the kernel name to the actual @@ -72,7 +72,7 @@ def __init__(self, kernel_spec_manager, **kwargs) -> None: if self.cache_enabled: # Remove configurable traits that have no bearing on monitors kwargs.pop("cache_enabled", None) - kwargs.pop("monitor_entry_point", None) + kwargs.pop("monitor_name", None) self.kernel_spec_monitor = KernelSpecMonitorBase.create_instance(self, **kwargs) async def get_kernel_spec(self, kernel_name: str) -> KernelSpec: @@ -214,7 +214,7 @@ def create_instance( """Creates an instance of the monitor class configured on the KernelSpecCache instance.""" kernel_spec_cache = kernel_spec_cache - entry_point_name = kernel_spec_cache.monitor_entry_point + entry_point_name = kernel_spec_cache.monitor_name eps = entry_points(group=KernelSpecMonitorBase.GROUP_NAME, name=entry_point_name) if eps: ep: EntryPoint = eps[entry_point_name] @@ -222,7 +222,7 @@ def create_instance( monitor_instance: KernelSpecMonitorBase = monitor_class(kernel_spec_cache, **kwargs) if not isinstance(monitor_instance, KernelSpecMonitorBase): msg = ( - f"Entrypoint '{kernel_spec_cache.monitor_entry_point}' of " + f"Entrypoint '{kernel_spec_cache.monitor_name}' of " f"group '{KernelSpecMonitorBase.GROUP_NAME}' is not a " f"subclass of '{KernelSpecMonitorBase.__name__}'" ) @@ -232,7 +232,7 @@ def create_instance( return monitor_instance else: msg = ( - f"Entrypoint '{kernel_spec_cache.monitor_entry_point}' of " + f"Entrypoint '{kernel_spec_cache.monitor_name}' of " f"group '{KernelSpecMonitorBase.GROUP_NAME}' cannot be located." ) raise RuntimeError(msg) diff --git a/tests/services/kernelspecs/test_kernelspec_cache.py b/tests/services/kernelspecs/test_kernelspec_cache.py index dee6404776..2f23822a50 100644 --- a/tests/services/kernelspecs/test_kernelspec_cache.py +++ b/tests/services/kernelspecs/test_kernelspec_cache.py @@ -69,7 +69,7 @@ def kernel_spec_cache( }, "KernelSpecCache": { "cache_enabled": is_enabled, - "monitor_entry_point": request.param, + "monitor_name": request.param, }, "KernelSpecPollingMonitor": { "interval": 1.0 if request.param == "polling-monitor" else 30.0, @@ -83,7 +83,7 @@ def kernel_spec_cache( def get_delay_factor(kernel_spec_cache: KernelSpecCache): if kernel_spec_cache.cache_enabled: - if kernel_spec_cache.monitor_entry_point == "polling-monitor": + if kernel_spec_cache.monitor_name == "polling-monitor": return 2.0 return 1.0 return 0.5 From 163ac8c01d4afe8f7feddfd46aed407628e55ea8 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Wed, 10 May 2023 09:59:44 -0700 Subject: [PATCH 09/16] Add checksums to polling monitor --- .../kernelspecs/monitors/polling_monitor.py | 32 +++++++++++++++++-- pyproject.toml | 2 ++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/jupyter_server/services/kernelspecs/monitors/polling_monitor.py b/jupyter_server/services/kernelspecs/monitors/polling_monitor.py index c9017ea29c..a7b45d27a8 100644 --- a/jupyter_server/services/kernelspecs/monitors/polling_monitor.py +++ b/jupyter_server/services/kernelspecs/monitors/polling_monitor.py @@ -2,6 +2,9 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. +import json +from hashlib import md5 + from overrides import overrides from traitlets.traitlets import Float @@ -19,11 +22,16 @@ class KernelSpecPollingMonitor(KernelSpecMonitorBase): _pcallback = None + # Keep track of hash values for each entry placed into the cache. This will lessen + # the churn and noise when publishing events + hash_values: dict[str, str] + def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs): """Initialize the handler.""" super().__init__(**kwargs) self.kernel_spec_cache: KernelSpecCache = kernel_spec_cache self.kernel_spec_manager = self.kernel_spec_cache.kernel_spec_manager + self.hash_values = {} self.log.info(f"Starting {self.__class__.__name__} with interval: {self.interval} ...") @overrides @@ -40,9 +48,29 @@ def destroy(self) -> None: self.stop() def poll(self): - self.kernel_spec_cache.remove_all_items() + diff_kernelspecs = {} kernelspecs = self.kernel_spec_manager.get_all_specs() - self.kernel_spec_cache.put_all_items(kernelspecs) + for kernel_name, entry in kernelspecs.items(): + hash_val = md5(json.dumps(entry).encode("utf-8")).hexdigest() + cached_hash_val = self.hash_values.get(kernel_name, "") + if hash_val != cached_hash_val: + diff_kernelspecs[kernel_name] = entry + self.hash_values[kernel_name] = hash_val + + self.log.debug( + f"{self.__class__.__name__} num fetched: {len(kernelspecs.keys())}, " + f"num cached: {len(diff_kernelspecs.keys())}" + ) + self.kernel_spec_cache.put_all_items(diff_kernelspecs) + + # Determine items to remove by calculating what kernelspec names are in the previous + # set and not in the current set + current_set: set = set(kernelspecs.keys()) + previous_set: set = set(self.hash_values.keys()) + to_be_removed = previous_set.difference(current_set) + for kernel_name in to_be_removed: + self.hash_values.pop(kernel_name, None) + self.kernel_spec_cache.remove_item(kernel_name) def start(self): """Start the polling of the kernel.""" diff --git a/pyproject.toml b/pyproject.toml index 51dbfc59a7..9233374465 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -265,6 +265,8 @@ unfixable = [ # S603 `subprocess` call: check for execution of untrusted input "jupyter_server/services/contents/filemanager.py" = ["S603", "S607"] "tests/unix_sockets/test_serverapp_integration.py" = ["S603", "S607"] +# S324 Probable use of insecure hash functions in `hashlib`: `md5` (ok - used as checksum for kernelspec cache) +"jupyter_server/services/kernelspecs/monitors/polling_monitor.py" = ["S324"] [tool.pytest.ini_options] addopts = "-raXs --durations 10 --color=yes --doctest-modules" From bbeb12a95651cec6832e415c5175481859780c77 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Wed, 10 May 2023 10:46:27 -0700 Subject: [PATCH 10/16] Fix cleanup --- .../services/kernelspecs/kernelspec_cache.py | 18 +++++++++++++----- .../kernelspecs/monitors/polling_monitor.py | 6 ++---- .../kernelspecs/monitors/watchdog_monitor.py | 2 +- .../kernelspecs/test_kernelspec_cache.py | 2 ++ 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/jupyter_server/services/kernelspecs/kernelspec_cache.py b/jupyter_server/services/kernelspecs/kernelspec_cache.py index 03f6b2189f..83fceef7d3 100644 --- a/jupyter_server/services/kernelspecs/kernelspec_cache.py +++ b/jupyter_server/services/kernelspecs/kernelspec_cache.py @@ -61,19 +61,28 @@ def _monitor_name_default(self): # The kernelspec cache consists of a dictionary mapping the kernel name to the actual # kernelspec data (CacheItemType). - cache_items: Dict = {} - cache_misses: int = 0 + cache_items: Dict + cache_misses: int def __init__(self, kernel_spec_manager, **kwargs) -> None: """Initialize the cache.""" super().__init__(**kwargs) self.kernel_spec_manager = kernel_spec_manager self.kernel_spec_monitor = None + # initialize cache vars + self.cache_items = {} + self.cache_misses = 0 if self.cache_enabled: # Remove configurable traits that have no bearing on monitors kwargs.pop("cache_enabled", None) kwargs.pop("monitor_name", None) self.kernel_spec_monitor = KernelSpecMonitorBase.create_instance(self, **kwargs) + self.kernel_spec_monitor.initialize() + + def __del__(self): + if self.kernel_spec_monitor is not None: + self.kernel_spec_monitor.destroy() + self.kernel_spec_monitor = None async def get_kernel_spec(self, kernel_name: str) -> KernelSpec: """Get the named kernel specification. @@ -227,9 +236,6 @@ def create_instance( f"subclass of '{KernelSpecMonitorBase.__name__}'" ) raise RuntimeError(msg) - - monitor_instance.initialize() - return monitor_instance else: msg = ( f"Entrypoint '{kernel_spec_cache.monitor_name}' of " @@ -237,6 +243,8 @@ def create_instance( ) raise RuntimeError(msg) + return monitor_instance + @abstractmethod def initialize(self) -> None: """Initializes the monitor.""" diff --git a/jupyter_server/services/kernelspecs/monitors/polling_monitor.py b/jupyter_server/services/kernelspecs/monitors/polling_monitor.py index a7b45d27a8..4933798209 100644 --- a/jupyter_server/services/kernelspecs/monitors/polling_monitor.py +++ b/jupyter_server/services/kernelspecs/monitors/polling_monitor.py @@ -35,10 +35,8 @@ def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs): self.log.info(f"Starting {self.__class__.__name__} with interval: {self.interval} ...") @overrides - def initialize(self): - """Initializes the cache and starts the registers the periodic poller.""" - - # Seed the cache and start the observer + def initialize(self) -> None: + """Initializes the cache and starts the periodic poller.""" if self.kernel_spec_cache.cache_enabled: self.poll() self.start() diff --git a/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py b/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py index a596850e98..0b9de5d07c 100644 --- a/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py +++ b/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py @@ -22,7 +22,7 @@ def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs): self.observer = None @overrides - def initialize(self): + def initialize(self) -> None: """Initializes the cache and starts the observer.""" from watchdog.observers import Observer diff --git a/tests/services/kernelspecs/test_kernelspec_cache.py b/tests/services/kernelspecs/test_kernelspec_cache.py index 2f23822a50..707a2b94bf 100644 --- a/tests/services/kernelspecs/test_kernelspec_cache.py +++ b/tests/services/kernelspecs/test_kernelspec_cache.py @@ -79,6 +79,8 @@ def kernel_spec_cache( ) app = jp_configurable_serverapp(config=config) yield app.kernel_spec_cache + app.kernel_spec_cache = None + app.clear_instance() def get_delay_factor(kernel_spec_cache: KernelSpecCache): From 37a0bf27e2d5729198149a4b0ee2fc2657fb0efa Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Wed, 10 May 2023 13:29:07 -0700 Subject: [PATCH 11/16] Add watchdog to test dependencies --- pyproject.toml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 9233374465..b05d81a4e1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -63,7 +63,9 @@ test = [ "pytest-jupyter[server]>=0.4", "pytest>=7.0", "requests", - "pre-commit" + "pre-commit", + # needed to test kernelspec monitors + "watchdog" ] docs = [ # needed because m2r uses deprecated APIs From c9ec0fc3a17e28f1ab5bfdaad969ba0f6224f2ac Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Wed, 10 May 2023 13:57:48 -0700 Subject: [PATCH 12/16] Add ability to configure traits via envs --- .../services/kernelspecs/kernelspec_cache.py | 6 ++++-- .../kernelspecs/monitors/polling_monitor.py | 13 +++++++++---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/jupyter_server/services/kernelspecs/kernelspec_cache.py b/jupyter_server/services/kernelspecs/kernelspec_cache.py index 83fceef7d3..3f241c6767 100644 --- a/jupyter_server/services/kernelspecs/kernelspec_cache.py +++ b/jupyter_server/services/kernelspecs/kernelspec_cache.py @@ -51,13 +51,15 @@ def _cache_enabled_default(self): kernel_spec_manager = Instance("jupyter_client.kernelspec.KernelSpecManager") + monitor_name_env = "JUPYTER_KERNELSPEC_MONITOR_NAME" monitor_name = Unicode( - help="""The name of the entry_point used to monitor changes to kernelspecs.""", + help="""The name of the entry_point used to monitor changes to kernelspecs. +(JUPYTER_KERNELSPEC_MONITOR_NAME env var)""", ).tag(config=True) @default("monitor_name") def _monitor_name_default(self): - return "polling-monitor" + return os.getenv(self.monitor_name_env, "polling-monitor") # The kernelspec cache consists of a dictionary mapping the kernel name to the actual # kernelspec data (CacheItemType). diff --git a/jupyter_server/services/kernelspecs/monitors/polling_monitor.py b/jupyter_server/services/kernelspecs/monitors/polling_monitor.py index 4933798209..1e82acd798 100644 --- a/jupyter_server/services/kernelspecs/monitors/polling_monitor.py +++ b/jupyter_server/services/kernelspecs/monitors/polling_monitor.py @@ -1,12 +1,12 @@ """KernelSpec watchdog monitor used by KernelspecCache.""" # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. - import json +import os from hashlib import md5 from overrides import overrides -from traitlets.traitlets import Float +from traitlets.traitlets import Float, default from ..kernelspec_cache import KernelSpecCache, KernelSpecMonitorBase @@ -14,12 +14,17 @@ class KernelSpecPollingMonitor(KernelSpecMonitorBase): """Polling monitor that uses a periodic poll period to reload the kernelspec cache.""" + interval_env = "JUPYTER_POLLING_MONITOR_INTERVAL" interval = Float( - default_value=30.0, config=True, - help="""The interval (in seconds) at which kernelspecs are updated in the cache.""", + help="""The interval (in seconds) at which kernelspecs are updated in the cache. +(JUPYTER_POLLING_MONITOR_INTERVAL env var)""", ) + @default("interval") + def _interval_default(self): + return float(os.getenv(self.interval_env, "30.0")) + _pcallback = None # Keep track of hash values for each entry placed into the cache. This will lessen From 1610d4b80c536973f78ac3b0d21b0c4065dd3143 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Wed, 10 May 2023 15:12:19 -0700 Subject: [PATCH 13/16] Fix build (linting and docs) --- ...r_server.services.kernelspecs.monitors.rst | 25 +++ .../jupyter_server.services.kernelspecs.rst | 14 ++ .../kernelspecs/monitors/polling_monitor.py | 3 +- .../kernelspecs/monitors/watchdog_monitor.py | 177 ++++++++---------- pyproject.toml | 4 +- 5 files changed, 127 insertions(+), 96 deletions(-) create mode 100644 docs/source/api/jupyter_server.services.kernelspecs.monitors.rst diff --git a/docs/source/api/jupyter_server.services.kernelspecs.monitors.rst b/docs/source/api/jupyter_server.services.kernelspecs.monitors.rst new file mode 100644 index 0000000000..cfee291d10 --- /dev/null +++ b/docs/source/api/jupyter_server.services.kernelspecs.monitors.rst @@ -0,0 +1,25 @@ +jupyter\_server.services.kernelspecs.monitors package +===================================================== + +Submodules +---------- + + +.. automodule:: jupyter_server.services.kernelspecs.monitors.polling_monitor + :members: + :undoc-members: + :show-inheritance: + + +.. automodule:: jupyter_server.services.kernelspecs.monitors.watchdog_monitor + :members: + :undoc-members: + :show-inheritance: + +Module contents +--------------- + +.. automodule:: jupyter_server.services.kernelspecs.monitors + :members: + :undoc-members: + :show-inheritance: diff --git a/docs/source/api/jupyter_server.services.kernelspecs.rst b/docs/source/api/jupyter_server.services.kernelspecs.rst index 3f210d0f55..7217dd6ef4 100644 --- a/docs/source/api/jupyter_server.services.kernelspecs.rst +++ b/docs/source/api/jupyter_server.services.kernelspecs.rst @@ -1,6 +1,14 @@ jupyter\_server.services.kernelspecs package ============================================ +Subpackages +----------- + +.. toctree:: + :maxdepth: 4 + + jupyter_server.services.kernelspecs.monitors + Submodules ---------- @@ -10,6 +18,12 @@ Submodules :undoc-members: :show-inheritance: + +.. automodule:: jupyter_server.services.kernelspecs.kernelspec_cache + :members: + :undoc-members: + :show-inheritance: + Module contents --------------- diff --git a/jupyter_server/services/kernelspecs/monitors/polling_monitor.py b/jupyter_server/services/kernelspecs/monitors/polling_monitor.py index 1e82acd798..8b631c0bc2 100644 --- a/jupyter_server/services/kernelspecs/monitors/polling_monitor.py +++ b/jupyter_server/services/kernelspecs/monitors/polling_monitor.py @@ -4,6 +4,7 @@ import json import os from hashlib import md5 +from typing import Dict from overrides import overrides from traitlets.traitlets import Float, default @@ -29,7 +30,7 @@ def _interval_default(self): # Keep track of hash values for each entry placed into the cache. This will lessen # the churn and noise when publishing events - hash_values: dict[str, str] + hash_values: Dict[str, str] def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs): """Initialize the handler.""" diff --git a/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py b/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py index 0b9de5d07c..d369731804 100644 --- a/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py +++ b/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py @@ -4,6 +4,8 @@ import os from overrides import overrides +from watchdog.events import FileMovedEvent, FileSystemEventHandler +from watchdog.observers import Observer from ..kernelspec_cache import KernelSpecCache, KernelSpecMonitorBase @@ -11,8 +13,6 @@ class KernelSpecWatchdogMonitor(KernelSpecMonitorBase): """Watchdog handler that filters on specific files deemed representative of a kernel specification.""" - from watchdog.events import FileSystemEventHandler - def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs): """Initialize the handler.""" super().__init__(**kwargs) @@ -24,9 +24,7 @@ def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs): @overrides def initialize(self) -> None: """Initializes the cache and starts the observer.""" - from watchdog.observers import Observer - # Seed the cache and start the observer if self.kernel_spec_cache.cache_enabled: self.observer = Observer() kernelspecs = self.kernel_spec_manager.get_all_specs() @@ -41,11 +39,7 @@ def initialize(self) -> None: ) ) self.observed_dirs.add(kernel_dir) - self.observer.schedule( - KernelSpecWatchdogMonitor.WatchDogHandler(self), - kernel_dir, - recursive=True, - ) + self.observer.schedule(WatchDogHandler(self), kernel_dir, recursive=True) else: self.log.warning( "KernelSpecCache: kernel_dir '{kernel_dir}' does not exist" @@ -57,88 +51,83 @@ def initialize(self) -> None: def destroy(self) -> None: self.observer = None - class WatchDogHandler(FileSystemEventHandler): - # Events related to these files trigger the management of the KernelSpec cache. Should we find - # other files qualify as indicators of a kernel specification's state (like perhaps detached parameter - # files in the future) should be added to this list - at which time it should become configurable. - watched_files = ["kernel.json"] - - def __init__(self, monitor: "KernelSpecWatchdogMonitor", **kwargs): - """Initialize the handler.""" - super().__init__(**kwargs) - self.kernel_spec_cache = monitor.kernel_spec_cache - self.log = monitor.kernel_spec_cache.log - - def dispatch(self, event): - """Dispatches events pertaining to kernelspecs to the appropriate methods. - - - The primary purpose of this method is to ensure the action is occurring against - the a file in the list of watched files and adds some additional attributes to - the event instance to make the actual event handling method easier. - :param event: - The event object representing the file system event. - :type event: - :class:`FileSystemEvent` - """ - from watchdog.events import FileMovedEvent - - if os.path.basename(event.src_path) in self.watched_files: - src_resource_dir = os.path.dirname(event.src_path) - event.src_resource_dir = src_resource_dir - event.src_kernel_name = os.path.basename(src_resource_dir) - if type(event) is FileMovedEvent: - dest_resource_dir = os.path.dirname(event.dest_path) - event.dest_resource_dir = dest_resource_dir - event.dest_kernel_name = os.path.basename(dest_resource_dir) - - super().dispatch(event) - - def on_created(self, event): - """Fires when a watched file is created. - - This will trigger a call to the configured KernelSpecManager to fetch the instance - associated with the created file, which is then added to the cache. - """ - kernel_name = event.src_kernel_name - try: - kernelspec = self.kernel_spec_cache.kernel_spec_manager.get_kernel_spec(kernel_name) - self.kernel_spec_cache.put_item(kernel_name, kernelspec) - except Exception as e: - self.log.warning( - "The following exception occurred creating cache entry for: {src_resource_dir} " - "- continuing... ({e})".format(src_resource_dir=event.src_resource_dir, e=e) - ) - - def on_deleted(self, event): - """Fires when a watched file is deleted, triggering a removal of the corresponding item from the cache.""" - kernel_name = event.src_kernel_name - self.kernel_spec_cache.remove_item(kernel_name) - - def on_modified(self, event): - """Fires when a watched file is modified. - - This will trigger a call to the configured KernelSpecManager to fetch the instance - associated with the modified file, which is then replaced in the cache. - """ - kernel_name = event.src_kernel_name - try: - kernelspec = self.kernel_spec_cache.kernel_spec_manager.get_kernel_spec(kernel_name) - self.kernel_spec_cache.put_item(kernel_name, kernelspec) - except Exception as e: - self.log.warning( - "The following exception occurred updating cache entry for: {src_resource_dir} " - "- continuing... ({e})".format(src_resource_dir=event.src_resource_dir, e=e) - ) - - def on_moved(self, event): - """Fires when a watched file is moved. - - This will trigger the update of the existing cached item, replacing its resource_dir entry - with that of the new destination. - """ - src_kernel_name = event.src_kernel_name - dest_kernel_name = event.dest_kernel_name - cache_item = self.kernel_spec_cache.remove_item(src_kernel_name) - cache_item["resource_dir"] = event.dest_resource_dir - self.kernel_spec_cache.put_item(dest_kernel_name, cache_item) + +class WatchDogHandler(FileSystemEventHandler): + # Events related to these files trigger the management of the KernelSpec cache. Should we find + # other files qualify as indicators of a kernel specification's state (like perhaps detached parameter + # files in the future) should be added to this list - at which time it should become configurable. + watched_files = ["kernel.json"] + + def __init__(self, monitor: "KernelSpecWatchdogMonitor", **kwargs): + """Initialize the handler.""" + super().__init__(**kwargs) + self.kernel_spec_cache = monitor.kernel_spec_cache + self.log = monitor.kernel_spec_cache.log + + def dispatch(self, event): + """Dispatches events pertaining to kernelspecs to the appropriate methods. + + The primary purpose of this method is to ensure the action is occurring against + a file in the list of watched files and adds some additional attributes to + the event instance to make the actual event handling method easier. + """ + + if os.path.basename(event.src_path) in self.watched_files: + src_resource_dir = os.path.dirname(event.src_path) + event.src_resource_dir = src_resource_dir + event.src_kernel_name = os.path.basename(src_resource_dir) + if type(event) is FileMovedEvent: + dest_resource_dir = os.path.dirname(event.dest_path) + event.dest_resource_dir = dest_resource_dir + event.dest_kernel_name = os.path.basename(dest_resource_dir) + + super().dispatch(event) + + def on_created(self, event): + """Fires when a watched file is created. + + This will trigger a call to the configured KernelSpecManager to fetch the instance + associated with the created file, which is then added to the cache. + """ + kernel_name = event.src_kernel_name + try: + kernelspec = self.kernel_spec_cache.kernel_spec_manager.get_kernel_spec(kernel_name) + self.kernel_spec_cache.put_item(kernel_name, kernelspec) + except Exception as e: + self.log.warning( + "The following exception occurred creating cache entry for: {src_resource_dir} " + "- continuing... ({e})".format(src_resource_dir=event.src_resource_dir, e=e) + ) + + def on_deleted(self, event): + """Fires when a watched file is deleted, triggering a removal of the corresponding item from the cache.""" + kernel_name = event.src_kernel_name + self.kernel_spec_cache.remove_item(kernel_name) + + def on_modified(self, event): + """Fires when a watched file is modified. + + This will trigger a call to the configured KernelSpecManager to fetch the instance + associated with the modified file, which is then replaced in the cache. + """ + kernel_name = event.src_kernel_name + try: + kernelspec = self.kernel_spec_cache.kernel_spec_manager.get_kernel_spec(kernel_name) + self.kernel_spec_cache.put_item(kernel_name, kernelspec) + except Exception as e: + self.log.warning( + "The following exception occurred updating cache entry for: {src_resource_dir} " + "- continuing... ({e})".format(src_resource_dir=event.src_resource_dir, e=e) + ) + + def on_moved(self, event): + """Fires when a watched file is moved. + + This will trigger the update of the existing cached item, replacing its resource_dir entry + with that of the new destination. + """ + src_kernel_name = event.src_kernel_name + dest_kernel_name = event.dest_kernel_name + cache_item = self.kernel_spec_cache.remove_item(src_kernel_name) + cache_item["resource_dir"] = event.dest_resource_dir + self.kernel_spec_cache.put_item(dest_kernel_name, cache_item) diff --git a/pyproject.toml b/pyproject.toml index b05d81a4e1..6b493be97d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -89,7 +89,9 @@ docs = [ "tornado", # workaround for an unknown downstream library that is now # missing typing_extensions - "typing_extensions" + "typing_extensions", + # required to build api docs + "watchdog" ] watchdog-monitor = [ "watchdog" From bafcdce318ccd9080c24fd6bb065c690f75137e9 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Thu, 11 May 2023 07:45:37 -0700 Subject: [PATCH 14/16] Fix min versions test by adding conditional dependency --- pyproject.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 6b493be97d..1be3784fb3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,7 +45,8 @@ dependencies = [ "traitlets>=5.6.0", "websocket-client", "jupyter_events>=0.6.0", - "overrides" + "overrides", + "importlib_metadata>=4.8.3;python_version<\"3.10\"", ] [project.urls] From 015a539eb0db2d4e11a072dab4dd0db54947c758 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Thu, 11 May 2023 11:45:36 -0700 Subject: [PATCH 15/16] Address linting test issues --- .../services/kernelspecs/kernelspec_cache.py | 21 ++++---- .../kernelspecs/monitors/polling_monitor.py | 6 +-- .../kernelspecs/monitors/watchdog_monitor.py | 48 +++++++++---------- .../kernelspecs/test_kernelspec_cache.py | 46 +++++++++--------- 4 files changed, 60 insertions(+), 61 deletions(-) diff --git a/jupyter_server/services/kernelspecs/kernelspec_cache.py b/jupyter_server/services/kernelspecs/kernelspec_cache.py index 3f241c6767..754ce9c3ab 100644 --- a/jupyter_server/services/kernelspecs/kernelspec_cache.py +++ b/jupyter_server/services/kernelspecs/kernelspec_cache.py @@ -6,7 +6,7 @@ import os import sys from abc import ABC, ABCMeta, abstractmethod -from typing import Dict, Optional, Union +from typing import Any, Dict, Optional, Union # See compatibility note on `group` keyword in https://docs.python.org/3/library/importlib.metadata.html#entry-points if sys.version_info < (3, 10): # pragma: no cover @@ -14,7 +14,7 @@ else: # pragma: no cover from importlib.metadata import EntryPoint, entry_points -from jupyter_client.kernelspec import KernelSpec +from jupyter_client.kernelspec import KernelSpec, KernelSpecManager from traitlets.config import LoggingConfigurable from traitlets.traitlets import CBool, Instance, Unicode, default @@ -63,10 +63,10 @@ def _monitor_name_default(self): # The kernelspec cache consists of a dictionary mapping the kernel name to the actual # kernelspec data (CacheItemType). - cache_items: Dict + cache_items: Dict[str, CacheItemType] cache_misses: int - def __init__(self, kernel_spec_manager, **kwargs) -> None: + def __init__(self, kernel_spec_manager: KernelSpecManager, **kwargs: Any) -> None: """Initialize the cache.""" super().__init__(**kwargs) self.kernel_spec_manager = kernel_spec_manager @@ -86,7 +86,7 @@ def __del__(self): self.kernel_spec_monitor.destroy() self.kernel_spec_monitor = None - async def get_kernel_spec(self, kernel_name: str) -> KernelSpec: + async def get_kernel_spec(self, kernel_name: str) -> Optional[KernelSpec]: """Get the named kernel specification. This method is equivalent to calling KernelSpecManager.get_kernel_spec(). If @@ -130,7 +130,7 @@ def get_item(self, kernel_name: str) -> Optional[KernelSpec]: If cache is disabled or the item is not in the cache, None is returned; otherwise, a KernelSpec instance of the item is returned. """ - kernelspec = None + kernelspec: Optional[KernelSpec] = None if self.cache_enabled: cache_item = self.cache_items.get(kernel_name.lower()) if cache_item: # Convert to KernelSpec @@ -149,7 +149,7 @@ def get_item(self, kernel_name: str) -> Optional[KernelSpec]: ) return kernelspec - def get_all_items(self) -> Dict[str, CacheItemType]: + def get_all_items(self) -> Dict: # type is Dict[str, CacheItemType] """Retrieves all kernel specification from the cache. If cache is disabled or no items are in the cache, an empty dictionary is returned; @@ -175,9 +175,9 @@ def put_item(self, kernel_name: str, cache_item: Union[KernelSpec, CacheItemType self.log.info(f"KernelSpecCache: adding/updating kernelspec: {kernel_name}") if type(cache_item) is KernelSpec: cache_item = KernelSpecCache.kernel_spec_to_cache_item(cache_item) - self.cache_items[kernel_name.lower()] = cache_item + self.cache_items[kernel_name.lower()] = cache_item # type: ignore - def put_all_items(self, kernelspecs: Dict[str, CacheItemType]) -> None: + def put_all_items(self, kernelspecs: Dict[str, Union[KernelSpec, CacheItemType]]) -> None: """Adds or updates a dictionary of kernel specification in the cache.""" for kernel_name, cache_item in kernelspecs.items(): self.put_item(kernel_name, cache_item) @@ -205,6 +205,7 @@ def kernel_spec_to_cache_item(kernelspec: KernelSpec) -> CacheItemType: @staticmethod def cache_item_to_kernel_spec(cache_item: CacheItemType) -> KernelSpec: """Converts a CacheItemType to a KernelSpec instance for user consumption.""" + assert type(cache_item["spec"]) is dict kernel_spec = KernelSpec(resource_dir=cache_item["resource_dir"], **cache_item["spec"]) return kernel_spec @@ -220,7 +221,7 @@ class KernelSpecMonitorBase( # type:ignore[misc] @classmethod def create_instance( - cls, kernel_spec_cache: KernelSpecCache, **kwargs + cls, kernel_spec_cache: KernelSpecCache, **kwargs: Any ) -> "KernelSpecMonitorBase": """Creates an instance of the monitor class configured on the KernelSpecCache instance.""" diff --git a/jupyter_server/services/kernelspecs/monitors/polling_monitor.py b/jupyter_server/services/kernelspecs/monitors/polling_monitor.py index 8b631c0bc2..ab4bf896f2 100644 --- a/jupyter_server/services/kernelspecs/monitors/polling_monitor.py +++ b/jupyter_server/services/kernelspecs/monitors/polling_monitor.py @@ -4,7 +4,7 @@ import json import os from hashlib import md5 -from typing import Dict +from typing import Any, Dict from overrides import overrides from traitlets.traitlets import Float, default @@ -12,7 +12,7 @@ from ..kernelspec_cache import KernelSpecCache, KernelSpecMonitorBase -class KernelSpecPollingMonitor(KernelSpecMonitorBase): +class KernelSpecPollingMonitor(KernelSpecMonitorBase): # type:ignore[misc] """Polling monitor that uses a periodic poll period to reload the kernelspec cache.""" interval_env = "JUPYTER_POLLING_MONITOR_INTERVAL" @@ -32,7 +32,7 @@ def _interval_default(self): # the churn and noise when publishing events hash_values: Dict[str, str] - def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs): + def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs: Any): """Initialize the handler.""" super().__init__(**kwargs) self.kernel_spec_cache: KernelSpecCache = kernel_spec_cache diff --git a/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py b/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py index d369731804..108868aadb 100644 --- a/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py +++ b/jupyter_server/services/kernelspecs/monitors/watchdog_monitor.py @@ -2,24 +2,25 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. import os +from typing import Any, Set, Tuple from overrides import overrides -from watchdog.events import FileMovedEvent, FileSystemEventHandler +from watchdog.events import FileSystemEventHandler from watchdog.observers import Observer from ..kernelspec_cache import KernelSpecCache, KernelSpecMonitorBase -class KernelSpecWatchdogMonitor(KernelSpecMonitorBase): +class KernelSpecWatchdogMonitor(KernelSpecMonitorBase): # type:ignore[misc] """Watchdog handler that filters on specific files deemed representative of a kernel specification.""" - def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs): + def __init__(self, kernel_spec_cache: KernelSpecCache, **kwargs: Any): """Initialize the handler.""" super().__init__(**kwargs) self.kernel_spec_cache: KernelSpecCache = kernel_spec_cache self.kernel_spec_manager = self.kernel_spec_cache.kernel_spec_manager - self.observed_dirs = set() # Tracks which directories are being watched - self.observer = None + self.observed_dirs: Set[str] = set() # Tracks which directories are being watched + self.observer: Any = None @overrides def initialize(self) -> None: @@ -58,7 +59,7 @@ class WatchDogHandler(FileSystemEventHandler): # files in the future) should be added to this list - at which time it should become configurable. watched_files = ["kernel.json"] - def __init__(self, monitor: "KernelSpecWatchdogMonitor", **kwargs): + def __init__(self, monitor: "KernelSpecWatchdogMonitor", **kwargs: Any): """Initialize the handler.""" super().__init__(**kwargs) self.kernel_spec_cache = monitor.kernel_spec_cache @@ -73,14 +74,6 @@ def dispatch(self, event): """ if os.path.basename(event.src_path) in self.watched_files: - src_resource_dir = os.path.dirname(event.src_path) - event.src_resource_dir = src_resource_dir - event.src_kernel_name = os.path.basename(src_resource_dir) - if type(event) is FileMovedEvent: - dest_resource_dir = os.path.dirname(event.dest_path) - event.dest_resource_dir = dest_resource_dir - event.dest_kernel_name = os.path.basename(dest_resource_dir) - super().dispatch(event) def on_created(self, event): @@ -89,19 +82,18 @@ def on_created(self, event): This will trigger a call to the configured KernelSpecManager to fetch the instance associated with the created file, which is then added to the cache. """ - kernel_name = event.src_kernel_name + resource_dir, kernel_name = WatchDogHandler._extract_info(event.src_path) try: kernelspec = self.kernel_spec_cache.kernel_spec_manager.get_kernel_spec(kernel_name) self.kernel_spec_cache.put_item(kernel_name, kernelspec) except Exception as e: self.log.warning( - "The following exception occurred creating cache entry for: {src_resource_dir} " - "- continuing... ({e})".format(src_resource_dir=event.src_resource_dir, e=e) + f"The following exception occurred creating cache entry for: {resource_dir} - continuing... ({e})" ) def on_deleted(self, event): """Fires when a watched file is deleted, triggering a removal of the corresponding item from the cache.""" - kernel_name = event.src_kernel_name + _, kernel_name = WatchDogHandler._extract_info(event.src_path) self.kernel_spec_cache.remove_item(kernel_name) def on_modified(self, event): @@ -110,14 +102,13 @@ def on_modified(self, event): This will trigger a call to the configured KernelSpecManager to fetch the instance associated with the modified file, which is then replaced in the cache. """ - kernel_name = event.src_kernel_name + resource_dir, kernel_name = WatchDogHandler._extract_info(event.src_path) try: kernelspec = self.kernel_spec_cache.kernel_spec_manager.get_kernel_spec(kernel_name) self.kernel_spec_cache.put_item(kernel_name, kernelspec) except Exception as e: self.log.warning( - "The following exception occurred updating cache entry for: {src_resource_dir} " - "- continuing... ({e})".format(src_resource_dir=event.src_resource_dir, e=e) + f"The following exception occurred updating cache entry for: {resource_dir} - continuing... ({e})" ) def on_moved(self, event): @@ -126,8 +117,15 @@ def on_moved(self, event): This will trigger the update of the existing cached item, replacing its resource_dir entry with that of the new destination. """ - src_kernel_name = event.src_kernel_name - dest_kernel_name = event.dest_kernel_name + _, src_kernel_name = WatchDogHandler._extract_info(event.src_path) + dest_resource_dir, dest_kernel_name = WatchDogHandler._extract_info(event.dest_path) cache_item = self.kernel_spec_cache.remove_item(src_kernel_name) - cache_item["resource_dir"] = event.dest_resource_dir - self.kernel_spec_cache.put_item(dest_kernel_name, cache_item) + if cache_item is not None: + cache_item["resource_dir"] = dest_resource_dir + self.kernel_spec_cache.put_item(dest_kernel_name, cache_item) + + @staticmethod + def _extract_info(dir_name: str) -> Tuple[str, str]: + """Extracts the resource directory and kernel_name from the given dir_name.""" + resource_dir: str = os.path.dirname(dir_name) # includes kernel_name + return resource_dir, os.path.basename(resource_dir) diff --git a/tests/services/kernelspecs/test_kernelspec_cache.py b/tests/services/kernelspecs/test_kernelspec_cache.py index 707a2b94bf..12d1aea857 100644 --- a/tests/services/kernelspecs/test_kernelspec_cache.py +++ b/tests/services/kernelspecs/test_kernelspec_cache.py @@ -4,12 +4,11 @@ import asyncio import json -import os import shutil +from pathlib import Path import pytest from jupyter_client.kernelspec import NoSuchKernel -from pytest_jupyter.utils import mkdir from traitlets.config import Config from jupyter_server.services.kernelspecs.kernelspec_cache import KernelSpecCache @@ -20,38 +19,39 @@ } -def _install_kernelspec(kernels_dir, kernel_name): +def _install_kernelspec(kernels_dir: Path, kernel_name: str) -> Path: """install a sample kernel in a kernels directory""" - kernelspec_dir = os.path.join(kernels_dir, kernel_name) - os.makedirs(kernelspec_dir) - json_file = os.path.join(kernelspec_dir, "kernel.json") + kernelspec_dir = kernels_dir / kernel_name + kernelspec_dir.mkdir(parents=True) + json_file = Path(kernelspec_dir) / "kernel.json" named_json = kernelspec_json.copy() - named_json["display_name"] = named_json["display_name"].format(kernel_name=kernel_name) - with open(json_file, "w") as f: + named_json["display_name"] = str(named_json["display_name"]).format(kernel_name=kernel_name) + with open(str(json_file), "w") as f: json.dump(named_json, f) return kernelspec_dir -def _modify_kernelspec(kernelspec_dir, kernel_name): - json_file = os.path.join(kernelspec_dir, "kernel.json") +def _modify_kernelspec(kernelspec_dir: str, kernel_name: str) -> None: + json_file = Path(kernelspec_dir) / "kernel.json" kernel_json = kernelspec_json.copy() kernel_json["display_name"] = f"{kernel_name} modified!" - with open(json_file, "w") as f: + with open(str(json_file), "w") as f: json.dump(kernel_json, f) -kernelspec_location = pytest.fixture(lambda jp_data_dir: mkdir(jp_data_dir, "kernels")) -other_kernelspec_location = pytest.fixture( - lambda jp_env_jupyter_path: mkdir(jp_env_jupyter_path, "kernels") -) +@pytest.fixture +def other_kernelspec_location(jp_env_jupyter_path): + other_location = Path(jp_env_jupyter_path) / "kernels" + other_location.mkdir() + return other_location @pytest.fixture -def setup_kernelspecs(jp_environ, kernelspec_location): +def setup_kernelspecs(jp_environ, jp_kernel_dir): # Only populate factory info - _install_kernelspec(str(kernelspec_location), "test1") - _install_kernelspec(str(kernelspec_location), "test2") - _install_kernelspec(str(kernelspec_location), "test3") + _install_kernelspec(jp_kernel_dir, "test1") + _install_kernelspec(jp_kernel_dir, "test2") + _install_kernelspec(jp_kernel_dir, "test3") MONITORS = ["watchdog-monitor", "polling-monitor"] @@ -83,7 +83,7 @@ def kernel_spec_cache( app.clear_instance() -def get_delay_factor(kernel_spec_cache: KernelSpecCache): +def get_delay_factor(kernel_spec_cache: KernelSpecCache) -> float: if kernel_spec_cache.cache_enabled: if kernel_spec_cache.monitor_name == "polling-monitor": return 2.0 @@ -117,11 +117,11 @@ async def test_get_modified_spec(kernel_spec_cache): assert kspec.display_name == "test2 modified!" -async def test_add_spec(kernel_spec_cache, kernelspec_location, other_kernelspec_location): +async def test_add_spec(kernel_spec_cache, jp_kernel_dir, other_kernelspec_location): with pytest.raises(NoSuchKernel): await kernel_spec_cache.get_kernel_spec("added") # this will increment cache_miss - _install_kernelspec(str(other_kernelspec_location), "added") + _install_kernelspec(other_kernelspec_location, "added") # this will increment cache_miss prior to load kspec = await kernel_spec_cache.get_kernel_spec("added") @@ -130,7 +130,7 @@ async def test_add_spec(kernel_spec_cache, kernelspec_location, other_kernelspec assert kernel_spec_cache.cache_misses == (2 if kernel_spec_cache.cache_enabled else 0) # Add another to an existing observed directory, no cache miss here - _install_kernelspec(str(kernelspec_location), "added2") + _install_kernelspec(jp_kernel_dir, "added2") await asyncio.sleep( get_delay_factor(kernel_spec_cache) ) # sleep to allow cache to add item (no cache miss in this case) From f65e4af283b4afb49d1ae9a27e5a213c2701aca2 Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Thu, 11 May 2023 12:18:52 -0700 Subject: [PATCH 16/16] Increase watchdog delay on Windows --- tests/services/kernelspecs/test_kernelspec_cache.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/services/kernelspecs/test_kernelspec_cache.py b/tests/services/kernelspecs/test_kernelspec_cache.py index 12d1aea857..5f27d357a9 100644 --- a/tests/services/kernelspecs/test_kernelspec_cache.py +++ b/tests/services/kernelspecs/test_kernelspec_cache.py @@ -5,6 +5,7 @@ import asyncio import json import shutil +import sys from pathlib import Path import pytest @@ -71,12 +72,13 @@ def kernel_spec_cache( "cache_enabled": is_enabled, "monitor_name": request.param, }, - "KernelSpecPollingMonitor": { - "interval": 1.0 if request.param == "polling-monitor" else 30.0, - }, } } ) + # Increase polling frequency to avoid long test delays + if request.param == "polling-monitor" and is_enabled: + config["ServerApp"]["KernelSpecPollingMonitor"]["interval"] = 1.0 + app = jp_configurable_serverapp(config=config) yield app.kernel_spec_cache app.kernel_spec_cache = None @@ -87,7 +89,9 @@ def get_delay_factor(kernel_spec_cache: KernelSpecCache) -> float: if kernel_spec_cache.cache_enabled: if kernel_spec_cache.monitor_name == "polling-monitor": return 2.0 - return 1.0 + elif kernel_spec_cache.monitor_name == "watchdog-monitor": + # watchdog on Windows appears to be a bit slower + return 2.0 if sys.platform.startswith("win") else 1.0 return 0.5