Skip to content

Track retry mode and gzip feature ids #3481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 30, 2025
Merged

Track retry mode and gzip feature ids #3481

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1966877
Track gzip request compression feature
arandito May 28, 2025
148d7e1
Track retry mode feature ids
arandito May 28, 2025
a9a6516
Move retry feature tracking to client.py
arandito May 28, 2025
9826df0
Reorganize tests
arandito May 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions botocore/args.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,7 @@ def _compute_socket_options(self, scoped_config, client_config=None):
def _compute_retry_config(self, config_kwargs):
self._compute_retry_max_attempts(config_kwargs)
self._compute_retry_mode(config_kwargs)
self._register_retry_mode_feature_id(config_kwargs)

def _compute_retry_max_attempts(self, config_kwargs):
# There's a pre-existing max_attempts client config value that actually
Expand Down Expand Up @@ -601,6 +602,11 @@ def _compute_retry_mode(self, config_kwargs):
retry_mode = 'legacy'
retries['mode'] = retry_mode

def _register_retry_mode_feature_id(self, config_kwargs):
retries = config_kwargs.get('retries')
retry_mode = retries.get('mode')
register_feature_id(f'RETRY_MODE_{retry_mode.upper()}')

def _compute_connect_timeout(self, config_kwargs):
# Checking if connect_timeout is set on the client config.
# If it is not, we check the config_store in case a
Expand Down
2 changes: 2 additions & 0 deletions botocore/compress.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from gzip import compress as gzip_compress

from botocore.compat import urlencode
from botocore.useragent import register_feature_id
from botocore.utils import determine_content_length

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -88,6 +89,7 @@ def _get_body_size(body):


