Skip to content

Commit

Permalink
Re-add sync contents manager, fixes #179, fixes #181
Browse files Browse the repository at this point in the history
  • Loading branch information
timkpaine committed Nov 18, 2023
1 parent e7ea3ce commit c0748f5
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 47 deletions.
6 changes: 3 additions & 3 deletions jupyterfs/extension.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from jupyter_server.utils import url_path_join

from ._version import __version__ # noqa: F401
from .metamanager import MetaManager, MetaManagerHandler
from .metamanager import MetaManagerShared, MetaManager, MetaManagerHandler
from .snippets import SnippetsHandler

_mm_config_warning_msg = """Misconfiguration of MetaManager. Please add:
Expand All @@ -37,12 +37,12 @@ def _load_jupyter_server_extension(serverapp):
base_url = web_app.settings["base_url"]
host_pattern = ".*$"

if not isinstance(serverapp.contents_manager, MetaManager):
if not isinstance(serverapp.contents_manager, MetaManagerShared):
warnings.warn(_mm_config_warning_msg)
return

if isinstance(serverapp.contents_manager_class, type) and not issubclass(
serverapp.contents_manager_class, MetaManager
serverapp.contents_manager_class, MetaManagerShared
):
serverapp.contents_manager_class = MetaManager
serverapp.log.info(
Expand Down
70 changes: 50 additions & 20 deletions jupyterfs/metamanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
from fs.errors import FSError
from fs.opener.errors import OpenerError, ParseError
from jupyter_server.base.handlers import APIHandler
from jupyter_server.services.contents.manager import AsyncContentsManager
from jupyter_server.services.contents.manager import (
AsyncContentsManager,
ContentsManager,
)

from .auth import substituteAsk, substituteEnv, substituteNone
from .config import JupyterFs as JupyterFsConfig
Expand All @@ -30,10 +33,10 @@
stripDrive,
)

__all__ = ["MetaManager", "MetaManagerHandler"]
__all__ = ["MetaManager", "SyncMetaManager", "MetaManagerHandler"]


class MetaManager(AsyncContentsManager):
class MetaManagerShared:
copy_pat = re.compile(r"\-Copy\d*\.")

@default("files_handler_params")
Expand Down Expand Up @@ -153,7 +156,7 @@ def root_manager(self):
def root_dir(self):
return self.root_manager.root_dir

async def copy(self, from_path, to_path=None):
def copy(self, from_path, to_path=None):
"""Copy an existing file and return its new model.
If to_path not specified, it will be the parent directory of from_path.
Expand Down Expand Up @@ -205,30 +208,57 @@ def _getManagerForPath(self, path):

return mgr, stripDrive(path)

is_hidden = path_first_arg("is_hidden", False)
dir_exists = path_first_arg("dir_exists", False)
file_exists = path_kwarg("file_exists", "", False)
exists = path_first_arg("exists", False)
is_hidden = path_first_arg("is_hidden", False, sync=False)
dir_exists = path_first_arg("dir_exists", False, sync=False)
file_exists = path_kwarg("file_exists", "", False, sync=False)
exists = path_first_arg("exists", False, sync=False)

save = path_second_arg("save", "model", True, sync=False)
rename = path_old_new("rename", False, sync=False)

get = path_first_arg("get", True, sync=False)
delete = path_first_arg("delete", False, sync=False)

get_kernel_path = path_first_arg("get_kernel_path", False, sync=True)

create_checkpoint = path_first_arg("create_checkpoint", False, sync=False)
list_checkpoints = path_first_arg("list_checkpoints", False, sync=False)
restore_checkpoint = path_second_arg(
"restore_checkpoint", "checkpoint_id", False, sync=False
)
delete_checkpoint = path_second_arg(
"delete_checkpoint", "checkpoint_id", False, sync=False
)


class SyncMetaManager(MetaManagerShared, ContentsManager):
...


class MetaManager(MetaManagerShared, AsyncContentsManager):
async def copy(self, from_path, to_path=None):
return super(MetaManagerShared, self).copy(from_path=from_path, to_path=to_path)

