From 4a64615d29f5114fc87ff6e8a295aa42efd09e3b Mon Sep 17 00:00:00 2001
From: Carlos Ortiz <codatajoint@Carloss-MacBook-Pro.local>
Date: Tue, 31 Aug 2021 18:22:49 -0500
Subject: [PATCH 01/16] add test placeholder.

---
 tests/__init__.py      | 2 +-
 tests/test_external.py | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/__init__.py b/tests/__init__.py
index 6b802e332..d50672b49 100644
--- a/tests/__init__.py
+++ b/tests/__init__.py
@@ -20,7 +20,7 @@
 __author__ = 'Edgar Walker, Fabian Sinz, Dimitri Yatsenko, Raphael Guzman'
 
 # turn on verbose logging
-logging.basicConfig(level=logging.DEBUG)
+#logging.basicConfig(level=logging.DEBUG)
 
 __all__ = ['__author__', 'PREFIX', 'CONN_INFO']
 
diff --git a/tests/test_external.py b/tests/test_external.py
index 22fee51ab..6fb773642 100644
--- a/tests/test_external.py
+++ b/tests/test_external.py
@@ -103,3 +103,8 @@ def test_file_leading_slash():
     file external storage configured with leading slash
     """
     test_s3_leading_slash(index=200, store='local')
+
+def test_remove_fail():
+    #https://github.com/datajoint/datajoint-python/issues/953
+
+    raise Exception('error removing')

From f41b8d168385bbc1a92bf4f85206d70355bfd7e6 Mon Sep 17 00:00:00 2001
From: Carlos Ortiz <codatajoint@Carloss-MBP.attlocal.net>
Date: Wed, 1 Sep 2021 18:17:19 -0500
Subject: [PATCH 02/16] Add test file to see file location and permissions.

---
 tests/test_external.py | 44 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 41 insertions(+), 3 deletions(-)

diff --git a/tests/test_external.py b/tests/test_external.py
index 6fb773642..5f2703f32 100644
--- a/tests/test_external.py
+++ b/tests/test_external.py
@@ -4,10 +4,15 @@
 from datajoint.external import ExternalTable
 from datajoint.blob import pack, unpack
 
-from .schema_external import schema
+
 import datajoint as dj
-from .schema_external import stores_config
-from .schema_external import SimpleRemote
+from .schema_external import stores_config, SimpleRemote, Simple, schema
+
+import json
+import os 
+from os import stat
+import pwd
+from pwd import getpwuid
 current_location_s3 = dj.config['stores']['share']['location']
 current_location_local = dj.config['stores']['local']['location']
 
@@ -107,4 +112,37 @@ def test_file_leading_slash():
 def test_remove_fail():
     #https://github.com/datajoint/datajoint-python/issues/953
 
+    print(json.dumps(dj.config['stores'], indent=4))
+    print(dj.config['stores']['local']['location'])
+
+    data = dict(simple = 1, item = [1, 2, 3])
+    Simple.insert1(data)
+
+    print('location')
+    print(os.listdir(dj.config['stores']['local']['location'] + 
+    '/djtest_extern/4/c'))
+
+    path1 = dj.config['stores']['local']['location'] + '/djtest_extern/4/c/'
+
+    path2 = os.listdir(dj.config['stores']['local']['location'] + '/djtest_extern/4/c/')
+
+    print(path1 + path2[0])
+
+    path3 = path1 + path2[0]
+
+    # st = stat(path1 + path2[0])
+    # print(bool(st.st_mode & stat.S_IXUSR))
+
+    print(getpwuid(stat(path3).st_uid).pw_name)
+
+    print(Simple())
+    (Simple & 'simple=1').delete()
+    print(Simple())
+    print(schema.external['local'])
+    schema.external['local'].delete(delete_external_files=True)
+    print(os.listdir(dj.config['stores']['local']['location'] + 
+    '/djtest_extern/4/c'))
+
+
+    print(schema.external['local'])
     raise Exception('error removing')

From ad32b6eeb48d7b0e0b9061fcdd4af72fb8cfd7f5 Mon Sep 17 00:00:00 2001
From: Carlos Ortiz <codatajoint@Carloss-MBP.attlocal.net>
Date: Thu, 2 Sep 2021 17:22:42 -0500
Subject: [PATCH 03/16] Added new print statements and added new print
 statements within external.py

---
 datajoint/external.py  |  3 +++
 tests/test_external.py | 42 ++++++++++++++++++++++++++++--------------
 2 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/datajoint/external.py b/datajoint/external.py
index 808b2d65b..764a42a28 100644
--- a/datajoint/external.py
+++ b/datajoint/external.py
@@ -127,6 +127,9 @@ def _remove_external_file(self, external_path):
         if self.spec['protocol'] == 's3':
             self.s3.remove_object(external_path)
         elif self.spec['protocol'] == 'file':
+            print('we have reached the unlink statement in datajoint')
+            print(external_path)
+            print(Path(external_path).is_file())
             Path(external_path).unlink()
 
     def exists(self, external_filepath):
diff --git a/tests/test_external.py b/tests/test_external.py
index 5f2703f32..ba5af91fa 100644
--- a/tests/test_external.py
+++ b/tests/test_external.py
@@ -112,15 +112,20 @@ def test_file_leading_slash():
 def test_remove_fail():
     #https://github.com/datajoint/datajoint-python/issues/953
 
-    print(json.dumps(dj.config['stores'], indent=4))
-    print(dj.config['stores']['local']['location'])
+    #print(json.dumps(dj.config['stores'], indent=4))
+    #print(dj.config['stores']['local']['location'])
+
+    dirName = dj.config['stores']['local']['location']
+
+    #print('directory: ' + dirName)
+
+
 
     data = dict(simple = 1, item = [1, 2, 3])
     Simple.insert1(data)
 
-    print('location')
-    print(os.listdir(dj.config['stores']['local']['location'] + 
-    '/djtest_extern/4/c'))
+    #print('location')
+    #print(os.listdir(dj.config['stores']['local']['location'] + '/djtest_extern/4/c'))
 
     path1 = dj.config['stores']['local']['location'] + '/djtest_extern/4/c/'
 
@@ -128,21 +133,30 @@ def test_remove_fail():
 
     print(path1 + path2[0])
 
-    path3 = path1 + path2[0]
+    old_name = path1 + path2[0]
+
+    new_name = "/tmp/newfile"
+
+    os.rename(old_name, new_name)
+
+    print(f'\nis the new file name a file? {os.path.isfile(new_name)}')
+    print(f'\nis the old file name a file? {os.path.isfile(old_name)}')
+
+    print(os.listdir(dj.config['stores']['local']['location'] + '/djtest_extern/4/c/'))
 
     # st = stat(path1 + path2[0])
     # print(bool(st.st_mode & stat.S_IXUSR))
 
-    print(getpwuid(stat(path3).st_uid).pw_name)
+    #print(getpwuid(stat(path3).st_uid).pw_name)
 
-    print(Simple())
+    print(f'simple table before delete {Simple()}')
     (Simple & 'simple=1').delete()
-    print(Simple())
+    print(f'simple table after delete {Simple()}')
+    print('-------------showing external store before delete with flag---------')
     print(schema.external['local'])
-    schema.external['local'].delete(delete_external_files=True)
-    print(os.listdir(dj.config['stores']['local']['location'] + 
-    '/djtest_extern/4/c'))
-
-
+    listOfErrors = schema.external['local'].delete(delete_external_files=True)
+    print(f'list of errors: {listOfErrors}')
+    print(os.listdir(dj.config['stores']['local']['location'] + '/djtest_extern/4/c'))
+    print('-------------showing external store after delete with flag---------')
     print(schema.external['local'])
     raise Exception('error removing')

From 7b3600ad4cc471fc194d7798153f452f4d44dd1e Mon Sep 17 00:00:00 2001
From: Carlos Ortiz <codatajoint@Carloss-MacBook-Pro.local>
Date: Tue, 7 Sep 2021 13:40:28 -0500
Subject: [PATCH 04/16] adding more asserts to test file

---
 tests/test_external.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/test_external.py b/tests/test_external.py
index ba5af91fa..d07e8ac72 100644
--- a/tests/test_external.py
+++ b/tests/test_external.py
@@ -125,7 +125,8 @@ def test_remove_fail():
     Simple.insert1(data)
 
     #print('location')
-    #print(os.listdir(dj.config['stores']['local']['location'] + '/djtest_extern/4/c'))
+    print('\n BEFORE DELETE: list of dir stores, local, location')
+    print(os.listdir(dj.config['stores']['local']['location'] + '/djtest_extern/4/c'))
 
     path1 = dj.config['stores']['local']['location'] + '/djtest_extern/4/c/'
 
@@ -156,7 +157,13 @@ def test_remove_fail():
     print(schema.external['local'])
     listOfErrors = schema.external['local'].delete(delete_external_files=True)
     print(f'list of errors: {listOfErrors}')
+    print('list of dir stores, local, location')
     print(os.listdir(dj.config['stores']['local']['location'] + '/djtest_extern/4/c'))
     print('-------------showing external store after delete with flag---------')
     print(schema.external['local'])
+
+    assert len(listOfErrors) == 1, 'unexpected number of errors'
+    assert len(schema.external['local']) == 1, 'unexpected number of rows in external table'
+    listUID = listOfErrors[0][0] 
+    tableUID = schema.external['local']
     raise Exception('error removing')

From e244337b9d8e8ccf44e68772b893670d7d62f0fe Mon Sep 17 00:00:00 2001
From: Carlos Ortiz <codatajoint@Carloss-MacBook-Pro.local>
Date: Tue, 7 Sep 2021 18:17:08 -0500
Subject: [PATCH 05/16] adding new func to delete in external.py

---
 datajoint/external.py  |  5 ++++-
 tests/test_external.py | 12 ++++++++----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/datajoint/external.py b/datajoint/external.py
index 764a42a28..9ef9e17de 100644
--- a/datajoint/external.py
+++ b/datajoint/external.py
@@ -341,7 +341,8 @@ def delete(self, *, delete_external_files=None, limit=None, display_progress=Tru
             error_list = []
             for uuid, external_path in items:
                 try:
-                    count = (self & {'hash': uuid}).delete_quick(get_count=True)  # optimize
+                    count = len(self & {'hash': uuid}).delete_quick(get_count=True)  # optimize
+                    print(f'\n\n\nCOUNT IS: {count}')
                 except Exception:
                     pass   # if delete failed, do not remove the external file
                 else:
@@ -350,6 +351,8 @@ def delete(self, *, delete_external_files=None, limit=None, display_progress=Tru
                         self._remove_external_file(external_path)
                     except Exception as error:
                         error_list.append((uuid, external_path, str(error)))
+                    else:
+                        #(self & {'hash': uuid}).delete_quick(get_count=True)
             return error_list
 
 
diff --git a/tests/test_external.py b/tests/test_external.py
index d07e8ac72..ac87f6575 100644
--- a/tests/test_external.py
+++ b/tests/test_external.py
@@ -162,8 +162,12 @@ def test_remove_fail():
     print('-------------showing external store after delete with flag---------')
     print(schema.external['local'])
 
+    print(f'\n is this the UID or HASH? {listOfErrors[0][0]} ')
+
+    LENGTH_OF_QUERY = len(schema.external['local'] & dict(hash = listOfErrors[0][0]))
+
+    print(f'\nWHAT IS THE LENGTH OF THIS? {LENGTH_OF_QUERY}')
+
     assert len(listOfErrors) == 1, 'unexpected number of errors'
-    assert len(schema.external['local']) == 1, 'unexpected number of rows in external table'
-    listUID = listOfErrors[0][0] 
-    tableUID = schema.external['local']
-    raise Exception('error removing')
+    assert len(schema.external['local'] & dict(hash = listOfErrors[0][0])) == 1, 'unexpected number of rows in external table'
+    

From 6452838c9750bac3559c15598112b47b77d7e095 Mon Sep 17 00:00:00 2001
From: Carlos Ortiz <codatajoint@Carloss-MBP.attlocal.net>
Date: Wed, 8 Sep 2021 13:40:53 -0500
Subject: [PATCH 06/16] added code to test

---
 datajoint/external.py                         |   7 +-
 .../4/c/4c16e9e15b64474e4b56ba22bb17faae      | Bin 0 -> 49 bytes
 .../4/c/4c16e9e15b64474e4b56ba22bb17faae      | Bin 0 -> 49 bytes
 tests/test_external.py                        |  61 ++++++++++++------
 4 files changed, 44 insertions(+), 24 deletions(-)
 create mode 100644 djtest_extern/4/c/4c16e9e15b64474e4b56ba22bb17faae
 create mode 100644 tests/djtest_extern/4/c/4c16e9e15b64474e4b56ba22bb17faae

diff --git a/datajoint/external.py b/datajoint/external.py
index 9ef9e17de..ef3f8ecf2 100644
--- a/datajoint/external.py
+++ b/datajoint/external.py
@@ -341,7 +341,7 @@ def delete(self, *, delete_external_files=None, limit=None, display_progress=Tru
             error_list = []
             for uuid, external_path in items:
                 try:
-                    count = len(self & {'hash': uuid}).delete_quick(get_count=True)  # optimize
+                    count = len(self & {'hash': uuid})  # optimize
                     print(f'\n\n\nCOUNT IS: {count}')
                 except Exception:
                     pass   # if delete failed, do not remove the external file
@@ -351,8 +351,9 @@ def delete(self, *, delete_external_files=None, limit=None, display_progress=Tru
                         self._remove_external_file(external_path)
                     except Exception as error:
                         error_list.append((uuid, external_path, str(error)))
-                    else:
-                        #(self & {'hash': uuid}).delete_quick(get_count=True)
+                    else: 
+                        (self & {'hash': uuid}).delete_quick(get_count=True)
+                    
             return error_list
 
 
diff --git a/djtest_extern/4/c/4c16e9e15b64474e4b56ba22bb17faae b/djtest_extern/4/c/4c16e9e15b64474e4b56ba22bb17faae
new file mode 100644
index 0000000000000000000000000000000000000000..8b1bfda07ffaabdbd8f05cd123d824ae8321f6ff
GIT binary patch
literal 49
ccmYevGGJh0W`F<|D9y#lz=*_VLSi!m05nDcHvj+t

literal 0
HcmV?d00001

diff --git a/tests/djtest_extern/4/c/4c16e9e15b64474e4b56ba22bb17faae b/tests/djtest_extern/4/c/4c16e9e15b64474e4b56ba22bb17faae
new file mode 100644
index 0000000000000000000000000000000000000000..8b1bfda07ffaabdbd8f05cd123d824ae8321f6ff
GIT binary patch
literal 49
ccmYevGGJh0W`F<|D9y#lz=*_VLSi!m05nDcHvj+t

literal 0
HcmV?d00001

diff --git a/tests/test_external.py b/tests/test_external.py
index ac87f6575..92228124f 100644
--- a/tests/test_external.py
+++ b/tests/test_external.py
@@ -121,18 +121,26 @@ def test_remove_fail():
 
 
 
-    data = dict(simple = 1, item = [1, 2, 3])
+    data = dict(simple = 2, item = [1, 2, 3])
     Simple.insert1(data)
 
     #print('location')
-    print('\n BEFORE DELETE: list of dir stores, local, location')
-    print(os.listdir(dj.config['stores']['local']['location'] + '/djtest_extern/4/c'))
+    # print('\n IN TEST: BEFORE DELETE: list of dir stores, local, location')
+    print('stores location -----------\n')
+    print(dj.config['stores']['local']['location'])
+    print('local location -----------\n')
+    print(schema.external['local'])
+    print('----------------------------')
 
     path1 = dj.config['stores']['local']['location'] + '/djtest_extern/4/c/'
 
-    path2 = os.listdir(dj.config['stores']['local']['location'] + '/djtest_extern/4/c/')
+    argDir = dj.config['stores']['local']['location'] + '/djtest_extern/4/c/'
+
+    print(f'argDir----------\n{argDir}\n')
+
+    path2 = os.listdir(argDir)
 
-    print(path1 + path2[0])
+    # print(path1 + path2[0])
 
     old_name = path1 + path2[0]
 
@@ -140,34 +148,45 @@ def test_remove_fail():
 
     os.rename(old_name, new_name)
 
-    print(f'\nis the new file name a file? {os.path.isfile(new_name)}')
-    print(f'\nis the old file name a file? {os.path.isfile(old_name)}')
+    # print(f'\n IN TEST: is the new file name a file? {os.path.isfile(new_name)}')
+    # print(f'\n IN TEST: is the old file name a file? {os.path.isfile(old_name)}')
 
-    print(os.listdir(dj.config['stores']['local']['location'] + '/djtest_extern/4/c/'))
+    # print(os.listdir(dj.config['stores']['local']['location'] + '/djtest_extern/4/c/'))
 
     # st = stat(path1 + path2[0])
     # print(bool(st.st_mode & stat.S_IXUSR))
 
     #print(getpwuid(stat(path3).st_uid).pw_name)
 
-    print(f'simple table before delete {Simple()}')
-    (Simple & 'simple=1').delete()
-    print(f'simple table after delete {Simple()}')
-    print('-------------showing external store before delete with flag---------')
-    print(schema.external['local'])
+    # print(f' IN TEST: simple table before delete {Simple()}')
+    (Simple & 'simple=2').delete()
+    # print(f' IN TEST: simple table after delete {Simple()}')
+    # print(' IN TEST: -------------showing external store before delete with flag---------')
+    # print(schema.external['local'])
     listOfErrors = schema.external['local'].delete(delete_external_files=True)
-    print(f'list of errors: {listOfErrors}')
-    print('list of dir stores, local, location')
-    print(os.listdir(dj.config['stores']['local']['location'] + '/djtest_extern/4/c'))
-    print('-------------showing external store after delete with flag---------')
-    print(schema.external['local'])
+    # print(f' IN TEST: list of errors: {listOfErrors}')
+    # print(' IN TEST: list of dir stores, local, location')
+    # print(os.listdir(dj.config['stores']['local']['location'] + '/djtest_extern/4/c'))
+    # print(' IN TEST: -------------showing external store after delete with flag---------')
+    # print(schema.external['local'])
 
-    print(f'\n is this the UID or HASH? {listOfErrors[0][0]} ')
+    # print(f'\n IN TEST: is this the UID or HASH? {listOfErrors[0][0]}')
 
-    LENGTH_OF_QUERY = len(schema.external['local'] & dict(hash = listOfErrors[0][0]))
+    # LENGTH_OF_QUERY = len(schema.external['local'] & dict(hash = listOfErrors[0][0]))
 
-    print(f'\nWHAT IS THE LENGTH OF THIS? {LENGTH_OF_QUERY}')
+    # print(f'\n IN TEST: WHAT IS THE LENGTH OF THIS? {LENGTH_OF_QUERY}')
 
     assert len(listOfErrors) == 1, 'unexpected number of errors'
     assert len(schema.external['local'] & dict(hash = listOfErrors[0][0])) == 1, 'unexpected number of rows in external table'
     
+    #---------------------CLEAN UP--------------------
+    os.rename(new_name, old_name) #switching from the new name back to the old name 
+
+    # print(f'this is the old_name after the asserts {old_name}')
+
+    # print(f'\n IN TEST: is the new file name a file? {os.path.isfile(new_name)}')
+    # print(f'\n IN TEST: is the old file name a file? {os.path.isfile(old_name)}')
+
+    listOfErrors = schema.external['local'].delete(delete_external_files=True)
+
+    print(len(listOfErrors))
\ No newline at end of file

From e0bd234dc0f30aea824b5afaa706c6ed3ac8c13f Mon Sep 17 00:00:00 2001
From: Carlos Ortiz <carlosortiz9204@gmail.com>
Date: Thu, 9 Sep 2021 15:02:37 -0500
Subject: [PATCH 07/16] Uncomment to re-enable test

---
 tests/test_external.py | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/tests/test_external.py b/tests/test_external.py
index 92228124f..ba651505c 100644
--- a/tests/test_external.py
+++ b/tests/test_external.py
@@ -26,32 +26,14 @@ def tearDown(self):
     dj.config['stores']['local']['location'] = current_location_local
 
 
-def test_external_put():
-    """
-    external storage put and get and remove
-    """
-    ext = ExternalTable(schema.connection, store='raw', database=schema.database)
-    initial_length = len(ext)
-    input_ = np.random.randn(3, 7, 8)
-    count = 7
-    extra = 3
-    for i in range(count):
-        hash1 = ext.put(pack(input_))
-    for i in range(extra):
-        hash2 = ext.put(pack(np.random.randn(4, 3, 2)))
-
-    fetched_hashes = ext.fetch('hash')
-    assert_true(all(hash in fetched_hashes for hash in (hash1, hash2)))
-    assert_equal(len(ext), initial_length + 1 + extra)
-
-    output_ = unpack(ext.get(hash1))
-    assert_array_equal(input_, output_)
-
 
 def test_s3_leading_slash(index=100, store='share'):
     """
     s3 external storage configured with leading slash
     """
+
+    oldConfig = dj.config['stores'][store]['location']
+
     value = np.array([1, 2, 3])
 
     id = index
@@ -102,6 +84,7 @@ def test_s3_leading_slash(index=100, store='share'):
     assert_true(np.array_equal(
         value, (SimpleRemote & 'simple={}'.format(id)).fetch1('item')))
 
+    dj.config['stores'][store]['location'] = oldConfig
 
 def test_file_leading_slash():
     """
