Merge pull request #200 from mahendrapaipuri/fix_copy

Fix copy file operation
jpmorganchase · May 23, 2024 · 1a84b01 · 1a84b01
2 parents e7ea3ce + 99ee089
commit 1a84b01
Show file tree

Hide file tree

Showing 12 changed files with 52 additions and 177 deletions.
diff --git 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
@@ -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
@@ -62,20 +62,19 @@ tests: testpy testjs ## run the tests
 ###########
 .PHONY: lintpy lintjs lint fixpy fixjs fix format
 
-lintpy:  ## Black/flake8 python
-	python -m ruff jupyterfs setup.py
-	python -m black --check jupyterfs setup.py
+lintpy:  ## Lint Python with Ruff
+	python -m ruff 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
-	python -m ruff jupyterfs setup.py --fix
-	python -m black jupyterfs/ setup.py
+fixpy:  ## Autoformat Python with Ruff
+	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/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
@@ -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,17 +52,13 @@ 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",
                 ),
                 "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
@@ -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
@@ -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
@@ -25,9 +25,6 @@
     path_second_arg,
     path_kwarg,
     path_old_new,
-    getDrive,
-    isDrive,
-    stripDrive,
 )
 
 __all__ = ["MetaManager", "MetaManagerHandler"]
@@ -48,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
@@ -136,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
 
@@ -153,58 +144,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)
@@ -297,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)))