Skip to content

Commit

Permalink
Add version floor for transfer feature (#3957)
Browse files Browse the repository at this point in the history
  • Loading branch information
nateprewitt authored Nov 29, 2023
1 parent 5d80bae commit 7230084
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 2 deletions.
18 changes: 17 additions & 1 deletion boto3/s3/transfer.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,8 @@ def create_transfer_manager(client, config, osutil=None):


def _should_use_crt(config):
if HAS_CRT:
# This feature requires awscrt>=0.19.17
if HAS_CRT and has_minimum_crt_version((0, 19, 17)):

This comment has been minimized.

Copy link
@agicquelamz

agicquelamz Nov 29, 2023

looking at how has_minimum_crt_version is implemented, the check for HAS_CRT is extraneous

This comment has been minimized.

Copy link
@nateprewitt

nateprewitt Nov 29, 2023

Author Contributor

The check in has_minimum_crt_version is to ensure we're not failing if the CRT isn't present when the function is called in isolation (mainly for testing). We can remove it here, but ideally this goes away in the long term. I'd rather not optimize it away now if we have quick cleanup coming.

is_optimized_instance = awscrt.s3.is_optimized_for_system()
else:
is_optimized_instance = False
Expand All @@ -205,6 +206,21 @@ def _should_use_crt(config):
return False


def has_minimum_crt_version(minimum_version):
"""Not intended for use outside boto3."""
if not HAS_CRT:
return False

crt_version_str = awscrt.__version__
try:
crt_version_ints = map(int, crt_version_str.split("."))
crt_version_tuple = tuple(crt_version_ints)
except (TypeError, ValueError):
return False

return crt_version_tuple >= minimum_version


def _create_default_transfer_manager(client, config, osutil):
"""Create the default TransferManager implementation for s3transfer."""
executor_cls = None
Expand Down
17 changes: 16 additions & 1 deletion tests/functional/test_crt.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@
from botocore.compat import HAS_CRT
from botocore.credentials import Credentials

from boto3.s3.transfer import TransferConfig, create_transfer_manager
from boto3.s3.transfer import (
TransferConfig,
create_transfer_manager,
has_minimum_crt_version,
)
from tests import mock, requires_crt

if HAS_CRT:
Expand Down Expand Up @@ -62,3 +66,14 @@ def test_create_transfer_manager_on_optimized_instance(self):
config = TransferConfig()
transfer_manager = create_transfer_manager(client, config)
assert isinstance(transfer_manager, CRTTransferManager)

@requires_crt()
def test_minimum_crt_version(self):
assert has_minimum_crt_version((0, 16, 12)) is True

@requires_crt()
def test_minimum_crt_version_bad_crt_version(self):
with mock.patch("awscrt.__version__") as vers:
vers.return_value = None

assert has_minimum_crt_version((0, 16, 12)) is False

0 comments on commit 7230084

Please sign in to comment.