Browse Source

[s3] Fix delete op for directories and non-ascii keys deletion (#3548)

- Fetch key list with correct key_name prefix for bulk keys (directory like scenarios).
- Skip using iterator and simply pass the list of dir keys to boto delete method.

- This also fixes the deletion of non-ascii directory deletion issue.
Harsh Gupta 2 years ago
parent
commit
8f8e5883a6
2 changed files with 6 additions and 7 deletions
  1. 3 4
      desktop/libs/aws/src/aws/s3/s3fs.py
  2. 3 3
      desktop/libs/aws/src/aws/s3/s3fs_test.py

+ 3 - 4
desktop/libs/aws/src/aws/s3/s3fs.py

@@ -362,12 +362,11 @@ class S3FileSystem(object):
       key = self._get_key(path, validate=False)
       key = self._get_key(path, validate=False)
 
 
       if key.exists():
       if key.exists():
-        to_delete = [key]
         dir_keys = []
         dir_keys = []
 
 
         if self.isdir(path):
         if self.isdir(path):
-          dir_keys = key.bucket.list(prefix=path)
-          to_delete = itertools.chain(dir_keys, to_delete)
+          _, dir_key_name = s3.parse_uri(path)[:2]
+          dir_keys = key.bucket.list(prefix=dir_key_name)
 
 
         if not dir_keys:
         if not dir_keys:
           # Avoid Raz bulk delete issue
           # Avoid Raz bulk delete issue
@@ -375,7 +374,7 @@ class S3FileSystem(object):
           if deleted_key.exists():
           if deleted_key.exists():
             raise S3FileSystemException('Could not delete key %s' % deleted_key)
             raise S3FileSystemException('Could not delete key %s' % deleted_key)
         else:
         else:
-          result = key.bucket.delete_keys(to_delete)
+          result = key.bucket.delete_keys(list(dir_keys))
           if result.errors:
           if result.errors:
             msg = "%d errors occurred while attempting to delete the following S3 paths:\n%s" % (
             msg = "%d errors occurred while attempting to delete the following S3 paths:\n%s" % (
               len(result.errors), '\n'.join(['%s: %s' % (error.key, error.message) for error in result.errors])
               len(result.errors), '\n'.join(['%s: %s' % (error.key, error.message) for error in result.errors])

+ 3 - 3
desktop/libs/aws/src/aws/s3/s3fs_test.py

@@ -102,7 +102,7 @@ class TestS3FileSystem():
         fs.rmtree(path='s3a://gethue/data')
         fs.rmtree(path='s3a://gethue/data')
 
 
         key.delete.assert_called()
         key.delete.assert_called()
-        key.bucket.list.assert_called_with(prefix='s3a://gethue/data/')
+        key.bucket.list.assert_called_with(prefix='data/')
         key.bucket.delete_keys.assert_not_called()
         key.bucket.delete_keys.assert_not_called()
 
 
   def test_rmtree_non_empty_dir(self):
   def test_rmtree_non_empty_dir(self):
@@ -113,7 +113,7 @@ class TestS3FileSystem():
           name='data',
           name='data',
           exists=Mock(return_value=True),
           exists=Mock(return_value=True),
           bucket=Mock(
           bucket=Mock(
-            list=Mock(return_value=['s3a://gethue/data/1', 's3a://gethue/data/2']),
+            list=Mock(return_value=['data/1', 'data/2']),
             delete_keys=Mock(
             delete_keys=Mock(
               return_value=Mock(
               return_value=Mock(
                 errors=[]
                 errors=[]
@@ -134,7 +134,7 @@ class TestS3FileSystem():
         fs.rmtree(path='s3a://gethue/data')
         fs.rmtree(path='s3a://gethue/data')
 
 
         key.delete.assert_not_called()
         key.delete.assert_not_called()
-        key.bucket.list.assert_called_with(prefix='s3a://gethue/data/')
+        key.bucket.list.assert_called_with(prefix='data/')
         key.bucket.delete_keys.assert_called()
         key.bucket.delete_keys.assert_called()