def _gzip_compress_body(body):
register_feature_id('GZIP_REQUEST_COMPRESSION')
if isinstance(body, str):
return gzip_compress(body.encode('utf-8'))
elif isinstance(body, (bytes, bytearray)):
Expand Down
4 changes: 4 additions & 0 deletions botocore/useragent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@
_USERAGENT_FEATURE_MAPPINGS = {
'WAITER': 'B',
'PAGINATOR': 'C',
"RETRY_MODE_LEGACY": "D",
"RETRY_MODE_STANDARD": "E",
"RETRY_MODE_ADAPTIVE": "F",
'S3_TRANSFER': 'G',
'GZIP_REQUEST_COMPRESSION': 'L',
'PROTOCOL_RPC_V2_CBOR': 'M',
'ENDPOINT_OVERRIDE': 'N',
'ACCOUNT_ID_MODE_PREFERRED': 'P',
Expand Down
25 changes: 25 additions & 0 deletions tests/functional/test_compress.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
from botocore.compress import COMPRESSION_MAPPING
from botocore.config import Config
from tests import ALL_SERVICES, ClientHTTPStubber, patch_load_service_model
from tests.functional.test_useragent import (
get_captured_ua_strings,
parse_registered_feature_ids,
)

FAKE_MODEL = {
"version": "2.0",
Expand Down Expand Up @@ -127,3 +131,24 @@ def test_compression(patched_session, monkeypatch):
serialized_body = f"{additional_params}&{serialized_params}"
actual_body = gzip.decompress(http_stubber.requests[0].body)
assert serialized_body.encode('utf-8') == actual_body


def test_user_agent_has_gzip_feature_id(patched_session, monkeypatch):
patch_load_service_model(
patched_session, monkeypatch, FAKE_MODEL, FAKE_RULESET
)
client = patched_session.create_client(
"otherservice",
region_name="us-west-2",
config=Config(request_min_compression_size_bytes=100),
)
with ClientHTTPStubber(client, strict=True) as http_stubber:
http_stubber.add_response(status=200, body=b"<response/>")
params_list = [
{"MockOpParam": f"MockOpParamValue{i}"} for i in range(1, 21)
]
# The mock operation registers `'GZIP_REQUEST_COMPRESSION': 'L'`
client.mock_operation(MockOpParamList=params_list)
ua_string = get_captured_ua_strings(http_stubber)[0]
feature_list = parse_registered_feature_ids(ua_string)
assert 'L' in feature_list
50 changes: 50 additions & 0 deletions tests/functional/test_retry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
from botocore.config import Config
from botocore.exceptions import ClientError
from tests import BaseSessionTest, ClientHTTPStubber, mock
from tests.functional.test_useragent import (
get_captured_ua_strings,
parse_registered_feature_ids,
)

RETRY_MODES = ('legacy', 'standard', 'adaptive')

Expand Down Expand Up @@ -264,6 +268,20 @@ def test_can_clobber_max_attempts_on_session(self):
with self.assert_will_retry_n_times(client, 0):
client.list_repositories()

def test_user_agent_has_legacy_mode_feature_id(self):
client = self.session.create_client('dynamodb', self.region)
with ClientHTTPStubber(client) as http_stubber:
# Add two failed responses followed by a success
http_stubber.add_response(status=500, body=b'{}')
http_stubber.add_response(status=500, body=b'{}')
http_stubber.add_response(status=200, body=b'{}')
client.list_tables()
ua_strings = get_captured_ua_strings(http_stubber)
# Confirm all requests register `'RETRY_MODE_LEGACY': 'D'`
for ua_string in ua_strings:
feature_list = parse_registered_feature_ids(ua_string)
assert 'D' in feature_list


class TestRetriesV2(BaseRetryTest):
def create_client_with_retry_mode(
Expand Down Expand Up @@ -349,3 +367,35 @@ def test_can_exhaust_default_retry_quota(self):
self.assertTrue(
e.exception.response['ResponseMetadata'].get('RetryQuotaReached')
)

def test_user_agent_has_standard_mode_feature_id(self):
client = self.create_client_with_retry_mode(
'dynamodb', retry_mode='standard'
)
with ClientHTTPStubber(client) as http_stubber:
# Add two failed responses followed by a success
http_stubber.add_response(status=502, body=b'{}')
http_stubber.add_response(status=502, body=b'{}')
http_stubber.add_response(status=200, body=b'{}')
client.list_tables()
ua_strings = get_captured_ua_strings(http_stubber)
# Confirm all requests register `'RETRY_MODE_STANDARD': 'E'`
for ua_string in ua_strings:
feature_list = parse_registered_feature_ids(ua_string)
assert 'E' in feature_list

def test_user_agent_has_adaptive_mode_feature_id(self):
client = self.create_client_with_retry_mode(
'dynamodb', retry_mode='adaptive'
)
with ClientHTTPStubber(client) as http_stubber:
# Add two failed responses followed by a success
http_stubber.add_response(status=502, body=b'{}')
http_stubber.add_response(status=502, body=b'{}')
http_stubber.add_response(status=200, body=b'{}')
client.list_tables()
ua_strings = get_captured_ua_strings(http_stubber)
# Confirm all requests register `'RETRY_MODE_ADAPTIVE': 'F'`
for ua_string in ua_strings:
feature_list = parse_registered_feature_ids(ua_string)
assert 'F' in feature_list
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit; this seems like something we could/should be reusing rather than rewriting the same test many times if it can be parameterized. When you find yourself copy pasting scaffolding, unless there's something truly unique about it, try to find opportunities to reuse.

A simple way to do this is to either implement a standalone function, or a method on BaseRetryTest to help for this case. It could look something like:

    def _get_ua_strings_from_retries(self, client):
        with ClientHTTPStubber(client) as http_stubber:
            # Add two failed responses followed by a success
            http_stubber.add_response(status=502, body=b'{}')
            http_stubber.add_response(status=502, body=b'{}')
            http_stubber.add_response(status=200, body=b'{}')
            client.list_tables()
        ua_strings = get_captured_ua_strings(http_stubber)
        return  [parse_registered_feature_ids(ua_string) for ua_string in ua_strings]

and your test(s) would look something like:

def test_user_agent_has_standard_mode_feature_id(self):
    client = self.create_client_with_retry_mode(
        'dynamodb', retry_mode='standard'
    )
    feature_lists = self._get_ua_strings_from_retries(client)
    # Confirm all requests register `'RETRY_MODE_ADAPTIVE': 'E'`
    assert all('E' in feature_list for feature_list in feature_lists)
    
def test_user_agent_has_adaptive_mode_feature_id(self):
    client = self.create_client_with_retry_mode(
        'dynamodb', retry_mode='adaptive'
    )
    feature_lists = self._get_ua_strings_from_retries(client)
    # Confirm all requests register `'RETRY_MODE_ADAPTIVE': 'F'`
    assert all('F' in feature_list for feature_list in feature_lists)

You can also parametrize these as well with pytest if you're not using the base unittest parent class. That's a separate consideration though. I don't think this is blocking, but it's something to consider for development.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, I didn't catch that repeated logic but will keep that in mind for future development. I added a reusable helper to simplify testing if we add new retry modes in the future.