Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport PR #1162: Reapply preferred_dir fix, now with better backwards compatability #1167

Merged
merged 6 commits into from
Jan 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 3 additions & 46 deletions jupyter_server/serverapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -1629,22 +1629,11 @@ def _normalize_dir(self, value):
value = os.path.abspath(value)
return value

# Because the validation of preferred_dir depends on root_dir and validation
# occurs when the trait is loaded, there are times when we should defer the
# validation of preferred_dir (e.g., when preferred_dir is defined via CLI
# and root_dir is defined via a config file).
_defer_preferred_dir_validation = False

@validate("root_dir")
def _root_dir_validate(self, proposal):
value = self._normalize_dir(proposal["value"])
if not os.path.isdir(value):
raise TraitError(trans.gettext("No such directory: '%r'") % value)

if self._defer_preferred_dir_validation:
# If we're here, then preferred_dir is configured on the CLI and
# root_dir is configured in a file
self._preferred_dir_validation(self.preferred_dir, value)
return value

preferred_dir = Unicode(
Expand All @@ -1661,39 +1650,8 @@ def _preferred_dir_validate(self, proposal):
value = self._normalize_dir(proposal["value"])
if not os.path.isdir(value):
raise TraitError(trans.gettext("No such preferred dir: '%r'") % value)

# Before we validate against root_dir, check if this trait is defined on the CLI
# and root_dir is not. If that's the case, we'll defer it's further validation
# until root_dir is validated or the server is starting (the latter occurs when
# the default root_dir (cwd) is used).
cli_config = self.cli_config.get("ServerApp", {})
if "preferred_dir" in cli_config and "root_dir" not in cli_config:
self._defer_preferred_dir_validation = True

if not self._defer_preferred_dir_validation: # Validate now
self._preferred_dir_validation(value, self.root_dir)
return value

def _preferred_dir_validation(self, preferred_dir: str, root_dir: str) -> None:
"""Validate preferred dir relative to root_dir - preferred_dir must be equal or a subdir of root_dir"""
if not preferred_dir.startswith(root_dir):
raise TraitError(
trans.gettext(
"preferred_dir must be equal or a subdir of root_dir. preferred_dir: '%r' root_dir: '%r'"
)
% (preferred_dir, root_dir)
)
self._defer_preferred_dir_validation = False

@observe("root_dir")
def _root_dir_changed(self, change):
self._root_dir_set = True
if not self.preferred_dir.startswith(change["new"]):
self.log.warning(
trans.gettext("Value of preferred_dir updated to use value of root_dir")
)
self.preferred_dir = change["new"]

@observe("server_extensions")
def _update_server_extensions(self, change):
self.log.warning(_i18n("server_extensions is deprecated, use jpserver_extensions"))
Expand Down Expand Up @@ -1868,6 +1826,9 @@ def init_configurables(self):
parent=self,
log=self.log,
)
# Trigger a default/validation here explicitly while we still support the
# deprecated trait on ServerApp (FIXME remove when deprecation finalized)
self.contents_manager.preferred_dir
self.session_manager = self.session_manager_class(
parent=self,
log=self.log,
Expand Down Expand Up @@ -2432,10 +2393,6 @@ def initialize(
# Parse command line, load ServerApp config files,
# and update ServerApp config.
super().initialize(argv=argv)
if self._defer_preferred_dir_validation:
# If we're here, then preferred_dir is configured on the CLI and
# root_dir has the default value (cwd)
self._preferred_dir_validation(self.preferred_dir, self.root_dir)
if self._dispatching:
return
# Then, use extensions' config loading mechanism to
Expand Down
3 changes: 3 additions & 0 deletions jupyter_server/services/contents/fileio.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,9 @@ def _get_os_path(self, path):
404: if path is outside root
"""
root = os.path.abspath(self.root_dir)
# to_os_path is not safe if path starts with a drive, since os.path.join discards first part
if os.path.splitdrive(path)[0]:
raise HTTPError(404, "%s is not a relative API path" % path)
os_path = to_os_path(path, root)
if not (os.path.abspath(os_path) + os.path.sep).startswith(root):
raise HTTPError(404, "%s is outside root contents directory" % path)
Expand Down
31 changes: 31 additions & 0 deletions jupyter_server/services/contents/filemanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
import shutil
import stat
import sys
import warnings
from datetime import datetime
from pathlib import Path

import nbformat
from anyio.to_thread import run_sync
Expand All @@ -19,6 +21,7 @@
from jupyter_server import _tz as tz
from jupyter_server.base.handlers import AuthenticatedFileHandler
from jupyter_server.transutils import _i18n
from jupyter_server.utils import to_api_path

from .filecheckpoints import AsyncFileCheckpoints, FileCheckpoints
from .fileio import AsyncFileManagerMixin, FileManagerMixin
Expand Down Expand Up @@ -55,6 +58,34 @@ def _validate_root_dir(self, proposal):
raise TraitError("%r is not a directory" % value)
return value

@default("preferred_dir")
def _default_preferred_dir(self):
try:
value = self.parent.preferred_dir
if value == self.parent.root_dir:
value = None
except AttributeError:
pass
else:
if value is not None:
warnings.warn(
"ServerApp.preferred_dir config is deprecated in jupyter-server 2.0. Use FileContentsManager.preferred_dir instead",
FutureWarning,
stacklevel=3,
)
try:
path = Path(value)
return path.relative_to(self.root_dir).as_posix()
except ValueError:
raise TraitError("%s is outside root contents directory" % value) from None
return ""

@validate("preferred_dir")
def _validate_preferred_dir(self, proposal):
# It should be safe to pass an API path through this method:
proposal["value"] = to_api_path(proposal["value"], self.root_dir)
return super()._validate_preferred_dir(proposal)

@default("checkpoints_class")
def _checkpoints_class_default(self):
return FileCheckpoints
Expand Down
32 changes: 32 additions & 0 deletions jupyter_server/services/contents/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
# Distributed under the terms of the Modified BSD License.
import itertools
import json
import os
import re
import warnings
from fnmatch import fnmatch

from jupyter_client.utils import run_sync
from nbformat import ValidationError, sign
from nbformat import validate as validate_nb
from nbformat.v4 import new_notebook
Expand Down Expand Up @@ -55,10 +57,40 @@ class ContentsManager(LoggingConfigurable):

root_dir = Unicode("/", config=True)

preferred_dir = Unicode(
"",
config=True,
help=_i18n(
"Preferred starting directory to use for notebooks. This is an API path (`/` separated, relative to root dir)"
),
)

@validate("preferred_dir")
def _validate_preferred_dir(self, proposal):
value = proposal["value"].strip("/")
try:
import inspect

if inspect.isawaitable(self.dir_exists):
blink1073 marked this conversation as resolved.
Show resolved Hide resolved
dir_exists = run_sync(self.dir_exists)(value)
else:
dir_exists = self.dir_exists(value)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conditional probably needs to be forward-ported again.

except HTTPError as e:
raise TraitError(e.log_message) from e
if not dir_exists:
raise TraitError(_i18n("Preferred directory not found: %r") % value)
try:
if value != self.parent.preferred_dir:
self.parent.preferred_dir = os.path.join(self.root_dir, *value.split("/"))
except (AttributeError, TraitError):
pass
return value

allow_hidden = Bool(False, config=True, help="Allow access to hidden files")

notary = Instance(sign.NotebookNotary)

@default("notary")
def _notary_default(self):
return sign.NotebookNotary(parent=self)

Expand Down
10 changes: 5 additions & 5 deletions tests/test_gateway.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ async def test_gateway_class_mappings(init_gateway, jp_serverapp):
assert jp_serverapp.kernel_spec_manager_class.__name__ == "GatewayKernelSpecManager"


async def test_gateway_get_kernelspecs(init_gateway, jp_fetch):
async def test_gateway_get_kernelspecs(init_gateway, jp_serverapp, jp_fetch):
# Validate that kernelspecs come from gateway.
with mocked_gateway:
r = await jp_fetch("api", "kernelspecs", method="GET")
Expand All @@ -297,11 +297,11 @@ async def test_gateway_get_named_kernelspec(init_gateway, jp_fetch):
assert expected_http_error(e, 404)


async def test_gateway_session_lifecycle(init_gateway, jp_root_dir, jp_fetch):
async def test_gateway_session_lifecycle(init_gateway, jp_fetch):
# Validate session lifecycle functions; create and delete.

# create
session_id, kernel_id = await create_session(jp_root_dir, jp_fetch, "kspec_foo")
session_id, kernel_id = await create_session(jp_fetch, "kspec_foo")

# ensure kernel still considered running
assert await is_kernel_running(jp_fetch, kernel_id) is True
Expand Down Expand Up @@ -447,12 +447,12 @@ async def test_channel_queue_get_msg_when_response_router_had_finished():
#
# Test methods below...
#
async def create_session(root_dir, jp_fetch, kernel_name):
async def create_session(jp_fetch, kernel_name):
"""Creates a session for a kernel. The session is created against the server
which then uses the gateway for kernel management.
"""
with mocked_gateway:
nb_path = root_dir / "testgw.ipynb"
nb_path = "/testgw.ipynb"
body = json.dumps(
{"path": str(nb_path), "type": "notebook", "kernel": {"name": kernel_name}}
)
Expand Down
Loading