From 6a46458f1f13166c43a192fd84ea57c00c1f4776 Mon Sep 17 00:00:00 2001 From: Stefano Rivera Date: Tue, 1 Dec 2020 13:04:52 -0800 Subject: [PATCH] Check for TrashPermissionError rather than guess _check_trash() was added (in #3304) because TrashPermissionError didn't exist, yet. Now that it does, we can use it, and stop guessing what will cause a permission problem. Closes: #3374 --- notebook/services/contents/filemanager.py | 22 ++++------------------ setup.py | 2 +- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/notebook/services/contents/filemanager.py b/notebook/services/contents/filemanager.py index 3fa6dad212..0c9386b2fc 100644 --- a/notebook/services/contents/filemanager.py +++ b/notebook/services/contents/filemanager.py @@ -15,6 +15,7 @@ import nbformat from send2trash import send2trash +from send2trash.exceptions import TrashPermissionError from tornado import web from .filecheckpoints import FileCheckpoints @@ -512,17 +513,6 @@ def delete_file(self, path): if not os.path.exists(os_path): raise web.HTTPError(404, u'File or directory does not exist: %s' % os_path) - 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 @@ -538,16 +528,12 @@ 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, u'Directory %s not empty' % os_path) - if _check_trash(os_path): + try: self.log.debug("Sending %s to 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, just let them all get logged as server errors. send2trash(os_path) return - else: - self.log.warning("Skipping trash for %s, on different device " - "to home directory", os_path) + except TrashPermissionError as e: + self.log.warning("Skipping trash for %s, %s", os_path, e) if os.path.isdir(os_path): # Don't permanently delete non-empty directories. diff --git a/setup.py b/setup.py index 676c5763cd..7c0d10e6a3 100755 --- a/setup.py +++ b/setup.py @@ -110,7 +110,7 @@ 'nbformat', 'nbconvert', 'ipykernel', # bless IPython kernel for now - 'Send2Trash', + 'Send2Trash>=1.5.0', 'terminado>=0.8.3', 'prometheus_client' ],