@@ -115,6 +98,9 @@ def test_remove_fail():
     #print(json.dumps(dj.config['stores'], indent=4))
     #print(dj.config['stores']['local']['location'])
 
+    # oldConfig = dj.config['stores']
+    # dj.config['stores'] = stores_config
+
     dirName = dj.config['stores']['local']['location']
 
     #print('directory: ' + dirName)
@@ -189,4 +175,6 @@ def test_remove_fail():
 
     listOfErrors = schema.external['local'].delete(delete_external_files=True)
 
-    print(len(listOfErrors))
\ No newline at end of file
+    print(len(listOfErrors))
+
+    # dj.config['stores'] = oldConfig
\ No newline at end of file

From 8558adb065680ace49a84952d9963c9b1446bbd5 Mon Sep 17 00:00:00 2001
From: Carlos Ortiz <carlosortiz9204@gmail.com>
Date: Thu, 9 Sep 2021 15:10:39 -0500
Subject: [PATCH 08/16] Update lnx-docker and local-docker files with correct
 version of image

---
 LNX-docker-compose.yml   | 2 +-
 local-docker-compose.yml | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/LNX-docker-compose.yml b/LNX-docker-compose.yml
index d69677ccc..bee52b187 100644
--- a/LNX-docker-compose.yml
+++ b/LNX-docker-compose.yml
@@ -32,7 +32,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
diff --git a/local-docker-compose.yml b/local-docker-compose.yml
index 394240ad0..5d8bb1f9a 100644
--- a/local-docker-compose.yml
+++ b/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

