From 2cb994cdc9c0e51319e9059459ac1cccd7730673 Mon Sep 17 00:00:00 2001 From: Christopher Adams Date: Mon, 27 Jun 2022 10:44:46 -0400 Subject: [PATCH] Returned future result in S3Transfer's download and upload - Adds a `return` statement to S3Transfer's methods `upload_file()` and `download_file()`. - This gives them the same return behavior as `copy()`, `upload_fileobj()`, and `download_fileobj()` inside `s3/inject.py`. These functions return the value for their future objects. - Returning the value will allow integrations to access metadata returned by the service, such as the "ETag" for uploaded entities for example. --- boto3/s3/transfer.py | 8 ++++++-- tests/unit/s3/test_transfer.py | 6 ++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/boto3/s3/transfer.py b/boto3/s3/transfer.py index 3d572981e0..ee824b1333 100644 --- a/boto3/s3/transfer.py +++ b/boto3/s3/transfer.py @@ -276,6 +276,8 @@ def upload_file( .. seealso:: :py:meth:`S3.Client.upload_file` :py:meth:`S3.Client.upload_fileobj` + + :returns: The result of the specific manager's upload function. """ if not isinstance(filename, str): raise ValueError('Filename must be a string') @@ -285,7 +287,7 @@ def upload_file( filename, bucket, key, extra_args, subscribers ) try: - future.result() + return future.result() # If a client error was raised, add the backwards compatibility layer # that raises a S3UploadFailedError. These specific errors were only # ever thrown for upload_parts but now can be thrown for any related @@ -308,6 +310,8 @@ def download_file( .. seealso:: :py:meth:`S3.Client.download_file` :py:meth:`S3.Client.download_fileobj` + + :returns: The result of the specific manager's download function. """ if not isinstance(filename, str): raise ValueError('Filename must be a string') @@ -317,7 +321,7 @@ def download_file( bucket, key, filename, extra_args, subscribers ) try: - future.result() + return future.result() # This is for backwards compatibility where when retries are # exceeded we need to throw the same error from boto3 instead of # s3transfer's built in RetriesExceededError as current users are diff --git a/tests/unit/s3/test_transfer.py b/tests/unit/s3/test_transfer.py index 9f3456f196..f725831a19 100644 --- a/tests/unit/s3/test_transfer.py +++ b/tests/unit/s3/test_transfer.py @@ -141,24 +141,26 @@ def assert_callback_wrapped_in_subscriber(self, call_args): def test_upload_file(self): extra_args = {'ACL': 'public-read'} - self.transfer.upload_file( + upload_metadata = self.transfer.upload_file( 'smallfile', 'bucket', 'key', extra_args=extra_args ) self.manager.upload.assert_called_with( 'smallfile', 'bucket', 'key', extra_args, None ) + self.assertIsNotNone(upload_metadata) def test_download_file(self): extra_args = { 'SSECustomerKey': 'foo', 'SSECustomerAlgorithm': 'AES256', } - self.transfer.download_file( + download_metadata = self.transfer.download_file( 'bucket', 'key', '/tmp/smallfile', extra_args=extra_args ) self.manager.download.assert_called_with( 'bucket', 'key', '/tmp/smallfile', extra_args, None ) + self.assertIsNotNone(download_metadata) def test_upload_wraps_callback(self): self.transfer.upload_file(