Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(job_attachments): include file path in error message for upload failures #128

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

gahyusuh
Copy link
Contributor

@gahyusuh gahyusuh commented Dec 20, 2023

- Added file path in custom error messages when multipart uploads fail due to a ClientError.
- Corrected indentation in existing unit tests for error message verification (in test_download.py).

What was the problem/requirement? (What/Why)

Previously, when multipart upload failed, the custom error message (in the JobAttachmentsS3ClientError) did not provide an info of which file had failed to upload. This lack of detail made debugging more difficult.

What was the solution? (How)

This PR has the following changes:

  • Enhanced the error messages to include the file path when multipart uploads fail. The updated error message looks like below:
    Error uploading file in bucket 'test-bucket', Target key or prefix: 'test_key', HTTP Status Code: 400,  An error occurred (InvalidPart) when calling the UploadPart operation: Invalid Part (Failed to upload /tmp/pytest-of-gahyusuh/pytest-4555/popen-gw7/test_upload_error_message0/test_file.txt)
    
  • Fixed some existing unit tests in test_download.py to have correct indentation, ensuring that the assertions for verifying error messages are properly executed.

What is the impact of this change?

Provide clearer context for debugging, helping users to identify the file in question and address issues with file uploads.

How was this change tested?

  • Added a new unit test test_upload_file_to_s3_upload_part_failure to verify that the error message includes the file path during a failed upload.
  • Ran hatch run test, ensuring that the corrected unit tests are properly executed and all tests successfully passed.
  • Checked that job submission with some job attachments was working.

Was this change documented?

No.

Is this a breaking change?

No.

@gahyusuh gahyusuh force-pushed the gahyusuh/multipart_upload_error_msg branch 2 times, most recently from 4aa53eb to 8c2fc6b Compare December 20, 2023 14:11
@gahyusuh gahyusuh marked this pull request as ready for review December 20, 2023 15:00
@gahyusuh gahyusuh requested a review from a team as a code owner December 20, 2023 15:00
…ailures

- Added file path in custom error messages when multipart uploads fail due to a ClientError.
- Corrected indentation in existing unit tests for error message verification (in test_download.py).

Signed-off-by: Gahyun Suh <[email protected]>
@gahyusuh gahyusuh force-pushed the gahyusuh/multipart_upload_error_msg branch from 8c2fc6b to 12e2c73 Compare December 20, 2023 15:01
@gahyusuh gahyusuh merged commit 06cd864 into mainline Dec 20, 2023
18 checks passed
@gahyusuh gahyusuh deleted the gahyusuh/multipart_upload_error_msg branch December 20, 2023 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants