Merge branch 'release-0.10.2'

* release-0.10.2: Bumping version to 0.10.2 Pass operation name to awscrt.s3 (#309) CRTTransferManager validates ExtraArgs and Bucket, same as classic TransferManager (#294) Start testing on 3.13 beta build Move 3.8 and 3.9 builds back to macos-13 update wheel to latest release (#303) Upgrade pytest and associated test packages (#302)
boto · Jun 24, 2024 · 9a16829 · 9a16829
2 parents 106c814 + 7a0f702
commit 9a16829
Show file tree

Hide file tree

Showing 10 changed files with 155 additions and 18 deletions.
diff --git a/.changes/0.10.2.json 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"
+  }
+]
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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()