From 8c90eb5cdce4bb66ec14a22a02b0abbf5ec58b67 Mon Sep 17 00:00:00 2001 From: Clark Date: Wed, 28 Jun 2023 00:47:46 +0800 Subject: [PATCH] send2trash now supports deleting from different filesystem type(#1290) (#1291) Co-authored-by: qianjun.wqj --- .../services/contents/filemanager.py | 64 +++++-------------- pyproject.toml | 2 +- 2 files changed, 17 insertions(+), 49 deletions(-) diff --git a/jupyter_server/services/contents/filemanager.py b/jupyter_server/services/contents/filemanager.py index 1d5446c1ea..6df2c3ebfa 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) from None + 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) from 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) from None + 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 f`1ailed: %s" % e) from e + return if os.path.isdir(os_path): # Don't permanently delete non-empty directories. 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",