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

src/deadline/job_attachments/download.py

@@ -389,7 +389,7 @@ def download_file(
             status_code=status_code,
             bucket_name=s3_bucket,
             key_or_prefix=s3_key,
-            message=f"{status_code_guidance.get(status_code, '')} {str(exc)}",
+            message=f"{status_code_guidance.get(status_code, '')} {str(exc)} (Failed to download the file to {str(local_file_name)})",
         ) from exc
 
     download_logger.debug(f"Downloaded {file.path} to {str(local_file_name)}")

src/deadline/job_attachments/exceptions.py

@@ -48,13 +48,13 @@ def __init__(
         self.key_or_prefix = key_or_prefix
 
         message_parts = [
-            f"Error {action} in bucket '{bucket_name}'. Target key or prefix: '{key_or_prefix}'.",
+            f"Error {action} in bucket '{bucket_name}', Target key or prefix: '{key_or_prefix}'",
             f"HTTP Status Code: {status_code}",
         ]
         if message:
             message_parts.append(message)
 
-        super().__init__(" ".join(message_parts))
+        super().__init__(", ".join(message_parts))
 
 
 class MissingS3BucketError(JobAttachmentsError):
@@ -89,7 +89,7 @@ class MissingManifestError(JobAttachmentsError):
 
 class MissingAssetRootError(JobAttachmentsError):
     """
-    Exception for when trying to retrieve asset root from metatdata (in S3) that doesn't exist.
+    Exception for when trying to retrieve asset root from metadata (in S3) that doesn't exist.
     """
 
 

src/deadline/job_attachments/upload.py

@@ -342,7 +342,7 @@ def upload_file_to_s3(
                     self._upload_part,
                     s3_bucket,
                     s3_upload_key,
-                    local_path,
+                    str(local_path),
                     upload_id,
                     chunk_size,
                     progress_tracker,
@@ -397,7 +397,7 @@ def upload_file_to_s3(
                 status_code=status_code,
                 bucket_name=s3_bucket,
                 key_or_prefix=s3_upload_key,
-                message=status_code_guidance.get(status_code, ""),
+                message=f"{status_code_guidance.get(status_code, '')} {str(exc)} (Failed to upload {str(local_path)})",
             ) from exc
         except Exception as exc:
             if upload_id:

test/unit/deadline_job_attachments/test_download.py

@@ -1650,16 +1650,16 @@ def test_get_asset_root_from_s3_error_message_on_not_found(self):
         with stubber, patch(
             f"{deadline.__package__}.job_attachments.download.get_s3_client", return_value=s3_client
         ):
-            with pytest.raises(JobAttachmentsS3ClientError) as exc:
-                _get_asset_root_from_s3("not/existed/test.txt", "my-bucket")
-                assert isinstance(exc.value.__cause__, ClientError)
-                assert (
-                    exc.value.__cause__.response["ResponseMetadata"]["HTTPStatusCode"] == 404  # type: ignore[attr-defined]
-                )
-                assert (
-                    "Error checking if object exists in bucket 'my-bucket'. Target key or prefix: 'not/existed/test.txt'. "
-                    "HTTP Status Code: 404 Not found. "
-                ) in str(exc.value)
+            with pytest.raises(JobAttachmentsS3ClientError) as err:
+                _get_asset_root_from_s3("not/existed/test.txt", "test-bucket")
+            assert isinstance(err.value.__cause__, ClientError)
+            assert (
+                err.value.__cause__.response["ResponseMetadata"]["HTTPStatusCode"] == 404  # type: ignore[attr-defined]
+            )
+            assert (
+                "Error checking if object exists in bucket 'test-bucket', Target key or prefix: 'not/existed/test.txt', "
+                "HTTP Status Code: 404, Not found. "
+            ) in str(err.value)
 
     @mock_sts
     def test_get_manifest_from_s3_error_message_on_access_denied(self):
@@ -1680,15 +1680,15 @@ def test_get_manifest_from_s3_error_message_on_access_denied(self):
             f"{deadline.__package__}.job_attachments.download.get_s3_client", return_value=s3_client
         ):
             with pytest.raises(JobAttachmentsS3ClientError) as exc:
-                get_manifest_from_s3("test-key", "my-bucket")
-                assert isinstance(exc.value.__cause__, ClientError)
-                assert (
-                    exc.value.__cause__.response["ResponseMetadata"]["HTTPStatusCode"] == 403  # type: ignore[attr-defined]
-                )
-                assert (
-                    "Error downloading binary file in bucket 'my-bucket'. Target key or prefix: 'test-key'. "
-                    "HTTP Status Code: 403 Forbidden or Access denied. "
-                ) in str(exc.value)
+                get_manifest_from_s3("test-key", "test-bucket")
+            assert isinstance(exc.value.__cause__, ClientError)
+            assert (
+                exc.value.__cause__.response["ResponseMetadata"]["HTTPStatusCode"] == 403  # type: ignore[attr-defined]
+            )
+            assert (
+                "Error downloading binary file in bucket 'test-bucket', Target key or prefix: 'test-key', "
+                "HTTP Status Code: 403, Forbidden or Access denied. "
+            ) in str(exc.value)
 
     @mock_sts
     def test_get_tasks_manifests_keys_from_s3_error_message_on_access_denied(self):
@@ -1710,17 +1710,17 @@ def test_get_tasks_manifests_keys_from_s3_error_message_on_access_denied(self):
         ):
             with pytest.raises(JobAttachmentsS3ClientError) as exc:
                 _get_tasks_manifests_keys_from_s3(
-                    "test_prefix",
-                    "my-bucket",
+                    "assetRoot",
+                    "test-bucket",
                 )
