From 7af74bbebc04f2657967535d0ec390b5f8cdf4f5 Mon Sep 17 00:00:00 2001 From: "qianjun.wqj" Date: Sat, 24 Jun 2023 21:09:07 +0800 Subject: [PATCH 1/3] send2trash now supports deleting from different filesystem type (#1290) --- .../services/contents/filemanager.py | 64 +++++-------------- 1 file changed, 16 insertions(+), 48 deletions(-) diff --git a/jupyter_server/services/contents/filemanager.py b/jupyter_server/services/contents/filemanager.py index 1d5446c1ea..73bc580fa9 100644 --- a/jupyter_server/services/contents/filemanager.py +++ b/jupyter_server/services/contents/filemanager.py @@ -513,17 +513,6 @@ def delete_file(self, path): if not self.exists(path): raise web.HTTPError(404, four_o_four) - def _check_trash(os_path): - if sys.platform in {"win32", "darwin"}: - return True - - # It's a bit more nuanced than this, but until we can better - # distinguish errors from send2trash, assume that we can only trash - # files on the same partition as the home directory. - file_dev = os.stat(os_path).st_dev - home_dev = os.stat(os.path.expanduser("~")).st_dev - return file_dev == home_dev - def is_non_empty_dir(os_path): if os.path.isdir(os_path): # A directory containing only leftover checkpoints is @@ -539,20 +528,15 @@ def is_non_empty_dir(os_path): # send2trash can really delete files on Windows, so disallow # deleting non-empty files. See Github issue 3631. raise web.HTTPError(400, "Directory %s not empty" % os_path) - if _check_trash(os_path): - # Looking at the code in send2trash, I don't think the errors it - # raises let us distinguish permission errors from other errors in - # code. So for now, the "look before you leap" approach is used. - if not self.is_writable(path): - raise web.HTTPError(403, "Permission denied: %s" % path) - self.log.debug("Sending %s to trash", os_path) + # send2trash now supports deleting directories. see #1290 + if not self.is_writable(path): + raise web.HTTPError(403, "Permission denied: %s" % path) + self.log.debug("Sending %s to trash", os_path) + try: send2trash(os_path) - return - else: - self.log.warning( - "Skipping trash for %s, on different device to home directory", - os_path, - ) + except OSError as e: + raise web.HTTPError(400, "send2trash failed: %s" % e) + return if os.path.isdir(os_path): # Don't permanently delete non-empty directories. @@ -967,17 +951,6 @@ async def delete_file(self, path): if not os.path.exists(os_path): raise web.HTTPError(404, "File or directory does not exist: %s" % os_path) - async def _check_trash(os_path): - if sys.platform in {"win32", "darwin"}: - return True - - # It's a bit more nuanced than this, but until we can better - # distinguish errors from send2trash, assume that we can only trash - # files on the same partition as the home directory. - file_dev = (await run_sync(os.stat, os_path)).st_dev - home_dev = (await run_sync(os.stat, os.path.expanduser("~"))).st_dev - return file_dev == home_dev - async def is_non_empty_dir(os_path): if os.path.isdir(os_path): # A directory containing only leftover checkpoints is @@ -998,20 +971,15 @@ async def is_non_empty_dir(os_path): # send2trash can really delete files on Windows, so disallow # deleting non-empty files. See Github issue 3631. raise web.HTTPError(400, "Directory %s not empty" % os_path) - if await _check_trash(os_path): - # Looking at the code in send2trash, I don't think the errors it - # raises let us distinguish permission errors from other errors in - # code. So for now, the "look before you leap" approach is used. - if not self.is_writable(path): - raise web.HTTPError(403, "Permission denied: %s" % path) - self.log.debug("Sending %s to trash", os_path) + # send2trash now supports deleting directories. see #1290 + if not self.is_writable(path): + raise web.HTTPError(403, "Permission denied: %s" % path) + self.log.debug("Sending %s to trash", os_path) + try: send2trash(os_path) - return - else: - self.log.warning( - "Skipping trash for %s, on different device to home directory", - os_path, - ) + except OSError as e: + raise web.HTTPError(400, "send2trash failed: %s" % e) + return if os.path.isdir(os_path): # Don't permanently delete non-empty directories. From 8a2adca3426de3df757ee83407834abcb83702c7 Mon Sep 17 00:00:00 2001 From: "qianjun.wqj" Date: Sat, 24 Jun 2023 21:09:19 +0800 Subject: [PATCH 2/3] bump the minimum version for Send2Trash (#1290) --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 139e60ef59..b63f67c01b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,7 +39,7 @@ dependencies = [ "prometheus_client", "pywinpty;os_name=='nt'", "pyzmq>=24", - "Send2Trash", + "Send2Trash>=1.8.2", "terminado>=0.8.3", "tornado>=6.2.0", "traitlets>=5.6.0", From 9641dcdf142aeb3a9548157611e2250115e891fb Mon Sep 17 00:00:00 2001 From: "qianjun.wqj" Date: Mon, 26 Jun 2023 11:16:40 +0800 Subject: [PATCH 3/3] Fix lint error --- jupyter_server/services/contents/filemanager.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/jupyter_server/services/contents/filemanager.py b/jupyter_server/services/contents/filemanager.py index 73bc580fa9..6df2c3ebfa 100644 --- a/jupyter_server/services/contents/filemanager.py +++ b/jupyter_server/services/contents/filemanager.py @@ -530,12 +530,12 @@ def is_non_empty_dir(os_path): raise web.HTTPError(400, "Directory %s not empty" % os_path) # send2trash now supports deleting directories. see #1290 if not self.is_writable(path): - raise web.HTTPError(403, "Permission denied: %s" % path) + raise web.HTTPError(403, "Permission denied: %s" % path) from None self.log.debug("Sending %s to trash", os_path) try: send2trash(os_path) except OSError as e: - raise web.HTTPError(400, "send2trash failed: %s" % e) + raise web.HTTPError(400, "send2trash failed: %s" % e) from e return if os.path.isdir(os_path): @@ -973,12 +973,12 @@ async def is_non_empty_dir(os_path): raise web.HTTPError(400, "Directory %s not empty" % os_path) # send2trash now supports deleting directories. see #1290 if not self.is_writable(path): - raise web.HTTPError(403, "Permission denied: %s" % path) + raise web.HTTPError(403, "Permission denied: %s" % path) from None self.log.debug("Sending %s to trash", os_path) try: send2trash(os_path) except OSError as e: - raise web.HTTPError(400, "send2trash failed: %s" % e) + raise web.HTTPError(400, "send2trash f`1ailed: %s" % e) from e return if os.path.isdir(os_path):