Add tests

Author: hssyoo

awscli/customizations/s3/factory.py

@@ -15,6 +15,7 @@
 import awscrt.s3
 from botocore.client import Config
 from botocore.httpsession import DEFAULT_CA_BUNDLE
+from s3transfer.constants import GB
 from s3transfer.crt import (
     BotocoreCRTCredentialsWrapper,
     BotocoreCRTRequestSerializer,
@@ -142,7 +143,8 @@ def _create_crt_client(self, params, runtime_config):
         if (val := runtime_config.get('should_stream')) is not None:
             fio_options['should_stream'] = val
         if (val := runtime_config.get('disk_throughput')) is not None:
-            fio_options['disk_throughput_gbps'] = val
+            # Convert bytes to gigabits.
+            fio_options['disk_throughput_gbps'] = val * 8 / GB
         if (val := runtime_config.get('direct_io')) is not None:
             fio_options['direct_io'] = val
         create_crt_client_kwargs['fio_options'] = fio_options

awscli/s3transfer/crt.py

@@ -45,7 +45,7 @@
 from botocore.exceptions import NoCredentialsError
 from botocore.useragent import register_feature_id
 from botocore.utils import ArnParser, InvalidArnException, is_s3express_bucket
-from s3transfer.constants import FULL_OBJECT_CHECKSUM_ARGS, GB, MB
+from s3transfer.constants import FULL_OBJECT_CHECKSUM_ARGS, MB
 from s3transfer.exceptions import TransferNotDoneError
 from s3transfer.futures import BaseTransferFuture, BaseTransferMeta
 from s3transfer.utils import CallArgs, OSUtils, get_callbacks
@@ -155,9 +155,9 @@ def create_s3_crt_client(
     target_gbps = _get_crt_throughput_target_gbps(
         provided_throughput_target_bytes=target_throughput
     )
-    fio_options = fio_options or {}
-    if disk_throughput := fio_options.get('disk_throughput_gbps'):
-        fio_options['disk_throughput_gbps'] = disk_throughput * 8 / GB
+    crt_fio_options = None
+    if fio_options:
+        crt_fio_options = S3FileIoOptions(**fio_options)
     return S3Client(
         bootstrap=bootstrap,
         region=region,
@@ -167,7 +167,7 @@ def create_s3_crt_client(
         tls_connection_options=tls_connection_options,
         throughput_target_gbps=target_gbps,
         enable_s3express=True,
-        fio_options=S3FileIoOptions(**fio_options),
+        fio_options=crt_fio_options,
     )
 
 

tests/unit/customizations/s3/test_factory.py

@@ -13,7 +13,7 @@
 import awscrt.s3
 import pytest
 import s3transfer.crt
-from awscrt.s3 import S3RequestTlsMode
+from awscrt.s3 import S3FileIoOptions, S3RequestTlsMode
 from botocore.config import Config
 from botocore.credentials import Credentials
 from botocore.httpsession import DEFAULT_CA_BUNDLE
@@ -483,6 +483,27 @@ def test_target_bandwidth_configure_for_crt_manager(self, mock_crt_client):
         self.assert_is_crt_manager(transfer_manager)
         self.assert_expected_throughput_target_gbps(mock_crt_client, 8)
 
+    @mock.patch('s3transfer.crt.S3Client')
+    def test_fio_options_configure_for_crt_manager(self, mock_crt_client):
+        self.runtime_config = self.get_runtime_config(
+            preferred_transfer_client='crt',
+            should_stream=True,
+            disk_throughput='5GB/s',
+            direct_io=True,
+        )
+        transfer_manager = self.factory.create_transfer_manager(
+            self.params, self.runtime_config
+        )
+        expected_fio_options = S3FileIoOptions(
+            should_stream=True,
+            disk_throughput_gbps=40.0,
+            direct_io=True,
+        )
+        self.assert_is_crt_manager(transfer_manager)
+        self.assertEqual(
+            mock_crt_client.call_args[1]['fio_options'], expected_fio_options
+        )
+
     @mock.patch('s3transfer.crt.get_recommended_throughput_target_gbps')
     @mock.patch('s3transfer.crt.S3Client')
     def test_target_bandwidth_uses_crt_recommended_throughput(

tests/unit/customizations/s3/test_transferconfig.py

@@ -103,6 +103,12 @@ def test_set_preferred_transfer_client(self, provided, resolved):
             ('target_bandwidth', '1000', 1000),
             ('target_bandwidth', '1000B/s', 1000),
             ('target_bandwidth', '8000b/s', 1000),
+            # disk_throughput cases
+            ('disk_throughput', '1MB/s', 1024 * 1024),
+            ('disk_throughput', '10Mb/s', 10 * 1024 * 1024 / 8),
+            ('disk_throughput', '1000', 1000),
+            ('disk_throughput', '1000B/s', 1000),
+            ('disk_throughput', '8000b/s', 1000),
         ],
     )
     def test_rate_conversions(self, config_name, provided, expected):
@@ -127,6 +133,13 @@ def test_rate_conversions(self, config_name, provided, expected):
             ('target_bandwidth', '100/s'),
             ('target_bandwidth', ''),
             ('target_bandwidth', 'value-with-no-digits'),
+            # disk_throughput cases
+            ('disk_throughput', '1MB'),
+            ('disk_throughput', '1B'),
+            ('disk_throughput', '1b'),
+            ('disk_throughput', '100/s'),
+            ('disk_throughput', ''),
+            ('disk_throughput', 'value-with-no-digits'),
         ],
     )
     def test_invalid_rate_values(self, config_name, provided):
