Skip to content

Handle zero timeout in the internal connection pool - #124

Closed
SuhrudhC wants to merge 2 commits into
openaq:mainfrom
SuhrudhC:fix/pool-timeouts
Closed

SuhrudhC wants to merge 2 commits into
openaq:mainfrom
SuhrudhC:fix/pool-timeouts

Conversation

@SuhrudhC

@SuhrudhC SuhrudhC commented Oct 6, 2026 •

Copy link
Copy Markdown

Problem and fix

Fixes #123.

The internal connection pool treats pool_timeout=0.0 as None because deadline creation checks truthiness. An internal acquisition with zero timeout can therefore wait indefinitely when the pool is full.

Scope after maintainer feedback: I have not established a reproduction through the SDK's supported public API. Both current main and the pending v1.2.0 branch construct OpenAQ with a default pool timeout of None; the constructor exposes no public pool-timeout setting, and _transport injection is documented as internal use only. This is a defensive internal correction, with no demonstrated effect on ordinary public-client usage. The PR is draft pending maintainer direction on whether that scope is useful.

Use pool_timeout is not None instead. Zero timeout now raises TimeoutError immediately when the pool is full, while an available new or idle connection can still be acquired. Positive timeouts and None retain their existing behavior.

Validation

  • Added a deterministic full-pool regression that fails on the original implementation, without leaving a blocked thread or using sleeps.
  • Added coverage for acquiring new/reused connections with zero timeout.
  • Added a client-level regression using concurrent client.countries.list() calls with the real client, transport, and pool; only the HTTPS connection is mocked. With an internally injected zero-timeout transport and a full pool, the second client call blocks on the original implementation and raises the SDK's TimeoutError with the patch. The pool=None case still waits for capacity and completes successfully. Both cases also verify a later client request can reuse the released connection.
  • 1,077 unit tests pass on Python 3.12.
  • Repository Ruff lint/format checks and mypy pass.
  • Confirmed the same deadline expression is present in the pending v1.2.0 branch; this PR targets main per the contribution guide and can be retargeted if preferred.

Generated and validated with Codex. No API requests or production data were used for the regression tests.

@SuhrudhC SuhrudhC changed the title Fix zero pool timeout waiting indefinitely Handle zero timeout in the internal connection pool Oct 6, 2026
@SuhrudhC
SuhrudhC marked this pull request as draft October 6, 2026 15:37
@russbiggs

Copy link
Copy Markdown
Member

Closing at detailed in #123

@russbiggs russbiggs closed this Oct 6, 2026
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.

Zero pool timeout waits indefinitely when the connection pool is full

2 participants