Skip to content

Commit 3ca781c

Browse files
authored
feat(projectOwnership): log errors in transferring attachments DEV-842 (#6060)
### 💭 Notes Right now we are swallowing errors when trying to move attachments. The jobs occasionally fail and the lack of logging makes it difficult to diagnose, so this logs the error before returning False. ### 👀 Preview steps Make sure you are using s3 storage 1. ℹ️ have at least 2 accounts and a project with an image question 2. Add a few submissions with attachments to the project 3. Transfer the project to another user 4. Before accepting the transfer, update `kpi.fields.file.py:20-23` with ``` from django.apps import apps Attachment = apps.get_model(model_name='Attachment', app_label='logger') if isinstance(self.instance, Attachment): name = 'bad name' else: name = self.name copy_source = { 'Bucket': self.storage.bucket.name, 'Key': name, } ``` Make sure to restart the worker to pick up the changes 5. As the other user, accept the transfer invitation 6. 🔴 [on main] Notice there is only an `AsyncTaskException` in the worker logs 7. 🟢 [on PR] Notice there is both an `AsyncTaskException` and a more detailed `ClientError` in the logs
1 parent 68d0cb4 commit 3ca781c

File tree

2 files changed

+10
-5
lines changed

2 files changed

+10
-5
lines changed

kpi/fields/file.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
from django.db.models.fields.files import FieldFile
66
from storages.backends.s3 import ClientError, S3Storage
77

8+
from kpi.utils.log import logging
9+
810

911
class ExtendedFieldFile(FieldFile):
1012

@@ -17,12 +19,15 @@ def move(self, target_folder: str) -> bool:
1719
if isinstance(self.storage, S3Storage):
1820
copy_source = {
1921
'Bucket': self.storage.bucket.name,
20-
'Key': self.name
22+
'Key': self.name,
2123
}
2224
try:
2325
self.storage.bucket.copy(copy_source, new_path)
2426
self.storage.delete(old_path)
25-
except ClientError:
27+
except ClientError as e:
28+
logging.error(
29+
f'Error copying {old_path} to {new_path}: {e}', exc_info=True
30+
)
2631
return False
2732

2833
self.name = new_path
@@ -38,8 +43,8 @@ def move(self, target_folder: str) -> bool:
3843
self.save(filename, f, save=False)
3944
self.storage.delete(old_path)
4045
success = True
41-
except FileNotFoundError:
42-
pass
46+
except FileNotFoundError as fe:
47+
logging.error(fe, exc_info=True)
4348
finally:
4449
# Restore `upload_to`
4550
self.field.upload_to = upload_to

kpi/tests/test_extended_file_field.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ class ExtendedFileFieldTestCase(TestCase):
1111
fixtures = ['test_data']
1212

1313
def test_move_file(self):
14-
# Use AssetFile but could not any model which uses `ExtendedFileField`
14+
# Use AssetFile but could be any model which uses `ExtendedFileField`
1515
asset = Asset.objects.get(pk=1)
1616
asset_file = AssetFile(
1717
asset=asset, user=asset.owner, file_type=AssetFile.FORM_MEDIA

0 commit comments

Comments
 (0)