@@ -138,6 +151,22 @@ def test_validates_preferred_transfer_client_choices(self):
         with pytest.raises(transferconfig.InvalidConfigError):
             self.build_config_with(preferred_transfer_client='not-supported')
 
+    @pytest.mark.parametrize(
+        'attr,val,expected',
+        [
+            ('should_stream', 'true', True),
+            ('should_stream', 'false', False),
+            ('should_stream', None, None),
+            ('direct_io', 'true', True),
+            ('direct_io', 'false', False),
+            ('direct_io', None, None),
+        ],
+    )
+    def test_convert_booleans(self, attr, val, expected):
+        params = {attr: val}
+        runtime_config = self.build_config_with(**params)
+        assert runtime_config[attr] == expected
+
 
 class TestConvertToS3TransferConfig:
     def test_convert(self):

tests/unit/s3transfer/test_crt.py

@@ -16,6 +16,7 @@
 from botocore.credentials import Credentials, ReadOnlyCredentials
 from botocore.exceptions import ClientError, NoCredentialsError
 from botocore.session import Session
+from s3transfer.constants import GB
 from s3transfer.exceptions import TransferNotDoneError
 from s3transfer.utils import CallArgs
 
@@ -365,3 +366,55 @@ def test_target_throughput(
     def test_always_enables_s3express(self, mock_s3_crt_client):
         s3transfer.crt.create_s3_crt_client('us-west-2')
         assert mock_s3_crt_client.call_args[1]['enable_s3express'] is True
+
+    @pytest.mark.parametrize(
+        'fio_options,should_stream,disk_throughput,direct_io',
+        [
+            ({'should_stream': True}, True, 0.0, False),
+            ({'disk_throughput_gbps': 8}, False, 8, False),
+            ({'direct_io': True}, False, 0.0, True),
+            (
+                {'should_stream': True, 'disk_throughput_gbps': 8},
+                True,
+                8,
+                False,
+            ),
+            ({'should_stream': True, 'direct_io': True}, True, 0.0, True),
+            ({'disk_throughput_gbps': 8, 'direct_io': True}, False, 8, True),
+            (
+                {
+                    'should_stream': True,
+                    'disk_throughput_gbps': 8,
+                    'direct_io': True,
+                },
+                True,
+                8,
+                True,
+            ),
+        ],
+    )
+    def test_fio_options(
+        self,
+        fio_options,
+        should_stream,
+        disk_throughput,
+        direct_io,
+        mock_s3_crt_client,
+    ):
+        params = {'fio_options': fio_options}
+        s3transfer.crt.create_s3_crt_client(
+            'us-west-2',
+            **params,
+        )
+        assert (
+            mock_s3_crt_client.call_args[1]['fio_options'].should_stream
+            is should_stream
+        )
+        assert (
+            mock_s3_crt_client.call_args[1]['fio_options'].disk_throughput_gbps
+            == disk_throughput
+        )
+        assert (
+            mock_s3_crt_client.call_args[1]['fio_options'].direct_io
+            is direct_io
+        )