Skip to content

feat(adk): make AgentEx client HTTP timeouts configurable by env var - #510

Open
chakrris wants to merge 5 commits into
nextfrom
chakrris/client-timeout-env-vars
Open

feat(adk): make AgentEx client HTTP timeouts configurable by env var#510
chakrris wants to merge 5 commits into
nextfrom
chakrris/client-timeout-env-vars

Conversation

@chakrris

@chakrris chakrris commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

The AgentEx client's HTTP timeout is fixed at the SDK default and cannot be changed from application code. This adds four environment variables to configure it, with defaults equal to today's values so nothing changes for an unconfigured process.

The problem

create_async_agentex_client() passes no timeout, so every client falls back to DEFAULT_TIMEOUT:

Timeout(connect=5.0, read=300, write=300, pool=300)

An application cannot override this. The ADK builds its own clients internally at 14 sites across messages, tasks, events and tracing, all through that factory, and none takes a timeout from the caller. No environment variable controlled it either, so wrapping the factory locally only covers the paths an app constructs itself, not the ADK's.

The connect timeout is the one that bites. An AgentEx backend accepts connections serially, so connect latency grows with the number of concurrent callers. Measured against a local backend with GET /readyz, a 10s client timeout, and no failures at any level:

Concurrent connects p50 max
1 84 ms 84 ms
20 423 ms 452 ms
100 500 ms 537 ms
200 1,010 ms 1,075 ms

Latency is linear in concurrency, which is the signature of serialised accepts rather than slow request handling. An agent running 100 concurrent Temporal activities, each making ADK calls, crosses the 5s budget and fails with httpcore.ConnectTimeout. One run produced 197 such failures from a single activity type.

A 5s connect alongside a 300s read is also internally inconsistent: the client will wait five minutes for a response but only five seconds to open the socket.

What changed

The whole change is in src/agentex/lib/adk/utils/_modules/client.py. _timeout_from_env() reads four variables from os.environ, and create_async_agentex_client() applies the result when the caller passes no timeout:

AGENTEX_CLIENT_CONNECT_TIMEOUT_SECONDS   default 5.0
AGENTEX_CLIENT_READ_TIMEOUT_SECONDS      default 300.0
AGENTEX_CLIENT_WRITE_TIMEOUT_SECONDS     default 300.0
AGENTEX_CLIENT_POOL_TIMEOUT_SECONDS      default 300.0

Because all 14 internal construction sites route through that factory, one change covers messages, tasks, events and tracing.

Three properties worth checking in review:

  • Defaults equal the current DEFAULT_TIMEOUT, so an unconfigured process behaves exactly as before.
  • An explicit timeout= argument still wins over the environment.
  • A malformed value raises a ValueError naming the variable, at the point the value is used.

Why not EnvironmentVariables

The first version of this PR declared the four values as fields on the shared EnvironmentVariables model, which is the repo's usual pattern. That turned out to be the wrong home for them, and the reasoning is worth recording because the failure was not obvious:

EnvironmentVariables.refresh() is called from about 20 places, none of which guard it, including AgentWorker startup and EnvAuth.auth_flow() on every authenticated request. A malformed timeout would therefore fail validation far from the client factory. The factory's try/except did not contain that; it only made the failure look handled at the one site that could have reported it clearly.

Removing that try/except exposed a second problem. agentex/lib/adk/utils/__init__.py constructs TemplatingModule() at module scope, and that constructor builds a client, so import agentex.lib.adk began requiring AGENT_NAME and ACP_URL to be set. The suppressed exception had been hiding an import-time dependency on a fully configured environment.

Reading os.environ directly avoids both. The shared model is untouched, so startup, authentication and import are unaffected, and a bad value affects only the timeout it describes.

Review guide

client.py is the entire behavioural change; it and its test are the only two files this PR touches. tests/lib/test_client_timeout_env.py covers the defaults, each override, the empty-string case, precedence of an explicit argument, that EnvAuth survives, the malformed-value error, and a regression test asserting the timeouts do not depend on the shared environment model.

Testing

