Raise exception if in debug mode when uploading documents in tests #2447
Conversation
kevincarrogan
force-pushed
the
fix-e2e-uploading-files
branch
from
bd54308 to
ff77c2b
Compare
![code-review-doctor[bot]](https://avatars.githubusercontent.com/in/83430?s=60&v=4){kind=small}
| @@ -32,6 +32,8 @@ def get(self, request): | |||
| try: | |||
| s3.upload_file(file_to_upload_abs_path, settings.AWS_STORAGE_BUCKET_NAME, s3_key) | |||
| except Exception as e: # noqa | |||
| if settings.DEBUG: | |||
There was a problem hiding this comment.
Using settings.DEBUG here makes it harder to unit test or manually test this code in isolation because the value of settings.DEBUG affects other Django behaviors such as showing technical error log when uncaught exceptions occur, sending admin emails, recording every SQL query executed, and many more.
Instead, consider adding a feature flag specifically for this rather than using settings.DEBUG. More details.
There was a problem hiding this comment.
Looks good. Worth considering though. View full project report here.
| @@ -32,6 +32,8 @@ def get(self, request): | |||
| try: | |||
| s3.upload_file(file_to_upload_abs_path, settings.AWS_STORAGE_BUCKET_NAME, s3_key) | |||
| except Exception as e: # noqa | |||
| if settings.DEBUG: | |||
There was a problem hiding this comment.
Using settings.DEBUG here makes it harder to unit test or manually test this code in isolation because the value of settings.DEBUG affects other Django behaviors such as showing technical error log when uncaught exceptions occur, sending admin emails, recording every SQL query executed, and many more.
Instead, consider adding a feature flag specifically for this rather than using settings.DEBUG. More.
Aim
When in debug mode, if there is a file upload issue then raise the exception.
This should make it easier to debug exactly what failed when an upload error occurs, especially when running e2e tests in CI.