From 81e1de277e913e926d3b9690d3bd1d3fce73ccb6 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Fri, 21 Jun 2024 13:19:19 -0700 Subject: [PATCH 1/5] Pass operation name to awscrt.s3 --- s3transfer/crt.py | 15 ++++++++++++--- tests/functional/test_crt.py | 1 + tests/integration/test_crt.py | 12 ++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/s3transfer/crt.py b/s3transfer/crt.py index 2954477e..b5e07b2e 100644 --- a/s3transfer/crt.py +++ b/s3transfer/crt.py @@ -818,13 +818,22 @@ def _default_get_make_request_args( on_done_before_calls, on_done_after_calls, ): + crt_request_type = getattr( + S3RequestType, request_type.upper(), S3RequestType.DEFAULT + ) + + # For DEFAULT requests, CRT requires the official S3 operation name. + # So transform string like "delete_object" -> "DeleteObject". + operation_name = None + if crt_request_type == S3RequestType.DEFAULT: + operation_name = ''.join(x.title() for x in request_type.split('_')) + make_request_args = { 'request': self._request_serializer.serialize_http_request( request_type, future ), - 'type': getattr( - S3RequestType, request_type.upper(), S3RequestType.DEFAULT - ), + 'type': crt_request_type, + 'operation_name': operation_name, 'on_done': self.get_crt_callback( future, 'done', on_done_before_calls, on_done_after_calls ), diff --git a/tests/functional/test_crt.py b/tests/functional/test_crt.py index 352e5854..4276220f 100644 --- a/tests/functional/test_crt.py +++ b/tests/functional/test_crt.py @@ -515,6 +515,7 @@ def test_delete(self): { 'request': mock.ANY, 'type': awscrt.s3.S3RequestType.DEFAULT, + 'operation_name': "DeleteObject", 'on_progress': mock.ANY, 'on_done': mock.ANY, }, diff --git a/tests/integration/test_crt.py b/tests/integration/test_crt.py index 7f16d85e..ec59dd91 100644 --- a/tests/integration/test_crt.py +++ b/tests/integration/test_crt.py @@ -13,6 +13,9 @@ import glob import io import os +from uuid import uuid4 + +from botocore.exceptions import ClientError from s3transfer.subscribers import BaseSubscriber from s3transfer.utils import OSUtils @@ -404,6 +407,15 @@ def test_delete(self): future.result() self.assertTrue(self.object_not_exists('foo.txt')) + def test_delete_exception_no_such_bucket(self): + # delete() uses awscrt.s3.S3RequestType.DEFAULT under the hood. + # Test that CRT exceptions translate properly into the botocore exceptions. + transfer = self._create_s3_transfer() + with self.assertRaises(ClientError) as ctx: + future = transfer.delete(f"{self.bucket_name}-NONEXISTENT-{uuid4()}", "foo.txt") + future.result() + self.assertEqual(ctx.exception.__class__.__name__, 'NoSuchBucket') + def test_many_files_download(self): transfer = self._create_s3_transfer() From 25a71c396cab81e57d86e8e6a08f400f1756f4f1 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Fri, 21 Jun 2024 13:42:45 -0700 Subject: [PATCH 2/5] format with black --- s3transfer/crt.py | 4 +++- tests/integration/test_crt.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/s3transfer/crt.py b/s3transfer/crt.py index b5e07b2e..7927304e 100644 --- a/s3transfer/crt.py +++ b/s3transfer/crt.py @@ -826,7 +826,9 @@ def _default_get_make_request_args( # So transform string like "delete_object" -> "DeleteObject". operation_name = None if crt_request_type == S3RequestType.DEFAULT: - operation_name = ''.join(x.title() for x in request_type.split('_')) + operation_name = ''.join( + x.title() for x in request_type.split('_') + ) make_request_args = { 'request': self._request_serializer.serialize_http_request( diff --git a/tests/integration/test_crt.py b/tests/integration/test_crt.py index ec59dd91..24fb06bc 100644 --- a/tests/integration/test_crt.py +++ b/tests/integration/test_crt.py @@ -412,7 +412,9 @@ def test_delete_exception_no_such_bucket(self): # Test that CRT exceptions translate properly into the botocore exceptions. transfer = self._create_s3_transfer() with self.assertRaises(ClientError) as ctx: - future = transfer.delete(f"{self.bucket_name}-NONEXISTENT-{uuid4()}", "foo.txt") + future = transfer.delete( + f"{self.bucket_name}-NONEXISTENT-{uuid4()}", "foo.txt" + ) future.result() self.assertEqual(ctx.exception.__class__.__name__, 'NoSuchBucket') From 6f6feccdf2dcbeb3e74527c80a0398bdbf580e4d Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Fri, 21 Jun 2024 13:55:21 -0700 Subject: [PATCH 3/5] fix tests. also simpler code --- s3transfer/crt.py | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/s3transfer/crt.py b/s3transfer/crt.py index 7927304e..ad008bc9 100644 --- a/s3transfer/crt.py +++ b/s3transfer/crt.py @@ -818,29 +818,26 @@ def _default_get_make_request_args( on_done_before_calls, on_done_after_calls, ): - crt_request_type = getattr( - S3RequestType, request_type.upper(), S3RequestType.DEFAULT - ) - - # For DEFAULT requests, CRT requires the official S3 operation name. - # So transform string like "delete_object" -> "DeleteObject". - operation_name = None - if crt_request_type == S3RequestType.DEFAULT: - operation_name = ''.join( - x.title() for x in request_type.split('_') - ) - make_request_args = { 'request': self._request_serializer.serialize_http_request( request_type, future ), - 'type': crt_request_type, - 'operation_name': operation_name, + 'type': getattr( + S3RequestType, request_type.upper(), S3RequestType.DEFAULT + ), 'on_done': self.get_crt_callback( future, 'done', on_done_before_calls, on_done_after_calls ), 'on_progress': self.get_crt_callback(future, 'progress'), } + + # For DEFAULT requests, CRT requires the official S3 operation name. + # So transform string like "delete_object" -> "DeleteObject". + if make_request_args['type'] == S3RequestType.DEFAULT: + make_request_args['operation_name'] = ''.join( + x.title() for x in request_type.split('_') + ) + if is_s3express_bucket(call_args.bucket): make_request_args['signing_config'] = AwsSigningConfig( algorithm=AwsSigningAlgorithm.V4_S3EXPRESS From 0ea06e2eb0571c24ea755a90ae9c737aa60dd2c0 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Fri, 21 Jun 2024 15:09:24 -0700 Subject: [PATCH 4/5] add release notes --- .changes/next-release/bugfix-awscrt-54174.json | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changes/next-release/bugfix-awscrt-54174.json diff --git a/.changes/next-release/bugfix-awscrt-54174.json b/.changes/next-release/bugfix-awscrt-54174.json new file mode 100644 index 00000000..287c5976 --- /dev/null +++ b/.changes/next-release/bugfix-awscrt-54174.json @@ -0,0 +1,5 @@ +{ + "type": "bugfix", + "category": "awscrt", + "description": "Pass operation name to awscrt.s3, to improve error handling" +} From caf051fb62b1a5cc6a56c8831daf01bd4b37b13a Mon Sep 17 00:00:00 2001 From: Nate Prewitt Date: Fri, 21 Jun 2024 16:16:12 -0600 Subject: [PATCH 5/5] Update .changes/next-release/bugfix-awscrt-54174.json --- .changes/next-release/bugfix-awscrt-54174.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changes/next-release/bugfix-awscrt-54174.json b/.changes/next-release/bugfix-awscrt-54174.json index 287c5976..f8eca6a0 100644 --- a/.changes/next-release/bugfix-awscrt-54174.json +++ b/.changes/next-release/bugfix-awscrt-54174.json @@ -1,5 +1,5 @@ { "type": "bugfix", - "category": "awscrt", - "description": "Pass operation name to awscrt.s3, to improve error handling" + "category": "``awscrt``", + "description": "Pass operation name to awscrt.s3 to improve error handling." }