Skip to content

Commit

Permalink
Make preferred_dir content manager trait
Browse files Browse the repository at this point in the history
Move the preferred_dir trait to content manager, but keep the old one for backwards compatibility. The new location should also cause the value to be read later in the init, allowing us to avoid the deferred validation. Also fixes an issue with escaping the root dir on Windows if an absolute path is passed.
  • Loading branch information
vidartf committed Nov 25, 2022
1 parent 5280377 commit 01e7d2e
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 95 deletions.
53 changes: 5 additions & 48 deletions jupyter_server/serverapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -1641,22 +1641,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 @@ -1673,39 +1662,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 @@ -1887,6 +1845,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 @@ -2086,7 +2047,7 @@ def init_resources(self):
)
return

old_soft, old_hard = resource.getrlimit(resource.RLIMIT_NOFILE)
old_soft, old_hard = resource.getrlimit(resource.RLIMIT_NOFILE) # noqa
soft = self.min_open_files_limit
hard = old_hard
if old_soft < soft:
Expand All @@ -2097,7 +2058,7 @@ def init_resources(self):
old_soft, soft, old_hard, hard
)
)
resource.setrlimit(resource.RLIMIT_NOFILE, (soft, hard))
resource.setrlimit(resource.RLIMIT_NOFILE, (soft, hard)) # noqa

def _get_urlparts(self, path=None, include_token=False):
"""Constructs a urllib named tuple, ParseResult,
Expand Down Expand Up @@ -2505,10 +2466,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
# initialize io loop as early as possible,
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
24 changes: 24 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 Down Expand Up @@ -55,6 +57,28 @@ 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 ContentsManager.preferred_dir with a relative path 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)
return "/"

@default("checkpoints_class")
def _checkpoints_class_default(self):
return FileCheckpoints
Expand Down
21 changes: 21 additions & 0 deletions jupyter_server/services/contents/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import warnings
from fnmatch import fnmatch

from jupyter_client.utils import run_sync
from jupyter_events import EventLogger
from nbformat import ValidationError, sign
from nbformat import validate as validate_nb
Expand Down Expand Up @@ -75,10 +76,30 @@ def emit(self, data):

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

preferred_dir = Unicode(
"/",
config=True,
help=_i18n(
"Preferred starting directory to use for notebooks, relative to the server root dir."
),
)

@validate("preferred_dir")
def _validate_preferred_dir(self, proposal):
value = proposal["value"]
try:
dir_exists = run_sync(self.dir_exists)(value)
except HTTPError as e:
raise TraitError(e.log_message) from e
if not dir_exists:
raise TraitError(_i18n("Preferred directory not found: %r") % value)
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
6 changes: 3 additions & 3 deletions tests/test_gateway.py
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ async def test_gateway_session_lifecycle(init_gateway, jp_root_dir, 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 @@ -583,12 +583,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

0 comments on commit 01e7d2e

Please sign in to comment.