test(providers): Mistral batch retry + retry-log detail + comment (#893 r4)
Round-4 review on PR #893 (no blockers, minor items): - Document why Mistral's _is_transient is SDK-level only (429/5xx): a bare connection drop the SDK surfaces as httpx/ConnectionError isn't an SDKError and isn't retried here by design — the pod-rollover target is the gateway (OpenAI-compatible) path, which does cover connection errors. - Include the last error (%r) in the retry helper's "not resolved after N attempts" error log. - Add test_mistral_embed_batch_retries_on_5xx (batch path parity with embed()). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
c4b6d4a017
commit
81f7403b12
@@ -77,10 +77,11 @@ def retry_on_transient(
|
|||||||
retry_delay = min(retry_delay * 2, MAX_RETRY_DELAY)
|
retry_delay = min(retry_delay * 2, MAX_RETRY_DELAY)
|
||||||
|
|
||||||
logger.error(
|
logger.error(
|
||||||
"%s %s not resolved after %d attempts",
|
"%s %s not resolved after %d attempts: %r",
|
||||||
provider_name,
|
provider_name,
|
||||||
label,
|
label,
|
||||||
MAX_RETRIES,
|
MAX_RETRIES,
|
||||||
|
last_error,
|
||||||
)
|
)
|
||||||
if last_error is None: # pragma: no cover — loop above always sets this
|
if last_error is None: # pragma: no cover — loop above always sets this
|
||||||
raise RuntimeError("retry loop exited without capturing an error")
|
raise RuntimeError("retry loop exited without capturing an error")
|
||||||
|
|||||||
@@ -31,7 +31,15 @@ _NO_EMBEDDING_MODEL_MSG = "Embedding not supported - no embedding_model configur
|
|||||||
|
|
||||||
|
|
||||||
def _is_transient(exc: BaseException) -> bool:
|
def _is_transient(exc: BaseException) -> bool:
|
||||||
"""Retry HTTP 429 (rate limit) and 5xx (server/transient) SDKErrors."""
|
"""Retry HTTP 429 (rate limit) and 5xx (server/transient) SDKErrors.
|
||||||
|
|
||||||
|
Scope is deliberately SDK-level: only ``SDKError`` (an HTTP-status error) is
|
||||||
|
caught by the decorator, so a pure connection drop that the Mistral SDK
|
||||||
|
surfaces as a bare ``httpx``/``ConnectionError`` is NOT retried here. The
|
||||||
|
primary pod-rollover resilience target (card 309) is the gateway path via
|
||||||
|
the OpenAI-compatible client, which does cover connection errors; direct
|
||||||
|
Mistral is a self-hoster fallback where 429/5xx is the common transient.
|
||||||
|
"""
|
||||||
status = getattr(exc, "status_code", None)
|
status = getattr(exc, "status_code", None)
|
||||||
return status == 429 or (isinstance(status, int) and status >= 500)
|
return status == 429 or (isinstance(status, int) and status >= 500)
|
||||||
|
|
||||||
|
|||||||
@@ -353,3 +353,23 @@ async def test_mistral_embed_retries_on_5xx(mock_mistral_client, monkeypatch):
|
|||||||
embedding = await provider.embed("hello")
|
embedding = await provider.embed("hello")
|
||||||
assert embedding == [0.1, 0.2, 0.3]
|
assert embedding == [0.1, 0.2, 0.3]
|
||||||
assert mock_mistral_client.embeddings.create_async.await_count == 2
|
assert mock_mistral_client.embeddings.create_async.await_count == 2
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
async def test_mistral_embed_batch_retries_on_5xx(mock_mistral_client, monkeypatch):
|
||||||
|
"""The batch path (_embed_batch_request) shares the transient retry too."""
|
||||||
|
from nextcloud_mcp_server.providers import _retry
|
||||||
|
|
||||||
|
monkeypatch.setattr(_retry.anyio, "sleep", AsyncMock(return_value=None))
|
||||||
|
|
||||||
|
err = SDKError.__new__(SDKError)
|
||||||
|
err.status_code = 503
|
||||||
|
|
||||||
|
mock_mistral_client.embeddings.create_async = AsyncMock(
|
||||||
|
side_effect=[err, _make_response([[0.1, 0.2], [0.3, 0.4]])]
|
||||||
|
)
|
||||||
|
provider = MistralProvider(api_key="test-key", embedding_model="mistral-embed")
|
||||||
|
|
||||||
|
embeddings = await provider.embed_batch(["a", "b"])
|
||||||
|
assert embeddings == [[0.1, 0.2], [0.3, 0.4]]
|
||||||
|
assert mock_mistral_client.embeddings.create_async.await_count == 2
|
||||||
|
|||||||
Reference in New Issue
Block a user