Merge branch 'master' of https://github.com/datajoint/datajoint-python into issue151

Author: dimitri-yatsenko

.github/workflows/development.yaml

@@ -40,7 +40,7 @@ jobs:
           PY_VER: ${{matrix.py_ver}}
           MYSQL_VER: ${{matrix.mysql_ver}}
           ALPINE_VER: "3.10"
-          MINIO_VER: RELEASE.2019-09-26T19-42-35Z
+          MINIO_VER: RELEASE.2021-09-03T03-56-13Z
           COMPOSE_HTTP_TIMEOUT: "120"
           COVERALLS_SERVICE_NAME: travis-ci
           COVERALLS_REPO_TOKEN: fd0BoXG46TPReEem0uMy7BJO5j0w1MQiY

CHANGELOG.md

@@ -3,7 +3,11 @@
 ### 0.13.3 -- TBD
 * Bugfix - Dependencies not properly loaded on populate. (#902) PR #919
 * Bugfix - Replace use of numpy aliases of built-in types with built-in type. (#938) PR #939
+<<<<<<< HEAD
 * Bugfix - Deletes and drops can cascade to part from master only. (#151 and #374) PR #957
+=======
+* Bugfix - Fix error handling of remove_object function in `s3.py` (#952) PR #955
+>>>>>>> 61dab59fe59a408b018bf2a6330ec0ce07c2ad42
 
 ### 0.13.2 -- May 7, 2021
 * Update `setuptools_certificate` dependency to new name `otumat`

LNX-docker-compose.yml

@@ -77,6 +77,9 @@ services:
       - -c
       - |
         set -e
+        wget -P /home/dja/.local/bin/ https://dl.min.io/client/mc/release/linux-amd64/mc
+        chmod +x /home/dja/.local/bin/mc
+        mc alias set myminio/ http://minio:9000 datajoint datajoint
         pip install --user -r test_requirements.txt
         pip install -e .
         pip freeze | grep datajoint

README.md

@@ -108,7 +108,7 @@ A number of labs are currently adopting DataJoint and we are quickly getting the
 PY_VER=3.7
 ALPINE_VER=3.10
 MYSQL_VER=5.7
-MINIO_VER=RELEASE.2019-09-26T19-42-35Z
+MINIO_VER=RELEASE.2021-09-03T03-56-13Z
 UID=1000
 GID=1000
 ```

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,8 @@ 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)))
+                        error_list.append((uuid, external_path,
+                                           str(error) if errors_as_string else error))
             return error_list
 
 

datajoint/s3.py

@@ -76,12 +76,11 @@ def get_size(self, name):
         except minio.error.S3Error as e:
             if e.code == 'NoSuchKey':
                 raise errors.MissingExternalFile
-            else:
-                raise e
+            raise e
 
     def remove_object(self, name):
         logger.debug('remove_object: {}:{}'.format(self.bucket, name))
         try:
             self.client.remove_object(self.bucket, str(name))
-        except minio.ResponseError:
-            return errors.DataJointError('Failed to delete %s from s3 storage' % name)
+        except minio.error.MinioException:
+            raise errors.DataJointError('Failed to delete %s from s3 storage' % name)

datajoint/table.py

@@ -361,59 +361,7 @@ def delete_quick(self, get_count=False):
         self._log(query[:255])
         return count
 
-    def _delete_cascade(self):
-        """service function to perform cascading deletes recursively."""
-        max_attempts = 50
-        for _ in range(max_attempts):
-            try:
-                delete_count = self.delete_quick(get_count=True)
-            except IntegrityError as error:
-                match = foreign_key_error_regexp.match(error.args[0]).groupdict()
-                if "`.`" not in match['child']:  # if schema name missing, use self
-                    match['child'] = '{}.{}'.format(self.full_table_name.split(".")[0],
-                                                    match['child'])
-                if match['pk_attrs'] is not None:  # fully matched, adjusting the keys
-                    match['fk_attrs'] = [k.strip('`') for k in match['fk_attrs'].split(',')]
-                    match['pk_attrs'] = [k.strip('`') for k in match['pk_attrs'].split(',')]
-                else:  # only partially matched, querying with constraint to determine keys
-                    match['fk_attrs'], match['parent'], match['pk_attrs'] = list(map(
-                        list, zip(*self.connection.query(constraint_info_query, args=(
-                            match['name'].strip('`'),
-                            *[_.strip('`') for _ in match['child'].split('`.`')]
-                            )).fetchall())))
-                    match['parent'] = match['parent'][0]
-
-                # Avoid deleting from child before master (See issue #151)
-                master = get_master(match['child'])
-                if master and self.full_table_name != master:
-                    raise DataJointError(
-                        'Attempt to delete from part table {part} before deleting from '
-                        'its master. Delete from {master} first.'.format(
-                            part=match['child'], master=master))
-
-                # Restrict child by self if
-                #   1. if self's restriction attributes are not in child's primary key
-                #   2. if child renames any attributes
-                # Otherwise restrict child by self's restriction.
-                child = FreeTable(self.connection, match['child'])
-                if set(self.restriction_attributes) <= set(child.primary_key) and \
-                        match['fk_attrs'] == match['pk_attrs']:
-                    child._restriction = self._restriction
-                elif match['fk_attrs'] != match['pk_attrs']:
-                    child &= self.proj(**dict(zip(match['fk_attrs'],
-                                                  match['pk_attrs'])))
-                else:
-                    child &= self.proj()
-                child._delete_cascade()
-            else:
-                print("Deleting {count} rows from {table}".format(
-                    count=delete_count, table=self.full_table_name))
-                break
-        else:
-            raise DataJointError('Exceeded maximum number of delete attempts.')
-        return delete_count
-
-    def delete(self, transaction=True, safemode=None):
+    def delete(self, transaction=True, safemode=None, force_parts=False):
         """
         Deletes the contents of the table and its dependent tables, recursively.
 
@@ -422,8 +370,56 @@ def delete(self, transaction=True, safemode=None):
             within another transaction.
         :param safemode: If True, prohibit nested transactions and prompt to confirm. Default
             is dj.config['safemode'].
+        :param force_parts: Delete from parts even when not deleting from their masters.
         :return: number of deleted rows (excluding those from dependent tables)
         """
+        deleted = set()
+
+        def cascade(table):
+            """service function to perform cascading deletes recursively."""
+            max_attempts = 50
+            for _ in range(max_attempts):
+                try:
+                    delete_count = table.delete_quick(get_count=True)
+                except IntegrityError as error:
+                    match = foreign_key_error_regexp.match(error.args[0]).groupdict()
+                    if "`.`" not in match['child']:  # if schema name missing, use table
+                        match['child'] = '{}.{}'.format(table.full_table_name.split(".")[0],
+                                                        match['child'])
+                    if match['pk_attrs'] is not None:  # fully matched, adjusting the keys
+                        match['fk_attrs'] = [k.strip('`') for k in match['fk_attrs'].split(',')]
+                        match['pk_attrs'] = [k.strip('`') for k in match['pk_attrs'].split(',')]
+                    else:  # only partially matched, querying with constraint to determine keys
+                        match['fk_attrs'], match['parent'], match['pk_attrs'] = list(map(
+                            list, zip(*table.connection.query(constraint_info_query, args=(
+                                match['name'].strip('`'),
+                                *[_.strip('`') for _ in match['child'].split('`.`')]
+                            )).fetchall())))
+                        match['parent'] = match['parent'][0]
+
+                    # Restrict child by table if
+                    #   1. if table's restriction attributes are not in child's primary key
+                    #   2. if child renames any attributes
+                    # Otherwise restrict child by table's restriction.
+                    child = FreeTable(table.connection, match['child'])
+                    if set(table.restriction_attributes) <= set(child.primary_key) and \
+                            match['fk_attrs'] == match['pk_attrs']:
+                        child._restriction = table._restriction
+                    elif match['fk_attrs'] != match['pk_attrs']:
+                        child &= table.proj(**dict(zip(match['fk_attrs'],
+                                                       match['pk_attrs'])))
+                    else:
+                        child &= table.proj()
+                    cascade(child)
+                else:
+                    deleted.add(table.full_table_name)
+                    print("Deleting {count} rows from {table}".format(
+                        count=delete_count, table=table.full_table_name))
+                    break
+            else:
+                raise DataJointError('Exceeded maximum number of delete attempts.')
+            return delete_count
+
         safemode = config['safemode'] if safemode is None else safemode
 
         # Start transaction
@@ -440,12 +436,23 @@ def delete(self, transaction=True, safemode=None):
 
         # Cascading delete
         try:
-            delete_count = self._delete_cascade()
+            delete_count = cascade(self)
         except:
             if transaction:
                 self.connection.cancel_transaction()
             raise
 
+        if not force_parts:
+            # Avoid deleting from child before master (See issue #151)
+            for part in deleted:
+                master = get_master(part)
+                if master and master not in deleted:
+                    if transaction:
+                        self.connection.cancel_transaction()
+                    raise DataJointError(
+                        'Attempt to delete part table {part} before deleting from '
+                        'its master {master} first.'.format(part=part, master=master))
+
         # Confirm and commit
         if delete_count == 0:
             if safemode:

datajoint/user_tables.py

@@ -173,9 +173,10 @@ def delete(self, force=False):
         unless force is True, prohibits direct deletes from parts.
         """
         if force:
-            super().delete()
+            super().delete(force_parts=True)
         else:
-            raise DataJointError('Cannot delete from a Part directly.  Delete from master instead')
+            raise DataJointError(
+                'Cannot delete from a Part directly. Delete from master instead')
 
     def drop(self, force=False):
         """

docs-parts/intro/Releases_lang1.rst

@@ -2,7 +2,11 @@
 ----------------------
 * Bugfix - Dependencies not properly loaded on populate. (#902) PR #919
 * Bugfix - Replace use of numpy aliases of built-in types with built-in type. (#938) PR #939
+<<<<<<< HEAD
 * Bugfix - Deletes and drops can cascade to part from master only. (#151 and #374) PR #957
+=======
+* Bugfix - Fix error handling of remove_object function in `s3.py` (#952) PR #955
+>>>>>>> 61dab59fe59a408b018bf2a6330ec0ce07c2ad42
 
 0.13.2 -- May 7, 2021
 ----------------------

local-docker-compose.yml

@@ -34,7 +34,7 @@ services:
       interval: 1s
   fakeservices.datajoint.io:
     <<: *net
-    image: datajoint/nginx:v0.0.17
+    image: datajoint/nginx:v0.0.18
     environment:
     - ADD_db_TYPE=DATABASE
     - ADD_db_ENDPOINT=db:3306
@@ -82,6 +82,9 @@ services:
       - -c
       - |
         set -e
+        wget -P /home/dja/.local/bin/ https://dl.min.io/client/mc/release/linux-amd64/mc
+        chmod +x /home/dja/.local/bin/mc
+        mc alias set myminio/ http://minio:9000 datajoint datajoint
         pip install --user nose nose-cov coveralls flake8 ptvsd
         pip install -e .
         pip freeze | grep datajoint