Check warning on line 240 in jupyterfs/metamanager.py

View check run for this annotation

Codecov / codecov/patch

jupyterfs/metamanager.py#L240

Added line #L240 was not covered by tests

is_hidden = path_first_arg("is_hidden", False, sync=False)
dir_exists = path_first_arg("dir_exists", False, sync=False)
file_exists = path_kwarg("file_exists", "", False, sync=False)
exists = path_first_arg("exists", False, sync=False)

save = path_second_arg("save", "model", True)
rename = path_old_new("rename", False)
save = path_second_arg("save", "model", True, sync=False)
rename = path_old_new("rename", False, sync=False)

get = path_first_arg("get", True)
delete = path_first_arg("delete", False)
get = path_first_arg("get", True, sync=False)
delete = path_first_arg("delete", False, sync=False)

get_kernel_path = path_first_arg("get_kernel_path", False, sync=True)

create_checkpoint = path_first_arg("create_checkpoint", False)
list_checkpoints = path_first_arg("list_checkpoints", False)
create_checkpoint = path_first_arg("create_checkpoint", False, sync=False)
list_checkpoints = path_first_arg("list_checkpoints", False, sync=False)
restore_checkpoint = path_second_arg(
"restore_checkpoint",
"checkpoint_id",
False,
"restore_checkpoint", "checkpoint_id", False, sync=False
)
delete_checkpoint = path_second_arg(
"delete_checkpoint",
"checkpoint_id",
False,
"delete_checkpoint", "checkpoint_id", False, sync=False
)


Expand Down
60 changes: 37 additions & 23 deletions jupyterfs/pathutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,61 +93,69 @@ def path_first_arg(method_name, returns_model, sync=False):
"""Decorator for methods that accept path as a first argument,
e.g. manager.get(path, ...)"""

if sync:

def _wrapper(self, *args, **kwargs):
path, args = _get_arg("path", args, kwargs)
_, mgr, mgr_path = _resolve_path(path, self._managers)
result = getattr(mgr, method_name)(mgr_path, *args, **kwargs)
return result
def _wrapper(self, *args, **kwargs):
path, args = _get_arg("path", args, kwargs)
_, mgr, mgr_path = _resolve_path(path, self._managers)
result = getattr(mgr, method_name)(mgr_path, *args, **kwargs)
return result

else:
if sync:
return _wrapper

async def _wrapper(self, *args, **kwargs):
path, args = _get_arg("path", args, kwargs)
_, mgr, mgr_path = _resolve_path(path, self._managers)
result = getattr(mgr, method_name)(mgr_path, *args, **kwargs)
return result
async def _wrapper2(self, *args, **kwargs):
return _wrapper(self, *args, **kwargs)

return _wrapper
return _wrapper2


def path_second_arg(method_name, first_argname, returns_model):
def path_second_arg(method_name, first_argname, returns_model, sync=False):
"""Decorator for methods that accept path as a second argument.
e.g. manager.save(model, path, ...)"""

async def _wrapper(self, *args, **kwargs):
def _wrapper(self, *args, **kwargs):
other, args = _get_arg(first_argname, args, kwargs)
path, args = _get_arg("path", args, kwargs)
_, mgr, mgr_path = _resolve_path(path, self._managers)
result = getattr(mgr, method_name)(other, mgr_path, *args, **kwargs)
return result

return _wrapper
if sync:
return _wrapper

Check warning on line 123 in jupyterfs/pathutils.py

View check run for this annotation

Codecov / codecov/patch

jupyterfs/pathutils.py#L123

Added line #L123 was not covered by tests

async def _wrapper2(self, *args, **kwargs):
return _wrapper(self, *args, **kwargs)

return _wrapper2


def path_kwarg(method_name, path_default, returns_model):
def path_kwarg(method_name, path_default, returns_model, sync=False):
"""Parameterized decorator for methods that accept path as a second
argument.
e.g. manager.file_exists(path='')
"""

async def _wrapper(self, path=path_default, **kwargs):
def _wrapper(self, path=path_default, **kwargs):
_, mgr, mgr_path = _resolve_path(path, self._managers)
result = getattr(mgr, method_name)(path=mgr_path, **kwargs)
return result

