Skip to content

DecisionTarget: optional client reuse and retries - #131

Open
avalyset wants to merge 3 commits into
SimulaMet:devfrom
avalyset:feat/decision-client-reuse
Open

avalyset wants to merge 3 commits into
SimulaMet:devfrom
avalyset:feat/decision-client-reuse

Conversation

@avalyset

Copy link
Copy Markdown
Contributor

Builds on #119. Part of #107.

Until #119 is merged, this branch carries its two commits. The change for review is the last commit, ccd99ee (see commit list).

DecisionTarget.send created a new httpx.AsyncClient per request and never retried (gaps 3 and 2 in #107). Both fixes are opt-in; the defaults keep current behaviour.

  • reuse_client=True keeps one client per event loop for all requests. aclose() and async with DecisionTarget(...) close it. Each sync run() gets its own loop, so a client created on an earlier loop is replaced, not reused. The target never closes an injected client=.
  • max_retries=N and retry_backoff (defaults 0 and 0.5 s, matching ModelTarget) retry httpx.TransportError and statuses 429, 500, 502, 503 and 504, waiting retry_backoff * 2**attempt between attempts. On 429 or 503, a Retry-After header (seconds or HTTP date) replaces that wait, capped at 60 s. After the last attempt, the original error or the usual HTTP <status> error is raised. Other 4xx responses are not retried.

Tested with pytest -p no:randomly on dev at cdb95cf plus #119, in a fresh venv with pip install -e ".[dev,tracing,plot,inspect]": 1737 passed, 3 skipped, 0 failed. 17 new tests in tests/test_decision_target.py.

Design questions:

  • Should reuse_client be the default? It is opt-in here because nothing in the auditor closes a target after a run, so a default client would stay open until garbage collection.
  • Should 500 be retried? Here it is, as a transient server failure. The alternative is to retry only 429, 502, 503 and 504.

AI disclosure: implemented with Claude Code.

ASRTarget, DecisionTarget and HTTPAppTarget add an endpoint block (scheme,
host, port, via_proxy) to TargetResponse.raw, warn once at construction on
plain http to a non-loopback host, and create their client through one
helper. ASRTarget uploads the file as audio<suffix>.
reuse_client=True keeps one httpx client per event loop instead of making
one per request; aclose() and the async context manager close it. A client
from an earlier loop (each sync run() has its own) is replaced, not reused.

max_retries and retry_backoff retry transport errors and 429/5xx with a
doubling backoff, with ModelTarget's names and defaults (0 and 0.5 s). A
Retry-After header on a 429 or 503 replaces the wait, capped at 60 s. Both
features default to today's behaviour: one client per request, one attempt.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant