From ccbdf95a6a6025c2852e4190107b1e3ae9b9ad96 Mon Sep 17 00:00:00 2001 From: Amarjeet LNU Date: Thu, 23 Jul 2026 14:13:38 -0700 Subject: [PATCH 1/2] test(integ): let exhausted IAM throttling fail instead of skipping The prior mitigation converted a SimulatePrincipalPolicy throttle that survived the adaptive retries into a skipped test (pytest_runtest_makereport). That hid a persistent rate-limit regression: a genuinely throttled run would silently drop out of the results instead of showing up as a failure. Keep the autouse adaptive-retry fixture (it still absorbs transient bursts), but remove the skip conversion and its now-unused helper/constants. Throttling that exhausts the retry budget now fails the test loudly so the regression is visible. --- sagemaker-serve/tests/integ/conftest.py | 60 +++++-------------------- 1 file changed, 11 insertions(+), 49 deletions(-) diff --git a/sagemaker-serve/tests/integ/conftest.py b/sagemaker-serve/tests/integ/conftest.py index 4170a1a934..dc4a8460ea 100644 --- a/sagemaker-serve/tests/integ/conftest.py +++ b/sagemaker-serve/tests/integ/conftest.py @@ -20,17 +20,17 @@ This is purely a test-harness concurrency problem, so the mitigation lives here in the test layer rather than in SDK source: -* ``_configure_default_boto_retries`` (autouse) — approach "A": set an adaptive - retry policy on boto3's *default* session. The role resolver, when no explicit - session is passed, falls back to ``sagemaker.core...Session()`` whose boto - session is ``boto3.DEFAULT_SESSION`` (see session_helper._initialize). Clients - created from it therefore inherit these retries, letting the internal IAM calls - ride out transient throttling without any source change. - -* ``pytest_runtest_makereport`` — approach "C": a belt-and-suspenders fallback. If - a residual ``SimulatePrincipalPolicy`` throttling error still escapes after the - adaptive retries, convert the failure into an xfail so a transient rate limit - never reds the build. Genuine failures are untouched. +* ``_configure_default_boto_retries`` (autouse) — set an adaptive retry policy on + boto3's *default* session. The role resolver, when no explicit session is + passed, falls back to ``sagemaker.core...Session()`` whose boto session is + ``boto3.DEFAULT_SESSION`` (see session_helper._initialize). Clients created from + it therefore inherit these retries, letting the internal IAM calls ride out + transient throttling without any source change. + +Throttling that still exhausts the adaptive retry budget is deliberately left to +fail the test loudly (rather than being converted to a skip), so a persistent +rate-limit regression stays visible instead of silently disappearing from the +results. """ from __future__ import absolute_import @@ -41,11 +41,6 @@ # Adaptive retry budget for throttling-prone IAM validation calls. _ADAPTIVE_RETRY_CONFIG = Config(retries={"max_attempts": 10, "mode": "adaptive"}) -# Only tolerate throttling on the IAM permission simulation used during role -# validation, so unrelated throttling still fails loudly. -_THROTTLE_ERROR_CODES = ("Throttling", "ThrottlingException", "RequestLimitExceeded") -_SIMULATE_OP = "SimulatePrincipalPolicy" - @pytest.fixture(autouse=True, scope="session") def _configure_default_boto_retries(): @@ -54,36 +49,3 @@ def _configure_default_boto_retries(): boto3.setup_default_session() boto3.DEFAULT_SESSION._session.set_default_client_config(_ADAPTIVE_RETRY_CONFIG) yield - - -def _is_simulate_policy_throttle(exc): - """Return True if ``exc`` is a SimulatePrincipalPolicy throttling ClientError.""" - from botocore.exceptions import ClientError - - if not isinstance(exc, ClientError): - return False - error_code = exc.response.get("Error", {}).get("Code", "") - operation = getattr(exc, "operation_name", "") or "" - return error_code in _THROTTLE_ERROR_CODES and operation == _SIMULATE_OP - - -@pytest.hookimpl(hookwrapper=True) -def pytest_runtest_makereport(item, call): - """Skip (rather than fail) on residual SimulatePrincipalPolicy throttling that - survives the adaptive retries under heavy concurrency.""" - outcome = yield - report = outcome.get_result() - - if report.when != "call" or not report.failed: - return - - exc = call.excinfo.value if call.excinfo else None - # Walk the exception chain so a wrapped ClientError is still detected. - while exc is not None: - if _is_simulate_policy_throttle(exc): - report.outcome = "skipped" - report.wasxfail = ( - "iam:SimulatePrincipalPolicy throttled under concurrent test load" - ) - break - exc = exc.__cause__ or exc.__context__ From 79120aae40c4a3e6c56aa22172614d8a3c23c708 Mon Sep 17 00:00:00 2001 From: Amarjeet LNU Date: Thu, 23 Jul 2026 14:29:59 -0700 Subject: [PATCH 2/2] test(integ): remove throttle skip hook from train & mlops; fix serve merge artifacts The master merge into this branch reintroduced the pytest_runtest_makereport skip hook in the train and mlops integ conftests (both landed by #6081), so a SimulatePrincipalPolicy throttle surviving the retries would still be silently skipped there. Remove the hook and its now-unused helper/constants from both, so exhausted throttling fails loudly in every suite. Also fix two artifacts the merge left in the serve conftest: - restore the fixture teardown that resets AWS_RETRY_MODE / AWS_MAX_ATTEMPTS (the 'previous' dict was captured but never restored -> unused-variable lint and env leak across the session); - update the stale docstring that still described the removed _configure_default_boto_retries / DEFAULT_SESSION approach. --- sagemaker-mlops/tests/integ/conftest.py | 44 +++-------------------- sagemaker-serve/tests/integ/conftest.py | 22 ++++++++---- sagemaker-train/tests/integ/conftest.py | 48 +++---------------------- 3 files changed, 24 insertions(+), 90 deletions(-) diff --git a/sagemaker-mlops/tests/integ/conftest.py b/sagemaker-mlops/tests/integ/conftest.py index a0981c9b5f..537e18c879 100644 --- a/sagemaker-mlops/tests/integ/conftest.py +++ b/sagemaker-mlops/tests/integ/conftest.py @@ -69,6 +69,11 @@ # ``ClientError: (Throttling) ... Rate exceeded``. This mitigation is a purely # test-harness concurrency fix and is intentionally identical to the block in # the sagemaker-train / sagemaker-serve integ conftests. +# +# Throttling that still exhausts the adaptive retry budget is deliberately left +# to fail the test loudly (rather than being converted to a skip), so a +# persistent rate-limit regression stays visible instead of silently +# disappearing from the results. # --------------------------------------------------------------------------- # botocore adaptive retry settings for throttling-prone IAM validation calls. @@ -77,11 +82,6 @@ _RETRY_MODE = "adaptive" _MAX_ATTEMPTS = "10" -# Only tolerate throttling on the IAM permission simulation used during role -# validation, so unrelated throttling still fails loudly. -_THROTTLE_ERROR_CODES = ("Throttling", "ThrottlingException", "RequestLimitExceeded") -_SIMULATE_OP = "SimulatePrincipalPolicy" - @pytest.fixture(autouse=True, scope="session") def _configure_boto_adaptive_retries(): @@ -102,40 +102,6 @@ def _configure_boto_adaptive_retries(): os.environ[key] = value -def _is_simulate_policy_throttle(exc): - """Return True if ``exc`` is a SimulatePrincipalPolicy throttling ClientError.""" - from botocore.exceptions import ClientError - - if not isinstance(exc, ClientError): - return False - error_code = exc.response.get("Error", {}).get("Code", "") - operation = getattr(exc, "operation_name", "") or "" - return error_code in _THROTTLE_ERROR_CODES and operation == _SIMULATE_OP - - -@pytest.hookimpl(hookwrapper=True) -def pytest_runtest_makereport(item, call): - """Skip (rather than fail) on residual SimulatePrincipalPolicy throttling that - survives the adaptive retries under heavy concurrency. Applies to setup and - call phases, since role resolution frequently happens in fixture setup.""" - outcome = yield - report = outcome.get_result() - - if report.when not in ("setup", "call") or not report.failed: - return - - exc = call.excinfo.value if call.excinfo else None - # Walk the exception chain so a wrapped ClientError is still detected. - while exc is not None: - if _is_simulate_policy_throttle(exc): - report.outcome = "skipped" - report.wasxfail = ( - "iam:SimulatePrincipalPolicy throttled under concurrent test load" - ) - break - exc = exc.__cause__ or exc.__context__ - - # --------------------------------------------------------------------------- # CLI options # --------------------------------------------------------------------------- diff --git a/sagemaker-serve/tests/integ/conftest.py b/sagemaker-serve/tests/integ/conftest.py index 0aa843bcf0..1609fe375a 100644 --- a/sagemaker-serve/tests/integ/conftest.py +++ b/sagemaker-serve/tests/integ/conftest.py @@ -18,14 +18,17 @@ ``ClientError: (Throttling) ... Rate exceeded`` and failing the build. This is purely a test-harness concurrency problem, so the mitigation lives here in -the test layer rather than in SDK source: +the test layer rather than in SDK source. It is intentionally identical to the +block in ``sagemaker-train`` / ``sagemaker-mlops`` integ conftests: -* ``_configure_default_boto_retries`` (autouse) — set an adaptive retry policy on - boto3's *default* session. The role resolver, when no explicit session is - passed, falls back to ``sagemaker.core...Session()`` whose boto session is - ``boto3.DEFAULT_SESSION`` (see session_helper._initialize). Clients created from - it therefore inherit these retries, letting the internal IAM calls ride out - transient throttling without any source change. +* ``_configure_boto_adaptive_retries`` (autouse) — set adaptive retries via the + ``AWS_RETRY_MODE`` / ``AWS_MAX_ATTEMPTS`` environment variables. Env vars apply + to *every* boto3 client created in the worker, so the internal IAM calls ride + out transient throttling whether the resolver falls back to the default session + or builds its client from an explicitly-passed ``Session`` (several serve tests + pass their own session, whose IAM client would otherwise carry botocore's + default 4-attempt retry policy). ``adaptive`` mode also adds client-side rate + limiting to smooth bursts. Throttling that still exhausts the adaptive retry budget is deliberately left to fail the test loudly (rather than being converted to a skip), so a persistent @@ -57,3 +60,8 @@ def _configure_boto_adaptive_retries(): os.environ["AWS_RETRY_MODE"] = _RETRY_MODE os.environ["AWS_MAX_ATTEMPTS"] = _MAX_ATTEMPTS yield + for key, value in previous.items(): + if value is None: + os.environ.pop(key, None) + else: + os.environ[key] = value diff --git a/sagemaker-train/tests/integ/conftest.py b/sagemaker-train/tests/integ/conftest.py index 87c861092a..de5e45aef8 100644 --- a/sagemaker-train/tests/integ/conftest.py +++ b/sagemaker-train/tests/integ/conftest.py @@ -31,11 +31,10 @@ its own). ``adaptive`` mode adds client-side rate limiting so bursts of ``SimulatePrincipalPolicy`` calls ride out transient throttling. -* ``pytest_runtest_makereport`` — a belt-and-suspenders fallback. If a residual - ``SimulatePrincipalPolicy`` throttling error still escapes after the adaptive - retries, convert the failure into a skip so a transient rate limit never reds - the build. Genuine failures (and throttling on any other operation) are - untouched. +Throttling that still exhausts the adaptive retry budget is deliberately left to +fail the test loudly (rather than being converted to a skip), so a persistent +rate-limit regression stays visible instead of silently disappearing from the +results. """ from __future__ import absolute_import @@ -49,11 +48,6 @@ _RETRY_MODE = "adaptive" _MAX_ATTEMPTS = "10" -# Only tolerate throttling on the IAM permission simulation used during role -# validation, so unrelated throttling still fails loudly. -_THROTTLE_ERROR_CODES = ("Throttling", "ThrottlingException", "RequestLimitExceeded") -_SIMULATE_OP = "SimulatePrincipalPolicy" - @pytest.fixture(autouse=True, scope="session") def _configure_boto_adaptive_retries(): @@ -72,37 +66,3 @@ def _configure_boto_adaptive_retries(): os.environ.pop(key, None) else: os.environ[key] = value - - -def _is_simulate_policy_throttle(exc): - """Return True if ``exc`` is a SimulatePrincipalPolicy throttling ClientError.""" - from botocore.exceptions import ClientError - - if not isinstance(exc, ClientError): - return False - error_code = exc.response.get("Error", {}).get("Code", "") - operation = getattr(exc, "operation_name", "") or "" - return error_code in _THROTTLE_ERROR_CODES and operation == _SIMULATE_OP - - -@pytest.hookimpl(hookwrapper=True) -def pytest_runtest_makereport(item, call): - """Skip (rather than fail) on residual SimulatePrincipalPolicy throttling that - survives the adaptive retries under heavy concurrency. Applies to setup and - call phases, since role resolution frequently happens in fixture setup.""" - outcome = yield - report = outcome.get_result() - - if report.when not in ("setup", "call") or not report.failed: - return - - exc = call.excinfo.value if call.excinfo else None - # Walk the exception chain so a wrapped ClientError is still detected. - while exc is not None: - if _is_simulate_policy_throttle(exc): - report.outcome = "skipped" - report.wasxfail = ( - "iam:SimulatePrincipalPolicy throttled under concurrent test load" - ) - break - exc = exc.__cause__ or exc.__context__