-                assert isinstance(exc.value.__cause__, ClientError)
-                assert (
-                    exc.value.__cause__.response["ResponseMetadata"]["HTTPStatusCode"] == 403  # type: ignore[attr-defined]
-                )
-                assert (
-                    "Error listing bucket contents in bucket 'my-bucket'. Target key or prefix: 'test_prefix'. "
-                    "HTTP Status Code: 403 Forbidden or Access denied. "
-                ) in str(exc.value)
+            assert isinstance(exc.value.__cause__, ClientError)
+            assert (
+                exc.value.__cause__.response["ResponseMetadata"]["HTTPStatusCode"] == 403  # type: ignore[attr-defined]
+            )
+            assert (
+                "Error listing bucket contents in bucket 'test-bucket', Target key or prefix: 'assetRoot', "
+                "HTTP Status Code: 403, Forbidden or Access denied. "
+            ) in str(exc.value)
 
     @mock_sts
     def test_download_file_error_message_on_access_denied(self):
@@ -1746,16 +1746,18 @@ def test_download_file_error_message_on_access_denied(self):
         ), patch(f"{deadline.__package__}.job_attachments.download.Path.mkdir"):
             with pytest.raises(JobAttachmentsS3ClientError) as exc:
                 download_file(
-                    file_path, "/home/username/assets", "my-bucket", "rootPrefix/Data", s3_client
+                    file_path, "/home/username/assets", "test-bucket", "rootPrefix/Data", s3_client
                 )
-                assert isinstance(exc.value.__cause__, ClientError)
-                assert (
-                    exc.value.__cause__.response["ResponseMetadata"]["HTTPStatusCode"] == 403  # type: ignore[attr-defined]
-                )
-                assert (
-                    "Error downloading file in bucket 'my-bucket'. Target key or prefix: 'rootPrefix/Data/relative_file_path'. "
-                    "HTTP Status Code: 403 Forbidden or Access denied. "
-                ) in str(exc.value)
+            assert isinstance(exc.value.__cause__, ClientError)
+            assert (
+                exc.value.__cause__.response["ResponseMetadata"]["HTTPStatusCode"] == 403  # type: ignore[attr-defined]
+            )
+            assert (
+                "Error downloading file in bucket 'test-bucket', Target key or prefix: 'rootPrefix/Data/input1', "
+                "HTTP Status Code: 403, Forbidden or Access denied. "
+            ) in str(exc.value)
+            failed_file_path = Path("/home/username/assets/inputs/input1.txt")
+            assert (f"(Failed to download the file to {str(failed_file_path)})") in str(exc.value)
 
 
 @pytest.mark.parametrize("manifest_version", [ManifestVersion.v2023_03_03])

test/unit/deadline_job_attachments/test_upload.py

@@ -1231,8 +1231,8 @@ def test_file_already_uploaded_bucket_in_different_account(self):
                 err.value.__cause__.response["ResponseMetadata"]["HTTPStatusCode"] == 403  # type: ignore[attr-defined]
             )
             assert (
-                "Error checking if object exists in bucket 'test-bucket'. Target key or prefix: 'test_key'. "
-                "HTTP Status Code: 403 Access denied. Ensure that the bucket is in the account 123456789012, "
+                "Error checking if object exists in bucket 'test-bucket', Target key or prefix: 'test_key', "
+                "HTTP Status Code: 403, Access denied. Ensure that the bucket is in the account 123456789012, "
                 "and your AWS IAM Role or User has the 's3:ListBucket' permission for this bucket."
             ) in str(err.value)
 