From 56cf8b738b8201d7ae65cbe250f4624a44444ff9 Mon Sep 17 00:00:00 2001
From: Carlos Ortiz <carlosortiz9204@gmail.com>
Date: Thu, 9 Sep 2021 15:29:13 -0500
Subject: [PATCH 09/16] Fix issues with style tests in external.py

---
 datajoint/external.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/datajoint/external.py b/datajoint/external.py
index ef3f8ecf2..788eab734 100644
--- a/datajoint/external.py
+++ b/datajoint/external.py
@@ -351,9 +351,8 @@ def delete(self, *, delete_external_files=None, limit=None, display_progress=Tru
                         self._remove_external_file(external_path)
                     except Exception as error:
                         error_list.append((uuid, external_path, str(error)))
-                    else: 
+                    else:
                         (self & {'hash': uuid}).delete_quick(get_count=True)
-                    
             return error_list
 
 

From 84e4aa4aac92a3cf135230cf54a5593e1851ab37 Mon Sep 17 00:00:00 2001
From: Carlos Ortiz <carlosortiz9204@gmail.com>
Date: Thu, 9 Sep 2021 16:02:46 -0500
Subject: [PATCH 10/16] Remove print statements

---
 datajoint/external.py | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/datajoint/external.py b/datajoint/external.py
index 788eab734..8e4716a10 100644
--- a/datajoint/external.py
+++ b/datajoint/external.py
@@ -127,9 +127,6 @@ def _remove_external_file(self, external_path):
         if self.spec['protocol'] == 's3':
             self.s3.remove_object(external_path)
         elif self.spec['protocol'] == 'file':
-            print('we have reached the unlink statement in datajoint')
-            print(external_path)
-            print(Path(external_path).is_file())
             Path(external_path).unlink()
 
     def exists(self, external_filepath):
@@ -342,7 +339,6 @@ def delete(self, *, delete_external_files=None, limit=None, display_progress=Tru
             for uuid, external_path in items:
                 try:
                     count = len(self & {'hash': uuid})  # optimize
-                    print(f'\n\n\nCOUNT IS: {count}')
                 except Exception:
                     pass   # if delete failed, do not remove the external file
                 else:

From 5aa9b1ec61599128f8b1aacafa9b6db21e80e95b Mon Sep 17 00:00:00 2001
From: Carlos Ortiz <carlosortiz9204@gmail.com>
Date: Fri, 10 Sep 2021 15:50:23 -0500
Subject: [PATCH 11/16] Ignore FileNotFound Exception when deleting, optimize
 external table delete method, and re write test_removal_fail

---
 datajoint/external.py                         |  26 ++--
 .../4/c/4c16e9e15b64474e4b56ba22bb17faae      | Bin 49 -> 0 bytes
 .../4/c/4c16e9e15b64474e4b56ba22bb17faae      | Bin 49 -> 0 bytes
 tests/test_external.py                        | 120 +++++-------------
 4 files changed, 48 insertions(+), 98 deletions(-)
 delete mode 100644 djtest_extern/4/c/4c16e9e15b64474e4b56ba22bb17faae
 delete mode 100644 tests/djtest_extern/4/c/4c16e9e15b64474e4b56ba22bb17faae

diff --git a/datajoint/external.py b/datajoint/external.py
index 8e4716a10..2cfd60c87 100644
--- a/datajoint/external.py
+++ b/datajoint/external.py
@@ -127,7 +127,10 @@ def _remove_external_file(self, external_path):
         if self.spec['protocol'] == 's3':
             self.s3.remove_object(external_path)
         elif self.spec['protocol'] == 'file':
-            Path(external_path).unlink()
+            try:
+                Path(external_path).unlink()
+            except FileNotFoundError:
+                pass
 
     def exists(self, external_filepath):
         """
@@ -337,18 +340,19 @@ def delete(self, *, delete_external_files=None, limit=None, display_progress=Tru
             # delete items one by one, close to transaction-safe
             error_list = []
             for uuid, external_path in items:
-                try:
-                    count = len(self & {'hash': uuid})  # optimize
-                except Exception:
-                    pass   # if delete failed, do not remove the external file
-                else:
-                    assert count in (0, 1)
+                row = (self & {'hash': uuid}).fetch()
+                if row.size:
                     try:
-                        self._remove_external_file(external_path)
-                    except Exception as error:
-                        error_list.append((uuid, external_path, str(error)))
+                        (self & {'hash': uuid}).delete_quick()
+                    except Exception:
+                        pass   # if delete failed, do not remove the external file
                     else:
-                        (self & {'hash': uuid}).delete_quick(get_count=True)
+                        try:
+                            self._remove_external_file(external_path)
+                        except Exception as error:
+                            # adding row back into table after failed delete
+                            self.insert1(row[0])
+                            error_list.append((uuid, external_path, str(error)))
             return error_list
 
 
diff --git a/djtest_extern/4/c/4c16e9e15b64474e4b56ba22bb17faae b/djtest_extern/4/c/4c16e9e15b64474e4b56ba22bb17faae
deleted file mode 100644
index 8b1bfda07ffaabdbd8f05cd123d824ae8321f6ff..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001

literal 49
ccmYevGGJh0W`F<|D9y#lz=*_VLSi!m05nDcHvj+t

diff --git a/tests/djtest_extern/4/c/4c16e9e15b64474e4b56ba22bb17faae b/tests/djtest_extern/4/c/4c16e9e15b64474e4b56ba22bb17faae
deleted file mode 100644
index 8b1bfda07ffaabdbd8f05cd123d824ae8321f6ff..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001

literal 49
ccmYevGGJh0W`F<|D9y#lz=*_VLSi!m05nDcHvj+t

diff --git a/tests/test_external.py b/tests/test_external.py
index ba651505c..04791299c 100644
--- a/tests/test_external.py
+++ b/tests/test_external.py
@@ -3,16 +3,10 @@
 from nose.tools import assert_true, assert_equal
 from datajoint.external import ExternalTable
 from datajoint.blob import pack, unpack
-
-
 import datajoint as dj
 from .schema_external import stores_config, SimpleRemote, Simple, schema
+import os
 
-import json
-import os 
-from os import stat
-import pwd
-from pwd import getpwuid
 current_location_s3 = dj.config['stores']['share']['location']
 current_location_local = dj.config['stores']['local']['location']
 
@@ -26,6 +20,27 @@ def tearDown(self):
     dj.config['stores']['local']['location'] = current_location_local
 
 
+def test_external_put():
+    """
+    external storage put and get and remove
+    """
+    ext = ExternalTable(schema.connection, store='raw', database=schema.database)
+    initial_length = len(ext)
+    input_ = np.random.randn(3, 7, 8)
+    count = 7
+    extra = 3
+    for i in range(count):
+        hash1 = ext.put(pack(input_))
+    for i in range(extra):
+        hash2 = ext.put(pack(np.random.randn(4, 3, 2)))
+
+    fetched_hashes = ext.fetch('hash')
+    assert_true(all(hash in fetched_hashes for hash in (hash1, hash2)))
+    assert_equal(len(ext), initial_length + 1 + extra)
+
+    output_ = unpack(ext.get(hash1))
+    assert_array_equal(input_, output_)
+
 
 def test_s3_leading_slash(index=100, store='share'):
     """
@@ -86,95 +101,26 @@ def test_s3_leading_slash(index=100, store='share'):
 
     dj.config['stores'][store]['location'] = oldConfig
 
+
 def test_file_leading_slash():
     """
     file external storage configured with leading slash
     """
     test_s3_leading_slash(index=200, store='local')
 
-def test_remove_fail():
-    #https://github.com/datajoint/datajoint-python/issues/953
-
-    #print(json.dumps(dj.config['stores'], indent=4))
-    #print(dj.config['stores']['local']['location'])
 
-    # oldConfig = dj.config['stores']
-    # dj.config['stores'] = stores_config
-
-    dirName = dj.config['stores']['local']['location']
-
-    #print('directory: ' + dirName)
-
-
-
-    data = dict(simple = 2, item = [1, 2, 3])
+def test_remove_fail():
+    # https://github.com/datajoint/datajoint-python/issues/953
+    data = dict(simple=2, item=[1, 2, 3])
     Simple.insert1(data)
-
-    #print('location')
-    # print('\n IN TEST: BEFORE DELETE: list of dir stores, local, location')
-    print('stores location -----------\n')
-    print(dj.config['stores']['local']['location'])
-    print('local location -----------\n')
-    print(schema.external['local'])
-    print('----------------------------')
-
     path1 = dj.config['stores']['local']['location'] + '/djtest_extern/4/c/'
-
-    argDir = dj.config['stores']['local']['location'] + '/djtest_extern/4/c/'
-
-    print(f'argDir----------\n{argDir}\n')
-
-    path2 = os.listdir(argDir)
-
-    # print(path1 + path2[0])
-
-    old_name = path1 + path2[0]
-
-    new_name = "/tmp/newfile"
-
-    os.rename(old_name, new_name)
-
-    # print(f'\n IN TEST: is the new file name a file? {os.path.isfile(new_name)}')
-    # print(f'\n IN TEST: is the old file name a file? {os.path.isfile(old_name)}')
-
-    # print(os.listdir(dj.config['stores']['local']['location'] + '/djtest_extern/4/c/'))
-
-    # st = stat(path1 + path2[0])
-    # print(bool(st.st_mode & stat.S_IXUSR))
-
-    #print(getpwuid(stat(path3).st_uid).pw_name)
-
-    # print(f' IN TEST: simple table before delete {Simple()}')
+    currentMode = int(oct(os.stat(path1).st_mode), 8)
+    os.chmod(path1, 0o40555)
     (Simple & 'simple=2').delete()
-    # print(f' IN TEST: simple table after delete {Simple()}')
-    # print(' IN TEST: -------------showing external store before delete with flag---------')
-    # print(schema.external['local'])
     listOfErrors = schema.external['local'].delete(delete_external_files=True)
-    # print(f' IN TEST: list of errors: {listOfErrors}')
-    # print(' IN TEST: list of dir stores, local, location')
-    # print(os.listdir(dj.config['stores']['local']['location'] + '/djtest_extern/4/c'))
-    # print(' IN TEST: -------------showing external store after delete with flag---------')
-    # print(schema.external['local'])
-
-    # print(f'\n IN TEST: is this the UID or HASH? {listOfErrors[0][0]}')
-
-    # LENGTH_OF_QUERY = len(schema.external['local'] & dict(hash = listOfErrors[0][0]))
-
-    # print(f'\n IN TEST: WHAT IS THE LENGTH OF THIS? {LENGTH_OF_QUERY}')
-
     assert len(listOfErrors) == 1, 'unexpected number of errors'
-    assert len(schema.external['local'] & dict(hash = listOfErrors[0][0])) == 1, 'unexpected number of rows in external table'
-    
-    #---------------------CLEAN UP--------------------
-    os.rename(new_name, old_name) #switching from the new name back to the old name 
-
-    # print(f'this is the old_name after the asserts {old_name}')
-
-    # print(f'\n IN TEST: is the new file name a file? {os.path.isfile(new_name)}')
-    # print(f'\n IN TEST: is the old file name a file? {os.path.isfile(old_name)}')
-
-    listOfErrors = schema.external['local'].delete(delete_external_files=True)
-
-    print(len(listOfErrors))
-
-    # dj.config['stores'] = oldConfig
\ No newline at end of file
+    assert len(schema.external['local'] & dict(hash=listOfErrors[0][0])) == 1, 'unexpec' + \
+        'number of rows in external table'
+    # ---------------------CLEAN UP--------------------
+    os.chmod(path1, currentMode)
+    listOfErrors = schema.external['local'].delete(delete_external_files=True)
\ No newline at end of file

From 957c563435fd2a2eb29fbf958d77b9953d261722 Mon Sep 17 00:00:00 2001
From: Carlos Ortiz <carlosortiz9204@gmail.com>
Date: Fri, 10 Sep 2021 16:01:44 -0500
Subject: [PATCH 12/16] Update CHANGELOG and DOCS-PART

---
 CHANGELOG.md                        | 1 +
 docs-parts/intro/Releases_lang1.rst | 1 +
 2 files changed, 2 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 671c0f0d9..706516025 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ### 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
+* Bugfix - `ExternalTable.delete` should not remove row on error (#953) PR #956
 
 ### 0.13.2 -- May 7, 2021
 * Update `setuptools_certificate` dependency to new name `otumat`
diff --git a/docs-parts/intro/Releases_lang1.rst b/docs-parts/intro/Releases_lang1.rst
index 5090d0d89..4216cde8b 100644
--- a/docs-parts/intro/Releases_lang1.rst
+++ b/docs-parts/intro/Releases_lang1.rst
@@ -2,6 +2,7 @@
 ----------------------
 * 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
+* Bugfix - `ExternalTable.delete` should not remove row on error (#953) PR #956
 
 0.13.2 -- May 7, 2021
 ----------------------

From cdae1d5520bf5edb0083a07d42bde74aeca58714 Mon Sep 17 00:00:00 2001
From: zitrosolrac <37351041+zitrosolrac@users.noreply.github.com>
Date: Fri, 10 Sep 2021 16:34:13 -0500
Subject: [PATCH 13/16] Update tests/test_external.py

Co-authored-by: Raphael Guzman <38401847+guzman-raphael@users.noreply.github.com>
---
 tests/test_external.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test_external.py b/tests/test_external.py
index 04791299c..fb85a2afc 100644
--- a/tests/test_external.py
+++ b/tests/test_external.py
@@ -119,8 +119,8 @@ def test_remove_fail():
     (Simple & 'simple=2').delete()
     listOfErrors = schema.external['local'].delete(delete_external_files=True)
     assert len(listOfErrors) == 1, 'unexpected number of errors'
-    assert len(schema.external['local'] & dict(hash=listOfErrors[0][0])) == 1, 'unexpec' + \
-        'number of rows in external table'
+    assert len(schema.external['local'] & dict(hash=listOfErrors[0][0])) == 1, \
+        'unexpected number of rows in external table'
     # ---------------------CLEAN UP--------------------
     os.chmod(path1, currentMode)
     listOfErrors = schema.external['local'].delete(delete_external_files=True)
\ No newline at end of file

From 554fc714b696ba2902f5479acbf2fc9040c9b728 Mon Sep 17 00:00:00 2001
From: Carlos Ortiz <carlosortiz9204@gmail.com>
Date: Fri, 10 Sep 2021 16:35:07 -0500
Subject: [PATCH 14/16] Fixed parenthesis issue and uncommented logging
 console.

---
 datajoint/external.py | 5 +++--
 tests/__init__.py     | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/datajoint/external.py b/datajoint/external.py
index 99f762ea8..83b96531a 100644
--- a/datajoint/external.py
+++ b/datajoint/external.py
@@ -353,8 +353,9 @@ def delete(self, *, delete_external_files=None, limit=None, display_progress=Tru
                         except Exception as error:
                             # adding row back into table after failed delete
                             self.insert1(row[0])
-                            error_list.append((uuid, external_path, str(error) if errors_as_string else error))))
-            return error_list
+                            error_list.append((uuid, external_path,
+                                               str(error) if errors_as_string else error))
+        return error_list
 
 
 class ExternalMapping(Mapping):
diff --git a/tests/__init__.py b/tests/__init__.py
index d50672b49..6b802e332 100644
--- a/tests/__init__.py
+++ b/tests/__init__.py
@@ -20,7 +20,7 @@
 __author__ = 'Edgar Walker, Fabian Sinz, Dimitri Yatsenko, Raphael Guzman'
 
 # turn on verbose logging
-#logging.basicConfig(level=logging.DEBUG)
+logging.basicConfig(level=logging.DEBUG)
 
 __all__ = ['__author__', 'PREFIX', 'CONN_INFO']
 

From d4b972f16158a84c0f1555763966cc6c0de2df0d Mon Sep 17 00:00:00 2001
From: Carlos Ortiz <carlosortiz9204@gmail.com>
Date: Fri, 10 Sep 2021 16:39:54 -0500
Subject: [PATCH 15/16] Fix return statement placement

---
 datajoint/external.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datajoint/external.py b/datajoint/external.py
index 83b96531a..304b8c2c6 100644
--- a/datajoint/external.py
+++ b/datajoint/external.py
@@ -355,7 +355,7 @@ def delete(self, *, delete_external_files=None, limit=None, display_progress=Tru
                             self.insert1(row[0])
                             error_list.append((uuid, external_path,
                                                str(error) if errors_as_string else error))
-        return error_list
+            return error_list
 
 
 class ExternalMapping(Mapping):

From f8e8196721b56c7d62e728c1ae0c9da69062a8b9 Mon Sep 17 00:00:00 2001
From: zitrosolrac <37351041+zitrosolrac@users.noreply.github.com>
Date: Mon, 13 Sep 2021 17:17:52 -0500
Subject: [PATCH 16/16] Update datajoint/external.py

Co-authored-by: Raphael Guzman <38401847+guzman-raphael@users.noreply.github.com>
---
 datajoint/external.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datajoint/external.py b/datajoint/external.py
index 304b8c2c6..bfac78d13 100644
--- a/datajoint/external.py
+++ b/datajoint/external.py
@@ -352,7 +352,7 @@ def delete(self, *, delete_external_files=None, limit=None, display_progress=Tru
                             self._remove_external_file(external_path)
                         except Exception as error:
                             # adding row back into table after failed delete
-                            self.insert1(row[0])
+                            self.insert1(row[0], skip_duplicates=True)
                             error_list.append((uuid, external_path,
                                                str(error) if errors_as_string else error))
             return error_list