From 05226ce60ac01d10eee53bbb05bdbe3d8229d8af Mon Sep 17 00:00:00 2001 From: Adam Dangoor Date: Wed, 14 Oct 2026 14:33:13 +0100 Subject: [PATCH] Supply Model Target retry policies from backend adapters --- .../fixtures/real_model_target_retries.py | 84 ++++++++++++ tests/mock_vws/fixtures/vuforia_backends.py | 9 +- tests/mock_vws/test_model_target_retries.py | 129 ++++++++++++++---- tests/mock_vws/utils/__init__.py | 4 +- tests/mock_vws/utils/model_target_retries.py | 108 ++------------- 5 files changed, 210 insertions(+), 124 deletions(-) create mode 100644 tests/mock_vws/fixtures/real_model_target_retries.py diff --git a/tests/mock_vws/fixtures/real_model_target_retries.py b/tests/mock_vws/fixtures/real_model_target_retries.py new file mode 100644 index 000000000..8271f423a --- /dev/null +++ b/tests/mock_vws/fixtures/real_model_target_retries.py @@ -0,0 +1,84 @@ +"""Retrying transient failures from the real Model Target Web API. + +The real Model Target Web API sits behind a load balancer which +occasionally answers a perfectly good request with a gateway error, or +drops the connection. That has nothing to do with the contract under +test, so a bounded number of retries for requests which are safe to +repeat keeps a one-off gateway error from failing CI. + +Only requests to the real backend are retried. The mock backends +answer immediately, and some tests configure a mock to return a 5xx +deliberately, so retrying those would be both pointless and wrong. +The policy is therefore switched on for the duration of a test against +the real Model Target backend and is off at all other times. + +Only ``GET`` requests are retried. Repeating a dataset creation +``POST`` can create a second dataset and can consume the account's +Model Target training allowance, and the outcome of a repeat is +ambiguous, so mutating requests are sent exactly once. +""" + +from http import HTTPStatus + +import requests +from beartype import beartype +from tenacity import RetryCallState, Retrying +from tenacity.retry import retry_if_exception_type, retry_if_result +from tenacity.stop import stop_after_attempt +from tenacity.wait import wait_exponential_jitter +from vws.response import Response + +# Status codes which mean "the gateway could not get an answer from +# Vuforia", rather than "Vuforia answered, and this is the answer". +TRANSIENT_MODEL_TARGET_STATUS_CODES = frozenset( + { + HTTPStatus.BAD_GATEWAY, + HTTPStatus.SERVICE_UNAVAILABLE, + HTTPStatus.GATEWAY_TIMEOUT, + }, +) + +# Transport failures which mean the request may never have been +# answered. +TRANSIENT_MODEL_TARGET_EXCEPTIONS = ( + requests.exceptions.ConnectionError, + requests.exceptions.Timeout, +) + +# The total number of times a safe request is sent, including the first +# attempt. This is deliberately small: a persistent upstream failure +# must still fail the test quickly. +MODEL_TARGET_RETRY_ATTEMPTS = 3 + + +@beartype +def _is_transient(response: Response | requests.Response) -> bool: + """Whether a response is a transient gateway failure.""" + return response.status_code in TRANSIENT_MODEL_TARGET_STATUS_CODES + + +@beartype +def _last_outcome(retry_state: RetryCallState) -> Response | requests.Response: + """Give up with the outcome of the last attempt. + + A transient failure on the last attempt is not hidden: a response is + returned as it is, so that the caller's assertion reports the real + status and body, and a transport failure propagates. + """ + assert retry_state.outcome is not None + outcome = retry_state.outcome.result() + assert isinstance(outcome, Response | requests.Response) + return outcome + + +REAL_MODEL_TARGET_RETRY_POLICY = Retrying( + retry=( + retry_if_exception_type( + exception_types=TRANSIENT_MODEL_TARGET_EXCEPTIONS, + ) + | retry_if_result(predicate=_is_transient) + ), + stop=stop_after_attempt(max_attempt_number=MODEL_TARGET_RETRY_ATTEMPTS), + wait=wait_exponential_jitter(initial=1, max=10), + retry_error_callback=_last_outcome, +) diff --git a/tests/mock_vws/fixtures/vuforia_backends.py b/tests/mock_vws/fixtures/vuforia_backends.py index fae9bce5c..ec633e2e4 100644 --- a/tests/mock_vws/fixtures/vuforia_backends.py +++ b/tests/mock_vws/fixtures/vuforia_backends.py @@ -42,13 +42,16 @@ get_real_inactive_vumark_database, get_real_vumark_database, ) +from tests.mock_vws.fixtures.real_model_target_retries import ( + REAL_MODEL_TARGET_RETRY_POLICY, +) from tests.mock_vws.utils.backend_capabilities import BackendCapabilities from tests.mock_vws.utils.cloud_databases import AddCloudDatabase from tests.mock_vws.utils.model_target_response_policy import ( handling_unexpected_model_target_responses, ) from tests.mock_vws.utils.model_target_retries import ( - retrying_transient_real_backend_failures, + retrying_model_target_requests, ) from tests.mock_vws.utils.recognition_counts import SeedRecognitionCounts from tests.mock_vws.utils.request_timeout_policy import ( @@ -407,7 +410,9 @@ def _enable_use_real_model_target_vuforia( del base_vws_url assert bool(monkeypatch) with ( - retrying_transient_real_backend_failures(), + retrying_model_target_requests( + retrying=REAL_MODEL_TARGET_RETRY_POLICY, + ), handling_request_timeouts(handler=_handle_real_request_timeout), handling_unexpected_model_target_responses( handler=_handle_real_model_target_response, diff --git a/tests/mock_vws/test_model_target_retries.py b/tests/mock_vws/test_model_target_retries.py index 799dca231..81a5c6a32 100644 --- a/tests/mock_vws/test_model_target_retries.py +++ b/tests/mock_vws/test_model_target_retries.py @@ -4,19 +4,26 @@ layered on it, which cannot be provoked against a backend on demand. """ -import time from http import HTTPMethod, HTTPStatus import pytest import requests import responses from beartype import beartype +from tenacity import Retrying +from tenacity.retry import retry_if_exception_type +from tenacity.stop import stop_after_attempt +from tenacity.wait import wait_none +from tests.mock_vws.fixtures.real_model_target_retries import ( + MODEL_TARGET_RETRY_ATTEMPTS, + REAL_MODEL_TARGET_RETRY_POLICY, +) +from tests.mock_vws.fixtures.vuforia_backends import VuforiaBackend from tests.mock_vws.utils import ModelTargetEndpoint from tests.mock_vws.utils.model_target_retries import ( - MODEL_TARGET_RETRY_ATTEMPTS, model_target_get, - retrying_transient_real_backend_failures, + retrying_model_target_requests, send_with_transient_retries, ) @@ -31,25 +38,25 @@ def _response(*, status_code: HTTPStatus) -> requests.Response: return response -@beartype -def _do_not_sleep(seconds: float) -> None: - """Do not sleep.""" - del seconds +@pytest.fixture(name="retry_policy") +def fixture_retry_policy() -> Retrying: + """Use the real retry criteria without waiting between attempts.""" + return REAL_MODEL_TARGET_RETRY_POLICY.copy(wait=wait_none()) -@pytest.fixture(autouse=True, name="no_sleep") -def fixture_no_sleep(*, monkeypatch: pytest.MonkeyPatch) -> None: - """Do not wait between attempts.""" - monkeypatch.setattr(target=time, name="sleep", value=_do_not_sleep) +@pytest.fixture(name="_model_target_backend") +def fixture_model_target_backend() -> VuforiaBackend: + """Select the real policy without credential lookup or requests.""" + return VuforiaBackend.REAL -def test_transient_response_is_retried() -> None: +def test_transient_response_is_retried(*, retry_policy: Retrying) -> None: """A transient gateway response is followed by another attempt, and the response to that attempt is returned. """ remaining = [HTTPStatus.BAD_GATEWAY, HTTPStatus.OK] - with retrying_transient_real_backend_failures(): + with retrying_model_target_requests(retrying=retry_policy): response = send_with_transient_retries( method=HTTPMethod.GET, send=lambda: _response(status_code=remaining.pop(0)), @@ -58,7 +65,7 @@ def test_transient_response_is_retried() -> None: assert response.status_code == HTTPStatus.OK -def test_persistent_transient_response() -> None: +def test_persistent_transient_response(*, retry_policy: Retrying) -> None: """The last response is returned rather than hidden, so that the caller's assertion reports the real status and body. """ @@ -70,7 +77,7 @@ def send() -> requests.Response: attempts += 1 return _response(status_code=HTTPStatus.SERVICE_UNAVAILABLE) - with retrying_transient_real_backend_failures(): + with retrying_model_target_requests(retrying=retry_policy): response = send_with_transient_retries( method=HTTPMethod.GET, send=send, @@ -80,7 +87,7 @@ def send() -> requests.Response: assert attempts == MODEL_TARGET_RETRY_ATTEMPTS -def test_persistent_transport_failure() -> None: +def test_persistent_transport_failure(*, retry_policy: Retrying) -> None: """A transport failure on the last attempt is not swallowed.""" def send() -> requests.Response: @@ -93,7 +100,7 @@ def send() -> requests.Response: raise requests.exceptions.ConnectionError(msg) with ( - retrying_transient_real_backend_failures(), + retrying_model_target_requests(retrying=retry_policy), pytest.raises( expected_exception=requests.exceptions.ConnectionError, match=r"Connection aborted\.", @@ -102,7 +109,7 @@ def send() -> requests.Response: _ = send_with_transient_retries(method=HTTPMethod.GET, send=send) -def test_error_response_is_not_retried() -> None: +def test_error_response_is_not_retried(*, retry_policy: Retrying) -> None: """A response which came from Vuforia rather than from the gateway is returned as it is, however unsuccessful it is. """ @@ -114,13 +121,13 @@ def send() -> requests.Response: attempts += 1 return _response(status_code=HTTPStatus.UNAUTHORIZED) - with retrying_transient_real_backend_failures(): + with retrying_model_target_requests(retrying=retry_policy): _ = send_with_transient_retries(method=HTTPMethod.GET, send=send) assert attempts == 1 -def test_mutating_request_is_not_retried() -> None: +def test_mutating_request_is_not_retried(*, retry_policy: Retrying) -> None: """A request which is not safe to repeat is sent exactly once, even when it fails transiently. @@ -135,13 +142,13 @@ def send() -> requests.Response: attempts += 1 return _response(status_code=HTTPStatus.BAD_GATEWAY) - with retrying_transient_real_backend_failures(): + with retrying_model_target_requests(retrying=retry_policy): _ = send_with_transient_retries(method=HTTPMethod.POST, send=send) assert attempts == 1 -def test_configured_failure_is_not_retried() -> None: +def test_configured_failure_is_not_retried(*, retry_policy: Retrying) -> None: """A 5xx response is returned immediately outside the context. Tests which configure a mock to fail with a 5xx expect exactly @@ -157,7 +164,7 @@ def send() -> requests.Response: # Enter and leave the context first, to check it is switched off # again for a mock backend test which runs after a real one. - with retrying_transient_real_backend_failures(): + with retrying_model_target_requests(retrying=retry_policy): pass response = send_with_transient_retries( @@ -170,7 +177,7 @@ def send() -> requests.Response: @responses.activate -def test_endpoint_send() -> None: +def test_endpoint_send(*, retry_policy: Retrying) -> None: """A ``GET`` endpoint retries a transient failure. This is the path which the cross-cutting Model Target endpoint tests @@ -193,7 +200,7 @@ def test_endpoint_send() -> None: takes_json_body=False, ) - with retrying_transient_real_backend_failures(): + with retrying_model_target_requests(retrying=retry_policy): response = endpoint.send() assert response.status_code == HTTPStatus.UNAUTHORIZED @@ -202,7 +209,7 @@ def test_endpoint_send() -> None: @responses.activate -def test_model_target_get() -> None: +def test_model_target_get(*, retry_policy: Retrying) -> None: """``model_target_get`` retries a transient failure.""" gateway_response = responses.Response( method=responses.GET, url=_URL, status=HTTPStatus.GATEWAY_TIMEOUT @@ -213,9 +220,77 @@ def test_model_target_get() -> None: assert responses.add(gateway_response) is gateway_response assert responses.add(dataset_response) is dataset_response - with retrying_transient_real_backend_failures(): + with retrying_model_target_requests(retrying=retry_policy): response = model_target_get(url=_URL, headers={}, timeout=30) assert response.content == b"dataset" assert gateway_response.call_count == 1 assert dataset_response.call_count == 1 + + +@pytest.mark.usefixtures("verify_model_target_mock_vuforia") +def test_real_adapter_installs_retry_policy() -> None: + """Real adapter setup installs its policy without account access.""" + remaining = [HTTPStatus.BAD_GATEWAY, HTTPStatus.OK] + response = send_with_transient_retries( + method=HTTPMethod.GET, + send=lambda: _response(status_code=remaining.pop(0)), + ) + assert response.status_code == HTTPStatus.OK + assert not bool(remaining) + + +def test_nested_retry_policies() -> None: + """Nested policies restore the outer policy after a raised + exception. + """ + outer = Retrying( + retry=retry_if_exception_type(exception_types=ValueError), + stop=stop_after_attempt(max_attempt_number=2), + wait=wait_none(), + reraise=True, + ) + inner = outer.copy( + retry=retry_if_exception_type(exception_types=KeyError), + ) + error = ValueError("Controlled failure") + response = _response(status_code=HTTPStatus.OK) + remaining: list[ValueError | requests.Response] = [ + error, + response, + error, + error, + response, + error, + ] + + def send() -> requests.Response: + """Return the next controlled response or raise its exception.""" + outcome = remaining.pop(0) + if isinstance(outcome, ValueError): + raise outcome + return outcome + + with retrying_model_target_requests(retrying=outer): + assert ( + send_with_transient_retries(method=HTTPMethod.GET, send=send) + is response + ) + with ( + pytest.raises( + expected_exception=ValueError, match="Controlled failure" + ) as exc, + retrying_model_target_requests(retrying=inner), + ): + _ = send_with_transient_retries(method=HTTPMethod.GET, send=send) + assert exc.value is error + assert ( + send_with_transient_retries(method=HTTPMethod.GET, send=send) + is response + ) + with pytest.raises( + expected_exception=ValueError, match="Controlled failure" + ) as exc: + _ = send_with_transient_retries(method=HTTPMethod.GET, send=send) + assert exc.value is error + assert not bool(remaining) diff --git a/tests/mock_vws/utils/__init__.py b/tests/mock_vws/utils/__init__.py index a093f6562..22db4dfee 100644 --- a/tests/mock_vws/utils/__init__.py +++ b/tests/mock_vws/utils/__init__.py @@ -148,8 +148,8 @@ class ModelTargetEndpoint: def send(self) -> Response: """Send the request. - A safe request to the real Model Target Web API is retried if it - hits a transient gateway failure. See + Safe requests use the retry policy supplied by the backend + adapter. See :py:mod:`tests.mock_vws.utils.model_target_retries`. """ return send_with_transient_retries( diff --git a/tests/mock_vws/utils/model_target_retries.py b/tests/mock_vws/utils/model_target_retries.py index 10b1b3e0a..62e6c80ce 100644 --- a/tests/mock_vws/utils/model_target_retries.py +++ b/tests/mock_vws/utils/model_target_retries.py @@ -1,112 +1,34 @@ -"""Retrying transient failures from the real Model Target Web API. - -The real Model Target Web API sits behind a load balancer which -occasionally answers a perfectly good request with a gateway error, or -drops the connection. That has nothing to do with the contract under -test, so a bounded number of retries for requests which are safe to -repeat keeps a one-off gateway error from failing CI. - -Only requests to the real backend are retried. The mock backends -answer immediately, and some tests configure a mock to return a 5xx -deliberately, so retrying those would be both pointless and wrong. -The policy is therefore switched on for the duration of a test against -the real Model Target backend and is off at all other times. - -Only ``GET`` requests are retried. Repeating a dataset creation -``POST`` can create a second dataset and can consume the account's -Model Target training allowance, and the outcome of a repeat is -ambiguous, so mutating requests are sent exactly once. +"""Apply retry policies supplied by Model Target backend adapters. + +Requests are sent once by default. An adapter can install a scoped retry +policy for GET requests; mutating requests are always sent exactly once. """ import contextlib from collections.abc import Callable, Generator, Mapping -from http import HTTPMethod, HTTPStatus +from http import HTTPMethod import requests from beartype import beartype -from tenacity import RetryCallState, Retrying -from tenacity.retry import retry_if_exception_type, retry_if_result -from tenacity.stop import stop_after_attempt -from tenacity.wait import wait_exponential_jitter +from tenacity import Retrying from vws.response import Response -# Status codes which mean "the gateway could not get an answer from -# Vuforia", rather than "Vuforia answered, and this is the answer". -TRANSIENT_MODEL_TARGET_STATUS_CODES = frozenset( - { - HTTPStatus.BAD_GATEWAY, - HTTPStatus.SERVICE_UNAVAILABLE, - HTTPStatus.GATEWAY_TIMEOUT, - }, -) - -# Transport failures which mean the request may never have been -# answered. -TRANSIENT_MODEL_TARGET_EXCEPTIONS = ( - requests.exceptions.ConnectionError, - requests.exceptions.Timeout, -) - -# The total number of times a safe request is sent, including the first -# attempt. This is deliberately small: a persistent upstream failure -# must still fail the test quickly. -MODEL_TARGET_RETRY_ATTEMPTS = 3 - -# Whether requests are going to the real Model Target backend. -# -# This is a list, rather than a boolean, so that the context manager can -# set and unset it without a ``global`` statement, in the same way as -# the running in-memory mock is tracked in ``vuforia_backends``. -_REAL_BACKEND: list[None] = [] +_RETRY_POLICIES: list[Retrying] = [] @contextlib.contextmanager @beartype -def retrying_transient_real_backend_failures() -> Generator[None]: - """Retry transient failures of safe requests sent within this - context. +def retrying_model_target_requests(*, retrying: Retrying) -> Generator[None]: + """Use the supplied retry policy for safe requests within this context. Yields: - ``None``, with the retry policy switched on. + ``None``, with the supplied policy active. """ - _REAL_BACKEND.append(None) + _RETRY_POLICIES.append(retrying) try: yield finally: - _REAL_BACKEND.pop() - - -@beartype -def _is_transient(response: Response | requests.Response) -> bool: - """Whether a response is a transient gateway failure.""" - return response.status_code in TRANSIENT_MODEL_TARGET_STATUS_CODES - - -@beartype -def _last_outcome(retry_state: RetryCallState) -> Response | requests.Response: - """Give up with the outcome of the last attempt. - - A transient failure on the last attempt is not hidden: a response is - returned as it is, so that the caller's assertion reports the real - status and body, and a transport failure propagates. - """ - assert retry_state.outcome is not None - outcome = retry_state.outcome.result() - assert isinstance(outcome, Response | requests.Response) - return outcome - - -_RETRYING = Retrying( - retry=( - retry_if_exception_type( - exception_types=TRANSIENT_MODEL_TARGET_EXCEPTIONS, - ) - | retry_if_result(predicate=_is_transient) - ), - stop=stop_after_attempt(max_attempt_number=MODEL_TARGET_RETRY_ATTEMPTS), - wait=wait_exponential_jitter(initial=1, max=10), - retry_error_callback=_last_outcome, -) + _ = _RETRY_POLICIES.pop() @beartype @@ -118,7 +40,7 @@ def send_with_transient_retries[ResponseT: (Response, requests.Response)]( """Send a Model Target request, retrying transient failures. The request is sent exactly once unless it is a ``GET`` sent while - :py:func:`retrying_transient_real_backend_failures` is active. + :py:func:`retrying_model_target_requests` is active. Args: method: The HTTP method of the request. @@ -128,11 +50,11 @@ def send_with_transient_retries[ResponseT: (Response, requests.Response)]( Returns: The response to the last attempt. """ - retryable = bool(_REAL_BACKEND) and method == HTTPMethod.GET + retryable = bool(_RETRY_POLICIES) and method == HTTPMethod.GET if not retryable: return send() - return _RETRYING(send) + return _RETRY_POLICIES[-1](send) @beartype