@@ -1265,8 +1265,8 @@ def test_filter_objects_to_upload_bucket_in_different_account(self):
                 err.value.__cause__.response["ResponseMetadata"]["HTTPStatusCode"] == 403  # type: ignore[attr-defined]
             )
             assert (
-                "Error listing bucket contents in bucket 'test-bucket'. Target key or prefix: 'test_prefix'. "
-                "HTTP Status Code: 403 Forbidden or Access denied. "
+                "Error listing bucket contents in bucket 'test-bucket', Target key or prefix: 'test_prefix', "
+                "HTTP Status Code: 403, Forbidden or Access denied. "
             ) in str(err.value)
 
     @mock_sts
@@ -1300,10 +1300,61 @@ def test_upload_bytes_to_s3_bucket_in_different_account(self):
                 err.value.__cause__.response["ResponseMetadata"]["HTTPStatusCode"] == 403  # type: ignore[attr-defined]
             )
             assert (
-                "Error uploading binary file in bucket 'test-bucket'. Target key or prefix: 'test_key'. "
-                "HTTP Status Code: 403 Forbidden or Access denied. "
+                "Error uploading binary file in bucket 'test-bucket', Target key or prefix: 'test_key', "
+                "HTTP Status Code: 403, Forbidden or Access denied. "
             ) in str(err.value)
 
+    @mock_sts
+    def test_upload_file_to_s3_upload_part_failure(self, tmp_path: Path):
+        """
+        Test that the error message correctly includes the failed file path when a multipart upload
+        (upload_part method) fails with a 400 error code.
+        """
+        bucket_name = self.job_attachment_s3_settings.s3BucketName
+        s3 = boto3.client("s3")
+        stubber = Stubber(s3)
+        stubber.add_response(
+            "create_multipart_upload",
+            service_response={"UploadId": "test_upload_id"},
+            expected_params={
+                "Bucket": bucket_name,
+                "Key": "test_key",
+                "ExpectedBucketOwner": "123456789012",
+            },
+        )
+        stubber.add_client_error(
+            "upload_part",
+            service_error_code="InvalidPart",
+            service_message="Invalid Part",
+            http_status_code=400,
+        )
+        stubber.add_response(
+            "abort_multipart_upload",
+            service_response={},
+            expected_params={
+                "Bucket": bucket_name,
+                "Key": "test_key",
+                "UploadId": "test_upload_id",
+                "ExpectedBucketOwner": "123456789012",
+            },
+        )
+
+        uploader = S3AssetUploader()
+        uploader._s3 = s3
+
+        file = tmp_path / "test_file.txt"
+        file.write_text("test file")
+
+        with stubber:
+            with pytest.raises(JobAttachmentsS3ClientError) as err:
+                uploader.upload_file_to_s3(file, bucket_name, "test_key")
+            assert isinstance(err.value.__cause__, ClientError)
+            assert err.value.__cause__.response["ResponseMetadata"]["HTTPStatusCode"] == 400
+            assert (
+                "Error uploading file in bucket 'test-bucket', Target key or prefix: 'test_key', HTTP Status Code: 400"
+            ) in str(err.value)
+            assert (f"(Failed to upload {str(file)})") in str(err.value)
+
     @mock_sts
     def test_upload_file_to_s3_bucket_in_different_account(self, tmp_path: Path):
         """
@@ -1338,9 +1389,10 @@ def test_upload_file_to_s3_bucket_in_different_account(self, tmp_path: Path):
                 err.value.__cause__.response["ResponseMetadata"]["HTTPStatusCode"] == 403  # type: ignore[attr-defined]
             )
             assert (
-                "Error uploading file in bucket 'test-bucket'. Target key or prefix: 'test_key'. "
-                "HTTP Status Code: 403 Forbidden or Access denied. "
+                "Error uploading file in bucket 'test-bucket', Target key or prefix: 'test_key', "
+                "HTTP Status Code: 403, Forbidden or Access denied. "
             ) in str(err.value)
+            assert (f"(Failed to upload {str(file)})") in str(err.value)
 
     @pytest.mark.parametrize(
         "manifest_version",