From 07874f6ca8ec856579f47bf363f8c1329c9a2401 Mon Sep 17 00:00:00 2001 From: Mahendra Paipuri Date: Mon, 13 May 2024 17:39:55 +0200 Subject: [PATCH 1/4] fix: Remove vestigial code that overrides copy method * Copying files is broken with Jlab 4 due to seemingly vestigial code * Add packages to pyproject.toml as package name does not match with directory name Signed-off-by: Mahendra Paipuri --- jupyterfs/metamanager.py | 55 ---------------------------------------- pyproject.toml | 3 +++ 2 files changed, 3 insertions(+), 55 deletions(-) diff --git a/jupyterfs/metamanager.py b/jupyterfs/metamanager.py index 97792ac..c99f86a 100644 --- a/jupyterfs/metamanager.py +++ b/jupyterfs/metamanager.py @@ -25,9 +25,6 @@ path_second_arg, path_kwarg, path_old_new, - getDrive, - isDrive, - stripDrive, ) __all__ = ["MetaManager", "MetaManagerHandler"] @@ -153,58 +150,6 @@ def root_manager(self): def root_dir(self): return self.root_manager.root_dir - async 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. - If to_path is a directory, filename will increment `from_path-Copy#.ext`. - Considering multi-part extensions, the Copy# part will be placed before the first dot for all the extensions except `ipynb`. - For easier manual searching in case of notebooks, the Copy# part will be placed before the last dot. - - from_path must be a full path to a file. - """ - path = from_path.strip("/") - if to_path is not None: - to_path = to_path.strip("/") - - if "/" in path: - from_dir, from_name = path.rsplit("/", 1) - else: - from_dir = "" - from_name = path - - model = self.get(path) - model.pop("path", None) - model.pop("name", None) - if model["type"] == "directory": - raise web.HTTPError(400, "Can't copy directories") - if to_path is None: - to_path = from_dir - if self.dir_exists(to_path): - name = self.copy_pat.sub(".", stripDrive(from_name)) - # ensure that any drives are stripped from the resulting filename - to_name = self.increment_filename(name, to_path, insert="-Copy") - # separate path and filename with a slash if to_path is not just a drive string - to_path = ("{0}{1}" if isDrive(to_path) else "{0}/{1}").format( - to_path, to_name - ) - - model = self.save(model, to_path) - return model - - def _getManagerForPath(self, path): - drive = getDrive(path) - mgr = self._managers.get(drive) - if mgr is None: - raise web.HTTPError( - 404, - "Couldn't find manager {mgrName} for {path}".format( - mgrName=drive, path=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) diff --git a/pyproject.toml b/pyproject.toml index bfeef50..5e73b97 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -98,6 +98,9 @@ artifacts = [ "jupyterfs/labextension" ] +[tool.hatch.build.targets.wheel] +packages = ["jupyterfs"] + [tool.hatch.build.targets.wheel.shared-data] "jupyterfs/labextension" = "share/jupyter/labextensions/jupyter-fs" "jupyterfs/labextension/schemas/jupyter-fs/plugin.json" = "share/jupyter/lab/schemas/jupyter-fs/plugin.json" From 45e755dcfc5e8ba46fdd69415aa5e65a770dfb03 Mon Sep 17 00:00:00 2001 From: Tim Paine <3105306+timkpaine@users.noreply.github.com> Date: Thu, 23 May 2024 11:16:32 +0200 Subject: [PATCH 2/4] Move from docker-compose to docker compose --- .github/workflows/build.yml | 4 ++-- Makefile | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 376aff4..db3a342 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -59,7 +59,7 @@ jobs: run: make develop - name: Setup Linux testing infra - run: make setup-infra-ubuntu DOCKER=docker + run: make setup-infra-ubuntu DOCKER_COMPOSE="docker compose" if: ${{ matrix.os == 'ubuntu-latest' }} - name: Setup Mac testing infra @@ -85,7 +85,7 @@ jobs: if: ${{ matrix.os == 'ubuntu-latest' }} - name: Teardown Linux testing infra - run: make teardown-infra-ubuntu DOCKER=docker + run: make teardown-infra-ubuntu DOCKER_COMPOSE="docker compose" if: ${{ matrix.os == 'ubuntu-latest' }} - name: Teardown Mac testing infra diff --git a/Makefile b/Makefile index fdcd4c7..8bf7b23 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -DOCKER := podman +DOCKER_COMPOSE := podman-compose YARN := jlpm ############### @@ -9,7 +9,7 @@ build: ## build python/javascript python -m build . develop: ## install to site-packages in editable mode - python -m pip install --upgrade build docker-compose jupyterlab pip setuptools twine wheel + python -m pip install --upgrade build jupyterlab pip setuptools twine wheel python -m pip install -vvv .[develop] install: ## install to site-packages @@ -40,13 +40,13 @@ teardown-infra-win: teardown-infra-common: dockerup: - ${DOCKER}-compose -f ci/docker-compose.yml up -d + ${DOCKER_COMPOSE} -f ci/docker-compose.yml up -d dockerdown: - ${DOCKER}-compose -f ci/docker-compose.yml down || echo "can't teardown docker compose" + ${DOCKER_COMPOSE} -f ci/docker-compose.yml down || echo "can't teardown docker compose" dockerlogs: - ${DOCKER}-compose -f ci/docker-compose.yml logs + ${DOCKER_COMPOSE} -f ci/docker-compose.yml logs testpy: ## Clean and Make unit tests python -m pytest -v jupyterfs/tests --junitxml=junit.xml --cov=jupyterfs --cov-report=xml:.coverage.xml --cov-branch --cov-fail-under=20 --cov-report term-missing From 720e7f4ae8c13ce8a0d0355413d961ea083de24a Mon Sep 17 00:00:00 2001 From: Tim Paine <3105306+timkpaine@users.noreply.github.com> Date: Thu, 23 May 2024 11:37:48 +0200 Subject: [PATCH 3/4] Move from black to ruff format, pin ruff versions --- Makefile | 12 ++++++------ pyproject.toml | 5 ++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 8bf7b23..a413863 100644 --- a/Makefile +++ b/Makefile @@ -62,20 +62,20 @@ tests: testpy testjs ## run the tests ########### .PHONY: lintpy lintjs lint fixpy fixjs fix format -lintpy: ## Black/flake8 python +lintpy: ## Lint Python with Ruff python -m ruff jupyterfs setup.py - python -m black --check jupyterfs setup.py + python -m ruff format --check jupyterfs setup.py -lintjs: ## ESlint javascript +lintjs: ## Lint Javascript with ESlint cd js; ${YARN} lint lint: lintpy lintjs ## run linter -fixpy: ## Black python +fixpy: ## Autoformat Python with Ruff python -m ruff jupyterfs setup.py --fix - python -m black jupyterfs/ setup.py + python -m ruff format jupyterfs/ setup.py -fixjs: ## ESlint Autofix JS +fixjs: ## Autoformat JavaScript with ESlint cd js; ${YARN} fix fix: fixpy fixjs ## run black/tslint fix diff --git a/pyproject.toml b/pyproject.toml index 5e73b97..b7f17a8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -61,10 +61,9 @@ develop = [ "pytest-asyncio", "pytest-jupyter[server]", # Other deps - "black>=23", "bump2version>=1.0.0", "check-manifest", - "ruff", + "ruff>=0.3.5,<0.4", ] test = [ @@ -134,5 +133,5 @@ npm = "jlpm" [tool.ruff] line-length = 150 -[tool.ruff.per-file-ignores] +[tool.ruff.lint.per-file-ignores] "__init__.py" = ["F401"] From 99ee0890ebbfafdfcf5a0969f73c88b022836c2b Mon Sep 17 00:00:00 2001 From: Tim Paine <3105306+timkpaine@users.noreply.github.com> Date: Thu, 23 May 2024 11:38:11 +0200 Subject: [PATCH 4/4] Run autoformatting --- Makefile | 3 +- jupyterfs/auth.py | 10 ++----- jupyterfs/config.py | 16 +++-------- jupyterfs/extension.py | 13 ++------- jupyterfs/fsmanager.py | 44 ++++++++--------------------- jupyterfs/metamanager.py | 14 ++------- jupyterfs/pathutils.py | 12 ++------ jupyterfs/tests/test_fsmanager.py | 12 ++------ jupyterfs/tests/test_init.py | 4 +-- jupyterfs/tests/test_metamanager.py | 12 ++------ 10 files changed, 34 insertions(+), 106 deletions(-) diff --git a/Makefile b/Makefile index a413863..ec4a8ea 100644 --- a/Makefile +++ b/Makefile @@ -63,7 +63,7 @@ tests: testpy testjs ## run the tests .PHONY: lintpy lintjs lint fixpy fixjs fix format lintpy: ## Lint Python with Ruff - python -m ruff jupyterfs setup.py + python -m ruff check jupyterfs setup.py python -m ruff format --check jupyterfs setup.py lintjs: ## Lint Javascript with ESlint @@ -72,7 +72,6 @@ lintjs: ## Lint Javascript with ESlint lint: lintpy lintjs ## run linter fixpy: ## Autoformat Python with Ruff - python -m ruff jupyterfs setup.py --fix python -m ruff format jupyterfs/ setup.py fixjs: ## Autoformat JavaScript with ESlint diff --git a/jupyterfs/auth.py b/jupyterfs/auth.py index a4e2b77..b514976 100644 --- a/jupyterfs/auth.py +++ b/jupyterfs/auth.py @@ -50,11 +50,7 @@ def get_identifiers(self): if named is not None and named not in ids: # add a named group only the first time it appears ids.append(named) - elif ( - named is None - and mo.group("invalid") is None - and mo.group("escaped") is None - ): + elif named is None and mo.group("invalid") is None and mo.group("escaped") is None: # If all the groups are None, there must be # another group we're not expecting raise ValueError("Unrecognized named group in pattern", self.pattern) @@ -75,9 +71,7 @@ def stdin_prompt(url): def substituteAsk(resource): if "tokenDict" in resource: - url = DoubleBraceTemplate(resource["url"]).safe_substitute( - {k: urllib.parse.quote(v) for k, v in resource.pop("tokenDict").items()} - ) + url = DoubleBraceTemplate(resource["url"]).safe_substitute({k: urllib.parse.quote(v) for k, v in resource.pop("tokenDict").items()}) else: url = resource["url"] diff --git a/jupyterfs/config.py b/jupyterfs/config.py index be12ad1..b773156 100644 --- a/jupyterfs/config.py +++ b/jupyterfs/config.py @@ -18,9 +18,7 @@ class JupyterFs(Configurable): root_manager_class = Type( config=True, default_value=LargeFileManager, - help=_i18n( - "the root contents manager class to use. Used by the Jupyterlab default filebrowser and elsewhere" - ), + help=_i18n("the root contents manager class to use. Used by the Jupyterlab default filebrowser and elsewhere"), klass=ContentsManager, ) @@ -40,9 +38,7 @@ class JupyterFs(Configurable): resource_validators = List( config=True, trait=Unicode(), - help=_i18n( - "regular expressions to match against resource URLs. At least one must match" - ), + help=_i18n("regular expressions to match against resource URLs. At least one must match"), ) surface_init_errors = Bool( @@ -56,9 +52,7 @@ class JupyterFs(Configurable): per_key_traits=Dict( { "label": Unicode(help="The designator to show to users"), - "caption": Unicode( - "", help="An optional, longer description to show to users" - ), + "caption": Unicode("", help="An optional, longer description to show to users"), "pattern": Unicode( "", help="A regular expression to match against the full URL of the entry, indicating if this snippet is valid for it", @@ -66,7 +60,5 @@ class JupyterFs(Configurable): "template": Unicode(help="A template string to build up the snippet"), } ), - help=_i18n( - "per entry snippets for how to use it, e.g. a snippet for how to open a file from a given resource" - ), + help=_i18n("per entry snippets for how to use it, e.g. a snippet for how to open a file from a given resource"), ) diff --git a/jupyterfs/extension.py b/jupyterfs/extension.py index 9414a94..0282928 100644 --- a/jupyterfs/extension.py +++ b/jupyterfs/extension.py @@ -41,19 +41,12 @@ def _load_jupyter_server_extension(serverapp): warnings.warn(_mm_config_warning_msg) return - if isinstance(serverapp.contents_manager_class, type) and not issubclass( - serverapp.contents_manager_class, MetaManager - ): + if isinstance(serverapp.contents_manager_class, type) and not issubclass(serverapp.contents_manager_class, MetaManager): serverapp.contents_manager_class = MetaManager - serverapp.log.info( - "Configuring jupyter-fs manager as the content manager class" - ) + serverapp.log.info("Configuring jupyter-fs manager as the content manager class") resources_url = "jupyterfs/resources" - serverapp.log.info( - "Installing jupyter-fs resources handler on path %s" - % url_path_join(base_url, resources_url) - ) + serverapp.log.info("Installing jupyter-fs resources handler on path %s" % url_path_join(base_url, resources_url)) web_app.add_handlers( host_pattern, [ diff --git a/jupyterfs/fsmanager.py b/jupyterfs/fsmanager.py index 7d3081e..38ffbab 100644 --- a/jupyterfs/fsmanager.py +++ b/jupyterfs/fsmanager.py @@ -176,9 +176,7 @@ def _is_path_hidden(self, path, info): import os syspath = self._pyfilesystem_instance.getsyspath(path) - if os.path.exists(syspath) and not os.access( - syspath, os.X_OK | os.R_OK - ): + if os.path.exists(syspath) and not os.access(syspath, os.X_OK | os.R_OK): return True except ResourceNotFound: @@ -313,14 +311,10 @@ def _dir_model(self, path, info, content=True): if content: model["content"] = contents = [] - for dir_entry in self._pyfilesystem_instance.scandir( - path, namespaces=("basic", "access", "details", "stat") - ): + for dir_entry in self._pyfilesystem_instance.scandir(path, namespaces=("basic", "access", "details", "stat")): try: if self.should_list(dir_entry.name): - if self.allow_hidden or not self._is_path_hidden( - dir_entry.make_path(path), dir_entry - ): + if self.allow_hidden or not self._is_path_hidden(dir_entry.make_path(path), dir_entry): contents.append( self.get( path="%s/%s" % (path, dir_entry.name), @@ -335,9 +329,7 @@ def _dir_model(self, path, info, content=True): # us from listing other entries pass except Exception as e: - self.log.warning( - "Error stat-ing %s: %s", dir_entry.make_path(path), e - ) + self.log.warning("Error stat-ing %s: %s", dir_entry.make_path(path), e) model["format"] = "json" return model @@ -443,25 +435,19 @@ def get(self, path, content=True, type=None, format=None, info=None): # gather info - by doing here can minimise further network requests from underlying fs functions if not info: try: - info = self._pyfilesystem_instance.getinfo( - path, namespaces=("basic", "stat", "access", "details") - ) + info = self._pyfilesystem_instance.getinfo(path, namespaces=("basic", "stat", "access", "details")) except Exception: raise web.HTTPError(404, "No such file or directory: %s" % path) if info.is_dir: if type not in (None, "directory"): - raise web.HTTPError( - 400, "%s is a directory, not a %s" % (path, type), reason="bad type" - ) + raise web.HTTPError(400, "%s is a directory, not a %s" % (path, type), reason="bad type") model = self._dir_model(path, content=content, info=info) elif type == "notebook" or (type is None and path.endswith(".ipynb")): model = self._notebook_model(path, content=content, info=info) else: if type == "directory": - raise web.HTTPError( - 400, "%s is not a directory" % path, reason="bad type" - ) + raise web.HTTPError(400, "%s is not a directory" % path, reason="bad type") model = self._file_model(path, content=content, format=format, info=info) return model @@ -519,9 +505,7 @@ def save(self, model, path=""): if chunk and model["type"] != "file": raise web.HTTPError( 400, - 'File type "{}" is not supported for chunked transfer'.format( - model["type"] - ), + 'File type "{}" is not supported for chunked transfer'.format(model["type"]), ) self.log.debug("Saving %s", path) @@ -549,9 +533,7 @@ def save(self, model, path=""): raise except Exception as e: self.log.error("Error while saving file: %s %s", path, e, exc_info=True) - raise web.HTTPError( - 500, "Unexpected error while saving file: %s %s" % (path, e) - ) + raise web.HTTPError(500, "Unexpected error while saving file: %s %s" % (path, e)) validation_message = None if model["type"] == "notebook": @@ -603,9 +585,7 @@ def rename_file(self, old_path, new_path): with self.perm_to_403(new_path): # Should we proceed with the move? - if self._pyfilesystem_instance.exists( - new_path - ): # TODO and not samefile(old_os_path, new_os_path): + if self._pyfilesystem_instance.exists(new_path): # TODO and not samefile(old_os_path, new_os_path): raise web.HTTPError(409, "File already exists: %s" % new_path) # Move the file or directory @@ -620,9 +600,7 @@ def rename_file(self, old_path, new_path): except web.HTTPError: raise except Exception as e: - raise web.HTTPError( - 500, "Unknown error renaming file: %s %s" % (old_path, e) - ) + raise web.HTTPError(500, "Unknown error renaming file: %s %s" % (old_path, e)) class PyFilesystemCheckpoints(GenericFileCheckpoints): diff --git a/jupyterfs/metamanager.py b/jupyterfs/metamanager.py index c99f86a..37a30b1 100644 --- a/jupyterfs/metamanager.py +++ b/jupyterfs/metamanager.py @@ -45,9 +45,7 @@ def __init__(self, **kwargs): self._pyfs_kw = {} self.resources = [] - self._default_root_manager = self._jupyterfsConfig.root_manager_class( - **self._kwargs - ) + self._default_root_manager = self._jupyterfsConfig.root_manager_class(**self._kwargs) self._managers = dict((("", self._default_root_manager),)) # copy kwargs to pyfs_kw, removing kwargs not relevant to pyfs @@ -133,11 +131,7 @@ def initResource(self, *resources, options={}): self._managers = managers if verbose: - print( - "jupyter-fs initialized: {} file system resources, {} managers".format( - len(self.resources), len(self._managers) - ) - ) + print("jupyter-fs initialized: {} file system resources, {} managers".format(len(self.resources), len(self._managers))) return self.resources @@ -242,6 +236,4 @@ async def post(self): else: resources = valid_resources - self.finish( - json.dumps(self.contents_manager.initResource(*resources, options=options)) - ) + self.finish(json.dumps(self.contents_manager.initResource(*resources, options=options))) diff --git a/jupyterfs/pathutils.py b/jupyterfs/pathutils.py index 65b8be7..ad3187d 100644 --- a/jupyterfs/pathutils.py +++ b/jupyterfs/pathutils.py @@ -48,9 +48,7 @@ def _resolve_path(path, manager_dict): raise HTTPError( 404, - "Couldn't find manager {mgrName} for {path}".format( - mgrName=parts[0], path=path - ), + "Couldn't find manager {mgrName} for {path}".format(mgrName=parts[0], path=path), ) else: # Try to find a sub-manager for the first subdirectory. @@ -60,9 +58,7 @@ def _resolve_path(path, manager_dict): raise HTTPError( 404, - "Couldn't find manager {mgrName} for {path}".format( - mgrName=parts[0], path=path - ), + "Couldn't find manager {mgrName} for {path}".format(mgrName=parts[0], path=path), ) @@ -160,9 +156,7 @@ async def _wrapper(self, old_path, new_path, *args, **kwargs): ), ) assert new_prefix == old_prefix - result = getattr(new_mgr, method_name)( - old_mgr_path, new_mgr_path, *args, **kwargs - ) + result = getattr(new_mgr, method_name)(old_mgr_path, new_mgr_path, *args, **kwargs) return result return _wrapper diff --git a/jupyterfs/tests/test_fsmanager.py b/jupyterfs/tests/test_fsmanager.py index 419bb65..1cbe69e 100644 --- a/jupyterfs/tests/test_fsmanager.py +++ b/jupyterfs/tests/test_fsmanager.py @@ -104,11 +104,7 @@ async def test_write_read(self, jp_fetch, resource_uri, jp_server_config): assert test_content == (await cc.get(p))["content"] for p in hidden_paths: - ctx = ( - nullcontext() - if allow_hidden - else pytest.raises(tornado.httpclient.HTTPClientError) - ) + ctx = nullcontext() if allow_hidden else pytest.raises(tornado.httpclient.HTTPClientError) with ctx as c: # save to root and tips await cc.save(p, _test_file_model) @@ -155,8 +151,7 @@ def setup_class(cls): cls._rootDirUtil.delete() @classmethod - def teardown_class(cls): - ... + def teardown_class(cls): ... def setup_method(self, method): self._rootDirUtil.create() @@ -212,8 +207,7 @@ def setup_class(cls): cls._rootDirUtil.delete() @classmethod - def teardown_class(cls): - ... + def teardown_class(cls): ... def setup_method(self, method): # create a root diff --git a/jupyterfs/tests/test_init.py b/jupyterfs/tests/test_init.py index 38a12f5..dfd597b 100644 --- a/jupyterfs/tests/test_init.py +++ b/jupyterfs/tests/test_init.py @@ -35,6 +35,4 @@ def test_open_fs(self, mock_getpass, mock_fs_open_fs): open_fs("osfs://{{foo}}/bar.txt") mock_getpass.assert_called_with("Enter value for 'foo': ") - mock_fs_open_fs.assert_called_with( - "osfs://test%20return%20getpass%20%3C%3E/%7C/bar.txt" - ) + mock_fs_open_fs.assert_called_with("osfs://test%20return%20getpass%20%3C%3E/%7C/bar.txt") diff --git a/jupyterfs/tests/test_metamanager.py b/jupyterfs/tests/test_metamanager.py index cd16f1a..8d5f89c 100644 --- a/jupyterfs/tests/test_metamanager.py +++ b/jupyterfs/tests/test_metamanager.py @@ -57,21 +57,15 @@ def jp_server_config(tmp_path, tmp_osfs_resource, our_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) - resources = await cc.set_resources( - [{"name": "test-2", "url": f"osfs://{tmp_path.as_posix()}"}] - ) + resources = await cc.set_resources([{"name": "test-2", "url": f"osfs://{tmp_path.as_posix()}"}]) assert resources == [] @pytest.mark.parametrize("our_config", [deny_client_config]) @pytest.mark.parametrize("tmp_osfs_resource", [True]) -async def test_client_creation_disallowed_retains_server_config( - tmp_path, jp_fetch, jp_server_config -): +async def test_client_creation_disallowed_retains_server_config(tmp_path, jp_fetch, jp_server_config): cc = ContentsClient(jp_fetch) - resources = await cc.set_resources( - [{"name": "test-2", "url": f"osfs://{tmp_path.as_posix()}"}] - ) + resources = await cc.set_resources([{"name": "test-2", "url": f"osfs://{tmp_path.as_posix()}"}]) names = set(map(lambda r: r["name"], resources)) assert names == {"test-server-config"}