Skip to content

Feat/configurable timeout - #76

Open
geoffhancock wants to merge 3 commits into
mainfrom
feat/configurable-timeout
Open

geoffhancock wants to merge 3 commits into
mainfrom
feat/configurable-timeout

Conversation

@geoffhancock

@geoffhancock geoffhancock commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Expose request timeout and historical chunk size, and point timeout errors at both

Why

The client hardcodes a 60 s read timeout and splits historical pulls into 30-day requests. Neither is configurable. When the API is slow to answer a large span, a user waits ~4 minutes (60 s x 4 attempts from the session's retry policy) and then gets RuntimeError: API Request Failed: ... Read timed out with no indication of why and nothing to change. The only workaround has been to monkeypatch the private _get_chunks.

This happened in August 2026: 30-day co2_moer pulls for some regions took over 180 s server-side while the same query at 10 days took ~11 s. The backend cause is being addressed separately; this PR is about not leaving SDK users stranded the next time something similar happens. This PR adds knobs; a follow-on PR will propose a more graceful retry strategy.

What changes

  1. timeout on the constructor (WattTimeBase.__init__, default (10, 60)), applied at all three HTTP call sites. Accepts the standard requests forms: a number, a (connect, read) tuple, or None.
  2. chunk_size on get_historical_jsons / _pandas / _csv (default None = 30 days, unchanged). _get_chunks now accepts None and rejects sizes of 5 minutes or less.
  3. Timeout errors carry a hint naming both knobs. Once retries are exhausted, requests raises ConnectionError -> MaxRetryError -> ReadTimeoutError rather than ReadTimeout, so a small _is_timeout helper walks the exception chain.

Both new parameters are keyword-only, and the internal call chain (get_historical_csv -> _pandas -> _jsons) passes every defaulted argument by keyword -- the same convention as the include_imputed_marker misrouting fix, so argument-order differences between signatures cannot silently route a value to the wrong parameter.

Implications

  • The default connect timeout on the data path drops from 60 s to 10 s. That request used a scalar 60 (connect = read = 60); it now shares the (10, 60) default with login and register. The default read timeout of 60 s is unchanged at all three sites.
  • _get_chunks raises ValueError for chunk_size <= 5 min. Before, zero or negative never terminated and 5 minutes produced an inverted chunk. No caller passed such values.
  • Timeouts are per attempt. The session retries up to 3 times with backoff, so a failing request takes roughly 4 x read_timeout + 6 s to surface. Documented on the parameter.
  • Nothing changes by default. Default chunking, default timeouts for login/register, and response shapes are as before. Both new parameters are keyword-only, so existing positional callers are unaffected and the positional contract of every signature is frozen where it is today.
  • Smaller chunk_size means more requests and, with include_meta=True, more meta rows -- one per response, as today.
  • The README note for timeout / chunk_size is the final commit on its own; drop it if you would rather not have that section.

Stacked on #75: the first commit here is #75's and drops out once it merges. Both PRs touch the _csv -> _pandas -> _jsons call chain, so this ordering avoids a conflict.

Tests

Nine mocked tests, no network: the timeout reaches all three call sites; tuple/scalar/None accepted; the new parameters are keyword-only and existing positional calls still work; the hint appears for both the bare and the retry-wrapped timeout (with __cause__ preserved) and not for other errors; chunk_size is honoured by the chunker, forwarded from _pandas, and controls the request count; the validation floor.

Follow-up (separate PR #77 )

These knobs let a user react, but they still have to hit the error first. PR #77 proposes a design concept to make retry policy failure-class-aware so the common cases need no intervention: split the span and retry smaller on read timeout / 504 instead of re-issuing the identical request four times; keep backoff for 5xx; refresh the token on 401; fail fast on other 4xx. It builds on the chunk_size plumbing here, which is why this lands first.

🤖 Generated with the help of Claude Code

geoffhancock and others added 3 commits September 18, 2026 13:14
…ical_csv

get_historical_csv passed include_imputed_marker as the sixth positional
argument to get_historical_pandas, whose sixth parameter is include_meta.
As a result, get_historical_csv(..., include_imputed_marker=True) wrote a
CSV containing a meta column and no imputed_data_used column.

Pass all defaulted parameters by keyword in the internal call chain
(get_historical_csv -> get_historical_pandas -> get_historical_jsons) so
argument order differences between signatures cannot misroute a flag.
Public signatures are unchanged.

Adds a regression test asserting the CSV contains imputed_data_used and
not meta when include_imputed_marker=True.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eout

Add a keyword-only `timeout` to WattTimeBase.__init__ (default (10, 60),
matching the existing login/register literals) and pass it at all three
HTTP call sites. The data request previously used a scalar 60, so its
default connect timeout drops from 60 s to 10 s; default read timeouts
are unchanged.

Add a keyword-only `chunk_size` to get_historical_jsons/_pandas/_csv,
forwarded to _get_chunks, which now accepts None (30 days) and rejects
sizes of 5 minutes or less -- the per-chunk trim -- instead of looping
forever or producing inverted chunks.

When a request fails on a timeout, append a hint to the RuntimeError
naming both knobs. Once the session's retries are exhausted, requests
raises ConnectionError -> MaxRetryError -> ReadTimeoutError rather than
ReadTimeout, so _is_timeout walks the exception chain.

Motivation: in August 2026, 30-day historical pulls for some regions took
over 180 s server-side while the same pulls at 10 days took ~11 s. Users
hit an opaque read timeout after ~4 minutes with no supported lever; the
only workaround was monkeypatching _get_chunks.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Add a short "Tuning requests that time out" note under the historical
data example showing both options and that the timeout is per attempt.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@geoffhancock
geoffhancock marked this pull request as ready for review September 18, 2026 19:12
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