return _wrapper
if sync:
return _wrapper

Check warning on line 144 in jupyterfs/pathutils.py

View check run for this annotation

Codecov / codecov/patch

jupyterfs/pathutils.py#L144

Added line #L144 was not covered by tests

async def _wrapper2(self, *args, **kwargs):
return _wrapper(self, *args, **kwargs)

return _wrapper2

def path_old_new(method_name, returns_model):

def path_old_new(method_name, returns_model, sync=False):
"""Decorator for methods accepting old_path and new_path.
e.g. manager.rename(old_path, new_path)
"""

async def _wrapper(self, old_path, new_path, *args, **kwargs):
def _wrapper(self, old_path, new_path, *args, **kwargs):
old_prefix, old_mgr, old_mgr_path = _resolve_path(old_path, self._managers)
new_prefix, new_mgr, new_mgr_path = _resolve_path(new_path, self._managers)
if old_mgr is not new_mgr:
Expand All @@ -165,7 +173,13 @@ async def _wrapper(self, old_path, new_path, *args, **kwargs):
)
return result

return _wrapper
if sync:
return _wrapper

Check warning on line 177 in jupyterfs/pathutils.py

View check run for this annotation

Codecov / codecov/patch

jupyterfs/pathutils.py#L177

Added line #L177 was not covered by tests

async def _wrapper2(self, *args, **kwargs):
return _wrapper(self, *args, **kwargs)

Check warning on line 180 in jupyterfs/pathutils.py

View check run for this annotation

Codecov / codecov/patch

jupyterfs/pathutils.py#L180

Added line #L180 was not covered by tests

return _wrapper2


# handlers for drive specifications in path strings, as in "fooDrive:bar/baz.buzz"
Expand Down
14 changes: 13 additions & 1 deletion jupyterfs/tests/test_metamanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@
"JupyterFs": {},
}

sync_base_config = {
"ServerApp": {
"jpserver_extensions": {"jupyterfs.extension": True},
"contents_manager_class": "jupyterfs.metamanager.SyncMetaManager",
},
"JupyterFs": {},
}

deny_client_config = {
"JupyterFs": {
"allow_user_resources": False,
Expand All @@ -40,7 +48,7 @@ def our_config():


@pytest.fixture
def jp_server_config(tmp_path, tmp_osfs_resource, our_config):
def jp_server_config(base_config, tmp_path, tmp_osfs_resource, our_config):
c = Config(base_config)
c.JupyterFs.setdefault("resources", [])
if tmp_osfs_resource:
Expand All @@ -54,6 +62,7 @@ def jp_server_config(tmp_path, tmp_osfs_resource, our_config):
return c


@pytest.mark.parametrize("base_config", [base_config, sync_base_config])
@pytest.mark.parametrize("our_config", [deny_client_config])
async def test_client_creation_disallowed(tmp_path, jp_fetch, jp_server_config):
cc = ContentsClient(jp_fetch)
Expand All @@ -63,6 +72,7 @@ async def test_client_creation_disallowed(tmp_path, jp_fetch, jp_server_config):
assert resources == []


@pytest.mark.parametrize("base_config", [base_config, sync_base_config])
@pytest.mark.parametrize("our_config", [deny_client_config])
@pytest.mark.parametrize("tmp_osfs_resource", [True])
async def test_client_creation_disallowed_retains_server_config(
Expand All @@ -76,6 +86,7 @@ async def test_client_creation_disallowed_retains_server_config(
assert names == {"test-server-config"}


@pytest.mark.parametrize("base_config", [base_config, sync_base_config])
@pytest.mark.parametrize(
"our_config",
[
Expand Down Expand Up @@ -124,6 +135,7 @@ async def test_resource_validators(tmp_path, jp_fetch, jp_server_config):
assert names == {"valid-1", "valid-2"}


@pytest.mark.parametrize("base_config", [base_config, sync_base_config])
@pytest.mark.parametrize(
"our_config",
[
Expand Down

0 comments on commit c0748f5

Please sign in to comment.