Merge pull request #5894 from stefanor/trashpermissionerror

Handle TrashPermissionError, now that it exists

Author: kevin-bates

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.

notebook/services/contents/tests/test_contents_api.py

@@ -12,6 +12,8 @@
 pjoin = os.path.join
 
 import requests
+from send2trash import send2trash
+from send2trash.exceptions import TrashPermissionError
 
 from ..filecheckpoints import GenericFileCheckpoints
 
@@ -197,6 +199,14 @@ def isfile(self, api_path):
     def isdir(self, api_path):
         return os.path.isdir(self.to_os_path(api_path))
 
+    def can_send2trash(self, api_path):
+        """Send a path to trash, if possible. Return success."""
+        try:
+            send2trash(self.to_os_path(api_path))
+            return True
+        except TrashPermissionError as e:
+            return False
+
     def setUp(self):
         for d in (self.dirs + self.hidden_dirs):
             self.make_dir(d)
@@ -526,7 +536,13 @@ def test_delete_non_empty_dir(self):
         if sys.platform == 'win32':
             self.skipTest("Disabled deleting non-empty dirs on Windows")
         # Test that non empty directory can be deleted
-        self.api.delete(u'å b')
+        try:
+            self.api.delete(u'å b')
+        except requests.HTTPError as e:
+            if e.response.status_code == 400:
+                if not self.can_send2trash(u'å b'):
+                    self.skipTest("Dir can't be sent to trash")
+            raise
         # Check if directory has actually been deleted
         with assert_http_error(404):
             self.api.list(u'å 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'
     ],