diff --git a/.changes/0.10.2.json b/.changes/0.10.2.json new file mode 100644 index 00000000..922b3e94 --- /dev/null +++ b/.changes/0.10.2.json @@ -0,0 +1,7 @@ +[ + { + "category": "``awscrt``", + "description": "Pass operation name to awscrt.s3 to improve error handling.", + "type": "bugfix" + } +] \ No newline at end of file diff --git a/.github/workflows/run-crt-test.yml b/.github/workflows/run-crt-test.yml index 1e20b57f..50141892 100644 --- a/.github/workflows/run-crt-test.yml +++ b/.github/workflows/run-crt-test.yml @@ -12,7 +12,17 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"] + python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13-dev"] + os: [ubuntu-latest, macOS-latest, windows-latest] + # Python 3.8 and 3.9 do not run on m1 hardware which is now standard for + # macOS-latest. + # https://github.com/actions/setup-python/issues/696#issuecomment-1637587760 + exclude: + - { python-version: "3.8", os: "macos-latest" } + - { python-version: "3.9", os: "macos-latest" } + include: + - { python-version: "3.8", os: "macos-13" } + - { python-version: "3.9", os: "macos-13" } steps: - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index de4bb8ef..92a8f7de 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -15,7 +15,17 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"] + python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13-dev"] + os: [ubuntu-latest, macOS-latest, windows-latest] + # Python 3.8 and 3.9 do not run on m1 hardware which is now standard for + # macOS-latest. + # https://github.com/actions/setup-python/issues/696#issuecomment-1637587760 + exclude: + - { python-version: "3.8", os: "macos-latest" } + - { python-version: "3.9", os: "macos-latest" } + include: + - { python-version: "3.8", os: "macos-13" } + - { python-version: "3.9", os: "macos-13" } steps: - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 07d09a83..c8f12ef3 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,12 @@ CHANGELOG ========= +0.10.2 +====== + +* bugfix:``awscrt``: Pass operation name to awscrt.s3 to improve error handling. + + 0.10.1 ====== diff --git a/requirements-dev-lock.txt b/requirements-dev-lock.txt index cc259424..d0972326 100644 --- a/requirements-dev-lock.txt +++ b/requirements-dev-lock.txt @@ -87,9 +87,9 @@ packaging==21.3 \ --hash=sha256:dd47c42927d89ab911e606518907cc2d3a1f38bbd026385970643f9c5b8ecfeb \ --hash=sha256:ef103e05f519cdc783ae24ea4e2e0f508a9c99b2d4969652eed6a2e1ea5bd522 # via pytest -pluggy==1.0.0 \ - --hash=sha256:4224373bacce55f955a878bf9cfa763c1e360858e330072059e10bad68531159 \ - --hash=sha256:74134bbf457f031a36d68416e1509f34bd5ccc019f0bcc952c7b909d06b37bd3 +pluggy==1.4.0 \ + --hash=sha256:7db9f7b503d67d1c5b95f59773ebb58a8c1c288129a88665838012cfb07b8981 \ + --hash=sha256:8c85c2876142a764e5b7548e7d9a0e0ddb46f5185161049a79b7e974454223be # via pytest psutil==4.4.2 \ --hash=sha256:10fbb631142a3200623f4ab49f8bf82c32b79b8fe179f6056d01da3dfc589da1 \ @@ -106,15 +106,15 @@ pyparsing==3.0.9 \ --hash=sha256:2b020ecf7d21b687f219b71ecad3631f644a47f01403fa1d1036b0c6416d70fb \ --hash=sha256:5026bae9a10eeaefb61dab2f09052b9f4307d44aee4eda64b309723d8d206bbc # via packaging -pytest==7.4.0 \ - --hash=sha256:78bf16451a2eb8c7a2ea98e32dc119fd2aa758f1d5d66dbf0a59d69a3969df32 \ - --hash=sha256:b4bf8c45bd59934ed84001ad51e11b4ee40d40a1229d2c79f9c592b0a3f6bd8a +pytest==8.1.1 \ + --hash=sha256:2a8386cfc11fa9d2c50ee7b2a57e7d898ef90470a7a34c4b949ff59662bb78b7 \ + --hash=sha256:ac978141a75948948817d360297b7aae0fcb9d6ff6bc9ec6d514b85d5a65c044 # via # -r requirements-dev.txt # pytest-cov -pytest-cov==4.1.0 \ - --hash=sha256:3904b13dfbfec47f003b8e77fd5b589cd11904a21ddf1ab38a64f204d6a10ef6 \ - --hash=sha256:6ba70b9e97e69fcc3fb45bfeab2d0a138fb65c4d0d6a41ef33983ad114be8c3a +pytest-cov==5.0.0 \ + --hash=sha256:4f0764a1219df53214206bf1feea4633c3b558a2925c8b59f144f682861ce652 \ + --hash=sha256:5837b58e9f6ebd335b0f8060eecce69b662415b16dc503883a02f45dfeb14857 # via -r requirements-dev.txt tabulate==0.7.5 \ --hash=sha256:9071aacbd97a9a915096c1aaf0dc684ac2672904cd876db5904085d6dac9810e @@ -125,7 +125,7 @@ tomli==2.0.1 \ # via # coverage # pytest -wheel==0.38.1 \ - --hash=sha256:7a95f9a8dc0924ef318bd55b616112c70903192f524d120acc614f59547a9e1f \ - --hash=sha256:ea041edf63f4ccba53ad6e035427997b3bb10ee88a4cd014ae82aeb9eea77bb9 +wheel==0.43.0 \ + --hash=sha256:465ef92c69fa5c5da2d1cf8ac40559a8c940886afcef87dcf14b9470862f1d85 \ + --hash=sha256:55c570405f142630c6b9f72fe09d9b67cf1477fcf543ae5b8dcb1f5b7377da81 # via -r requirements-dev.txt diff --git a/requirements-dev.txt b/requirements-dev.txt index 3d672fdb..eb837d8e 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,11 +1,11 @@ psutil>=4.1.0,<5.0.0 tabulate==0.7.5 coverage==7.2.7 -wheel==0.38.1 +wheel==0.43.0 setuptools==67.8.0;python_version>="3.12" # Pytest specific deps -pytest==7.4.0 -pytest-cov==4.1.0 +pytest==8.1.1 +pytest-cov==5.0.0 atomicwrites>=1.0 # Windows requirement colorama>0.3.0 # Windows requirement diff --git a/s3transfer/__init__.py b/s3transfer/__init__.py index 4ada4a89..e54e4ebd 100644 --- a/s3transfer/__init__.py +++ b/s3transfer/__init__.py @@ -144,7 +144,7 @@ def __call__(self, bytes_amount): from s3transfer.exceptions import RetriesExceededError, S3UploadFailedError __author__ = 'Amazon Web Services' -__version__ = '0.10.1' +__version__ = '0.10.2' class NullHandler(logging.Handler): diff --git a/s3transfer/crt.py b/s3transfer/crt.py index 2954477e..5695451f 100644 --- a/s3transfer/crt.py +++ b/s3transfer/crt.py @@ -40,6 +40,7 @@ from s3transfer.constants import MB from s3transfer.exceptions import TransferNotDoneError from s3transfer.futures import BaseTransferFuture, BaseTransferMeta +from s3transfer.manager import TransferManager from s3transfer.utils import ( CallArgs, OSUtils, @@ -181,6 +182,14 @@ def _get_crt_throughput_target_gbps(provided_throughput_target_bytes=None): class CRTTransferManager: + ALLOWED_DOWNLOAD_ARGS = TransferManager.ALLOWED_DOWNLOAD_ARGS + ALLOWED_UPLOAD_ARGS = TransferManager.ALLOWED_UPLOAD_ARGS + ALLOWED_DELETE_ARGS = TransferManager.ALLOWED_DELETE_ARGS + + VALIDATE_SUPPORTED_BUCKET_VALUES = True + + _UNSUPPORTED_BUCKET_PATTERNS = TransferManager._UNSUPPORTED_BUCKET_PATTERNS + def __init__(self, crt_s3_client, crt_request_serializer, osutil=None): """A transfer manager interface for Amazon S3 on CRT s3 client. @@ -226,6 +235,8 @@ def download( extra_args = {} if subscribers is None: subscribers = {} + self._validate_all_known_args(extra_args, self.ALLOWED_DOWNLOAD_ARGS) + self._validate_if_bucket_supported(bucket) callargs = CallArgs( bucket=bucket, key=key, @@ -240,6 +251,8 @@ def upload(self, fileobj, bucket, key, extra_args=None, subscribers=None): extra_args = {} if subscribers is None: subscribers = {} + self._validate_all_known_args(extra_args, self.ALLOWED_UPLOAD_ARGS) + self._validate_if_bucket_supported(bucket) self._validate_checksum_algorithm_supported(extra_args) callargs = CallArgs( bucket=bucket, @@ -255,6 +268,8 @@ def delete(self, bucket, key, extra_args=None, subscribers=None): extra_args = {} if subscribers is None: subscribers = {} + self._validate_all_known_args(extra_args, self.ALLOWED_DELETE_ARGS) + self._validate_if_bucket_supported(bucket) callargs = CallArgs( bucket=bucket, key=key, @@ -266,6 +281,27 @@ def delete(self, bucket, key, extra_args=None, subscribers=None): def shutdown(self, cancel=False): self._shutdown(cancel) + def _validate_if_bucket_supported(self, bucket): + # s3 high level operations don't support some resources + # (eg. S3 Object Lambda) only direct API calls are available + # for such resources + if self.VALIDATE_SUPPORTED_BUCKET_VALUES: + for resource, pattern in self._UNSUPPORTED_BUCKET_PATTERNS.items(): + match = pattern.match(bucket) + if match: + raise ValueError( + f'TransferManager methods do not support {resource} ' + 'resource. Use direct client calls instead.' + ) + + def _validate_all_known_args(self, actual, allowed): + for kwarg in actual: + if kwarg not in allowed: + raise ValueError( + f"Invalid extra_args key '{kwarg}', " + f"must be one of: {', '.join(allowed)}" + ) + def _validate_checksum_algorithm_supported(self, extra_args): checksum_algorithm = extra_args.get('ChecksumAlgorithm') if checksum_algorithm is None: @@ -830,6 +866,14 @@ def _default_get_make_request_args( ), '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 diff --git a/tests/functional/test_crt.py b/tests/functional/test_crt.py index 352e5854..23a2bee1 100644 --- a/tests/functional/test_crt.py +++ b/tests/functional/test_crt.py @@ -373,6 +373,28 @@ def test_upload_throws_error_for_unsupported_checksum(self): [self.record_subscriber], ) + def test_upload_throws_error_for_unsupported_arg(self): + with self.assertRaisesRegex( + ValueError, "Invalid extra_args key 'ContentMD5'" + ): + self.transfer_manager.upload( + self.filename, + self.bucket, + self.key, + {'ContentMD5': '938c2cc0dcc05f2b68c4287040cfcf71'}, + [self.record_subscriber], + ) + + def test_upload_throws_error_on_s3_object_lambda_resource(self): + s3_object_lambda_arn = ( + 'arn:aws:s3-object-lambda:us-west-2:123456789012:' + 'accesspoint:my-accesspoint' + ) + with self.assertRaisesRegex(ValueError, 'methods do not support'): + self.transfer_manager.upload( + self.filename, s3_object_lambda_arn, self.key + ) + def test_upload_with_s3express(self): future = self.transfer_manager.upload( self.filename, @@ -489,6 +511,18 @@ def test_download_to_nonseekable_stream(self): underlying_stream.getvalue(), self.expected_download_content ) + def test_download_throws_error_for_unsupported_arg(self): + with self.assertRaisesRegex( + ValueError, "Invalid extra_args key 'Range'" + ): + self.transfer_manager.download( + self.bucket, + self.key, + self.filename, + {'Range': 'bytes:0-1023'}, + [self.record_subscriber], + ) + def test_download_with_s3express(self): future = self.transfer_manager.download( self.s3express_bucket, @@ -515,6 +549,7 @@ def test_delete(self): { 'request': mock.ANY, 'type': awscrt.s3.S3RequestType.DEFAULT, + 'operation_name': "DeleteObject", 'on_progress': mock.ANY, 'on_done': mock.ANY, }, @@ -526,6 +561,17 @@ def test_delete(self): ) self._assert_subscribers_called(future) + def test_delete_throws_error_for_unsupported_arg(self): + with self.assertRaisesRegex( + ValueError, "Invalid extra_args key 'BypassGovernanceRetention'" + ): + self.transfer_manager.delete( + self.bucket, + self.key, + {'BypassGovernanceRetention': True}, + [self.record_subscriber], + ) + def test_delete_with_s3express(self): future = self.transfer_manager.delete( self.s3express_bucket, self.key, {}, [self.record_subscriber] diff --git a/tests/integration/test_crt.py b/tests/integration/test_crt.py index 7f16d85e..24fb06bc 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,17 @@ 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()