Added test for s3 delete error handling, fixed bug

Author: jverswijver

datajoint/external.py

@@ -314,11 +314,12 @@ def used(self):
         return self & [FreeTable(self.connection, ref['referencing_table']).proj(hash=ref['column_name'])
                        for ref in self.references]
 
-    def delete(self, *, delete_external_files=None, limit=None, display_progress=True):
+    def delete(self, *, delete_external_files=None, limit=None, display_progress=True, errors_as_string = True):
         """
         :param delete_external_files: True or False. If False, only the tracking info is removed from the
         external store table but the external files remain intact. If True, then the external files
         themselves are deleted too.
+        :param errors_as_string: If True any errors returned when deleting from external files will be strings
         :param limit: (integer) limit the number of items to delete
         :param display_progress: if True, display progress as files are cleaned up
         :return: if deleting external files, returns errors
@@ -346,7 +347,9 @@ def delete(self, *, delete_external_files=None, limit=None, display_progress=Tru
                     try:
                         self._remove_external_file(external_path)
                     except Exception as error:
-                        error_list.append((uuid, external_path, str(error)))
+                        if errors_as_string:
+                            error = str(error)
+                        error_list.append((uuid, external_path, error))
             return error_list
 
 

datajoint/s3.py

@@ -85,4 +85,4 @@ def remove_object(self, name):
         except minio.error.MinioException:
             raise errors.DataJointError('Failed to delete %s from s3 storage' % name)
         except ValueError:
-            raise errors.DataJointError('minIO ValueError')
+            raise errors.DataJointError('minIO ValueError something is wrong with the strings passed')

tests/test_s3.py

@@ -1,12 +1,10 @@
 from . import S3_CONN_INFO
-import time
 from minio import Minio
-from minio.objectlockconfig import ObjectLockConfig
-from minio.commonconfig import COMPLIANCE, ENABLED, GOVERNANCE
+import minio
+import json
 import urllib3
 import certifi
 from nose.tools import assert_true, raises
-
 from .schema_external import schema, SimpleRemote
 from datajoint.errors import DataJointError
 
@@ -59,27 +57,63 @@ def test_connection_secure():
     @raises(DataJointError)
     def test_remove_object_exception():
         #https://github.com/datajoint/datajoint-python/issues/952
-
-        # Initialize httpClient with relevant timeout.
-        http_client = urllib3.PoolManager(
-                                          timeout=30, cert_reqs='CERT_REQUIRED',
-                                          ca_certs=certifi.where(),
-                                          retries=urllib3.Retry(total=3, backoff_factor=0.2,
-                                                                status_forcelist=[
-                                                                                  500, 502,
-                                                                                  503, 504]))
-
         # Initialize minioClient with an endpoint and access/secret keys.
         minio_client = Minio(
-            S3_CONN_INFO['endpoint'],
-            access_key=S3_CONN_INFO['access_key'],
-            secret_key=S3_CONN_INFO['secret_key'],
-            secure=True,
-            http_client=http_client)
+            'minio:9000',
+            access_key= 'jeffjeff',
+            secret_key= 'jeffjeff',
+            secure=False)
+
+        minio_admin = minio.MinioAdmin(target = 'myminio')
+        minio_admin.user_add(access_key = 'jeffjeff', secret_key = 'jeffjeff')
 
+        testpolicy = {
+    "Version": "2012-10-17",
+    "Statement": [
+        {
+            "Action": [
+                "s3:GetBucketLocation",
+                "s3:ListBucket",
+                "s3:ListBucketMultipartUploads",
+                "s3:ListAllMyBuckets"
+            ],
+            "Effect": "Allow",
+            "Resource": [
+                "arn:aws:s3:::datajoint.test",
+                "arn:aws:s3:::datajoint.migrate"
+            ],
+            "Sid": ""
+        },
+        {
+            "Action": [
+                "s3:GetObject",
+                "s3:ListMultipartUploadParts"
+            ],
+            "Effect": "Allow",
+            "Resource": [
+                "arn:aws:s3:::datajoint.test/*",
+                "arn:aws:s3:::datajoint.migrate/*"
+            ],
+            "Sid": ""
+        }
+    ]
+}
+        with open('/tmp/policy.json', 'w') as f:
+            f.write(json.dumps(testpolicy))
+        minio_admin.policy_add(policy_name = 'test', policy_file = '/tmp/policy.json')
+        minio_admin.policy_set(policy_name = 'test', user = 'jeffjeff')
 
-        schema.external['share'].s3.bucket = ''
-        schema.external['share'].s3.remove_object('test')
+        
+        test = [1,[1,2,3]]
+        SimpleRemote.insert1(test)
+        SimpleRemote.delete()
+        
+        schema.external['share'].s3.client = minio_client
+        # This method returns a list of errors
+        error_list = schema.external['share'].delete(delete_external_files = True, errors_as_string = False)
+        raise error_list[0][2]
+        # schema.external['share'].s3.secret_key = ''
+        # schema.external['share'].s3.remove_object('test')