Skip to content

Commit

Permalink
Clean up handling of synchronous managers (jupyter-server#1044)
Browse files Browse the repository at this point in the history
  • Loading branch information
blink1073 authored Nov 3, 2022
1 parent 4b2ac37 commit efba15a
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 43 deletions.
12 changes: 6 additions & 6 deletions jupyter_server/pytest_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
from jupyter_server.auth import Authorizer
from jupyter_server.extension import serverextension
from jupyter_server.serverapp import JUPYTER_SERVICE_HANDLERS, ServerApp
from jupyter_server.services.contents.filemanager import FileContentsManager
from jupyter_server.services.contents.largefilemanager import LargeFileManager
from jupyter_server.services.contents.filemanager import AsyncFileContentsManager
from jupyter_server.services.contents.largefilemanager import AsyncLargeFileManager
from jupyter_server.utils import url_path_join

# List of dependencies needed for this plugin.
Expand Down Expand Up @@ -488,14 +488,14 @@ def jp_kernelspecs(jp_data_dir):

@pytest.fixture(params=[True, False])
def jp_contents_manager(request, tmp_path):
"""Returns a FileContentsManager instance based on the use_atomic_writing parameter value."""
return FileContentsManager(root_dir=str(tmp_path), use_atomic_writing=request.param)
"""Returns an AsyncFileContentsManager instance based on the use_atomic_writing parameter value."""
return AsyncFileContentsManager(root_dir=str(tmp_path), use_atomic_writing=request.param)


@pytest.fixture
def jp_large_contents_manager(tmp_path):
"""Returns a LargeFileManager instance."""
return LargeFileManager(root_dir=str(tmp_path))
"""Returns an AsyncLargeFileManager instance."""
return AsyncLargeFileManager(root_dir=str(tmp_path))


@pytest.fixture
Expand Down
53 changes: 20 additions & 33 deletions jupyter_server/serverapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import gettext
import hashlib
import hmac
import inspect
import ipaddress
import json
import logging
Expand Down Expand Up @@ -57,8 +56,8 @@
if not sys.platform.startswith("win"):
from tornado.netutil import bind_unix_socket

from jupyter_client import KernelManager
from jupyter_client.kernelspec import KernelSpecManager
from jupyter_client.manager import KernelManager
from jupyter_client.session import Session
from jupyter_core.application import JupyterApp, base_aliases, base_flags
from jupyter_core.paths import jupyter_runtime_dir
Expand Down Expand Up @@ -123,7 +122,7 @@
AsyncFileContentsManager,
FileContentsManager,
)
from jupyter_server.services.contents.largefilemanager import LargeFileManager
from jupyter_server.services.contents.largefilemanager import AsyncLargeFileManager
from jupyter_server.services.contents.manager import (
AsyncContentsManager,
ContentsManager,
Expand All @@ -133,7 +132,6 @@
MappingKernelManager,
)
from jupyter_server.services.sessions.sessionmanager import SessionManager
from jupyter_server.traittypes import TypeFromClasses
from jupyter_server.utils import (
check_pid,
fetch,
Expand Down Expand Up @@ -1164,7 +1162,7 @@ def _deprecated_password_config(self, change):
config=True,
help="""Disable cross-site-request-forgery protection
Jupyter notebook 4.3.1 introduces protection from cross-site request forgeries,
Jupyter server includes protection from cross-site request forgeries,
requiring API requests to either:
- originate from pages served by this server (validated with XSRF cookie and token), or
Expand Down Expand Up @@ -1435,38 +1433,13 @@ def template_file_path(self):
help="""If True, display controls to shut down the Jupyter server, such as menu items or buttons.""",
)

# REMOVE in VERSION 2.0
# Temporarily allow content managers to inherit from the 'notebook'
# package. We will deprecate this in the next major release.
contents_manager_class = TypeFromClasses(
default_value=LargeFileManager,
klasses=[
"jupyter_server.services.contents.manager.ContentsManager",
"notebook.services.contents.manager.ContentsManager",
],
contents_manager_class = Type(
default_value=AsyncLargeFileManager,
klass=ContentsManager,
config=True,
help=_i18n("The content manager class to use."),
)

# Throws a deprecation warning to notebook based contents managers.
@observe("contents_manager_class")
def _observe_contents_manager_class(self, change):
new = change["new"]
# If 'new' is a class, get a string representing the import
# module path.
if inspect.isclass(new):
new = new.__module__

if new.startswith("notebook"):
self.log.warning(
"The specified 'contents_manager_class' class inherits a manager from the "
"'notebook' package. This is not guaranteed to work in future "
"releases of Jupyter Server. Instead, consider switching the "
"manager to inherit from the 'jupyter_server' managers. "
"Jupyter Server will temporarily allow 'notebook' managers "
"until its next major release (2.x)."
)

kernel_manager_class = Type(
klass=MappingKernelManager,
config=True,
Expand Down Expand Up @@ -1865,6 +1838,20 @@ def init_configurables(self):
# this determination, instantiate the GatewayClient config singleton.
self.gateway_config = GatewayClient.instance(parent=self)

if not issubclass(self.kernel_manager_class, AsyncMappingKernelManager):
warnings.warn(
"The synchronous MappingKernelManager class is deprecated and will not be supported in Jupyter Server 3.0",
DeprecationWarning,
stacklevel=2,
)

if not issubclass(self.contents_manager_class, AsyncContentsManager):
warnings.warn(
"The synchronous ContentsManager classes are deprecated and will not be supported in Jupyter Server 3.0",
DeprecationWarning,
stacklevel=2,
)

self.kernel_spec_manager = self.kernel_spec_manager_class(
parent=self,
)
Expand Down
2 changes: 1 addition & 1 deletion jupyter_server/services/contents/filemanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ async def save(self, model, path=""):
"""Save the file model and return the model with no content."""
path = path.strip("/")

self.run_pre_save_hook(model=model, path=path)
self.run_pre_save_hooks(model=model, path=path)

if "type" not in model:
raise web.HTTPError(400, "No file type provided")
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ filterwarnings = [
"error",
"ignore:Passing a schema to Validator.iter_errors:DeprecationWarning",
"ignore:run_pre_save_hook is deprecated:DeprecationWarning",

"always:unclosed <socket.socket:ResourceWarning",
"module:Jupyter is migrating its paths to use standard platformdirs:DeprecationWarning",
]

Expand Down
12 changes: 12 additions & 0 deletions tests/services/contents/test_api.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import json
import pathlib
import sys
import warnings
from base64 import decodebytes, encodebytes
from unicodedata import normalize

Expand All @@ -14,6 +15,17 @@
from ...utils import expected_http_error


@pytest.fixture(autouse=True)
def suppress_deprecation_warnings():
with warnings.catch_warnings():
warnings.filterwarnings(
"ignore",
message="The synchronous ContentsManager",
category=DeprecationWarning,
)
yield


def notebooks_only(dir_model):
return [nb for nb in dir_model["content"] if nb["type"] == "notebook"]

Expand Down
4 changes: 2 additions & 2 deletions tests/services/contents/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@

from jupyter_server.services.contents.checkpoints import AsyncCheckpoints
from jupyter_server.services.contents.filecheckpoints import (
AsyncFileCheckpoints,
AsyncGenericFileCheckpoints,
GenericFileCheckpoints,
)
from jupyter_server.services.contents.manager import AsyncContentsManager


@pytest.fixture(params=[AsyncGenericFileCheckpoints, GenericFileCheckpoints])
@pytest.fixture(params=[AsyncGenericFileCheckpoints, AsyncFileCheckpoints])
def jp_server_config(request):
return {"FileContentsManager": {"checkpoints_class": request.param}}

Expand Down
12 changes: 12 additions & 0 deletions tests/services/kernels/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import json
import os
import time
import warnings

import jupyter_client
import pytest
Expand All @@ -16,6 +17,17 @@
TEST_TIMEOUT = 60


@pytest.fixture(autouse=True)
def suppress_deprecation_warnings():
with warnings.catch_warnings():
warnings.filterwarnings(
"ignore",
message="The synchronous MappingKernelManager",
category=DeprecationWarning,
)
yield


@pytest.fixture
def pending_kernel_is_ready(jp_serverapp):
async def _(kernel_id, ready=None):
Expand Down
12 changes: 12 additions & 0 deletions tests/services/kernels/test_cull.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import json
import os
import platform
import warnings

import jupyter_client
import pytest
Expand All @@ -12,6 +13,17 @@
CULL_INTERVAL = 1


@pytest.fixture(autouse=True)
def suppress_deprecation_warnings():
with warnings.catch_warnings():
warnings.filterwarnings(
"ignore",
message="The synchronous MappingKernelManager",
category=DeprecationWarning,
)
yield


@pytest.mark.parametrize(
"jp_server_config",
[
Expand Down
12 changes: 12 additions & 0 deletions tests/services/sessions/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import os
import shutil
import time
import warnings
from typing import Any

import jupyter_client
Expand All @@ -22,6 +23,17 @@
TEST_TIMEOUT = 60


@pytest.fixture(autouse=True)
def suppress_deprecation_warnings():
with warnings.catch_warnings():
warnings.filterwarnings(
"ignore",
message="The synchronous MappingKernelManager",
category=DeprecationWarning,
)
yield


def j(r):
return json.loads(r.body.decode())

Expand Down

0 comments on commit efba15a

Please sign in to comment.