uv sync requires uv >= 0.9 and this machine has 0.8.13, so I ran the suite through uvx with both packages installed editable. The 9 new tests pass, and tests/lib is 1028 passed / 3 skipped. pyright at the pinned 1.1.399 is clean on the touched files. The 12 errors in that run are all tests/lib/cli/test_agent_handlers.py raising on a Click DeprecationWarning under filterwarnings = error; they come from unpinned resolution in the ad-hoc environment rather than from uv.lock, and are unrelated to this change. ruff check and ruff format --check are clean on the touched files.

Greptile Summary

Adds environment-variable configuration for the AgentEx async client's connect, read, write, and pool timeouts.

  • Preserves the SDK’s existing timeout values when variables are unset or blank.
  • Gives explicit caller-provided timeout arguments precedence.
  • Reads timeout settings independently of the shared environment model, containing malformed-value failures to client construction.
  • Adds focused tests for defaults, overrides, precedence, malformed values, authentication attachment, and environment-model independence.

Confidence Score: 5/5

The PR appears safe to merge with no outstanding correctness, security, or repository-rule issues.

The previously reported shared-environment validation issue was fully addressed by reading timeout variables directly in the client factory, and that thread is resolved. The current implementation preserves existing defaults and explicit timeout precedence while limiting malformed configuration failures to client creation.

Important Files Changed

Filename Overview
src/agentex/lib/adk/utils/_modules/client.py Adds isolated environment parsing for all four HTTP timeout components while preserving explicit caller configuration.
tests/lib/test_client_timeout_env.py Covers default and configured timeout behavior, malformed input, explicit precedence, authentication, and regression boundaries.

Reviews (4): Last reviewed commit: "Drop an unrelated file change from the b..." | Re-trigger Greptile

The AgentEx client's timeout comes from the SDK's DEFAULT_TIMEOUT,
Timeout(connect=5.0, read=300, write=300, pool=300). Application code
cannot change it: the ADK constructs its own clients internally at 14
sites (messages, tasks, events, tracing), all through
create_async_agentex_client(), and none accepts a timeout from the caller.
No environment variable controlled it either.

The connect timeout is the one that matters. An AgentEx backend accepts
connections serially, so connect latency grows with the number of
concurrent callers. Measured against a local backend:

  concurrency    1     84 ms
  concurrency   20    423 ms
  concurrency  100    500 ms
  concurrency  200  1,010 ms

An agent running 100 concurrent activities, each making ADK calls, pushes
past the 5s budget and fails with httpcore.ConnectTimeout. One run
produced 197 such failures in a single activity. A 5s connect against a
300s read is also internally inconsistent.

Adds AGENTEX_CLIENT_{CONNECT,READ,WRITE,POOL}_TIMEOUT_SECONDS, following
the existing EnvVarKeys and EnvironmentVariables pattern. Defaults equal
the current DEFAULT_TIMEOUT, so an unconfigured process is unchanged. An
explicit timeout= argument still wins, and a malformed value falls back to
the SDK default with a warning rather than preventing client creation.
Comment thread src/agentex/lib/environment_variables.py Outdated
The EnvironmentVariables symbol is reached through the env_module alias,
so the direct import was dead and tripped ruff F401/I001 in CI.
@chakrris
chakrris changed the base branch from main to next September 7, 2026 01:02
Greptile flagged that adding the four timeout fields to the shared
EnvironmentVariables model meant a malformed value broke far more than the
client factory: refresh() is called from ~20 unguarded places, including
AgentWorker startup and EnvAuth.auth_flow on every request. The try/except in
create_async_agentex_client() did not contain that, it only made it look
handled.

Removing the try/except alone made it worse. agentex/lib/adk/utils/__init__.py
constructs TemplatingModule() at module scope, which builds a client, so
importing the ADK started requiring AGENT_NAME and ACP_URL to be set. The
suppressed exception had been hiding that.

Read the four values from os.environ in client.py instead. The shared model is
untouched, so startup, auth and import are unaffected, and a malformed value
raises a ValueError naming the variable at the point it is used rather than
being swallowed. Adds a regression test asserting the timeout does not depend
on the shared model.
AsyncAgentex.timeout is typed float | Timeout | None, so pyright rejected
reading .connect off it directly. ruff was clean, which is why CI caught this
and the local ruff run did not.
Reverting environment_variables.py in the previous commit checked it out from
origin/next rather than the merge base, which pulled AGENT_COMMIT_SHA from
another commit into this diff. This PR now touches only client.py and its test.
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