Skip to content

python(fix): bound sync calls at the transport layer#613

Merged
wei-qlu merged 2 commits into
mainfrom
python/fix-waiter-timeout
Jun 3, 2026
Merged

python(fix): bound sync calls at the transport layer#613
wei-qlu merged 2 commits into
mainfrom
python/fix-waiter-timeout

Conversation

@wei-qlu
Copy link
Copy Markdown
Contributor

@wei-qlu wei-qlu commented Jun 3, 2026

Summary

The sync timeout cap wrapped the whole coroutine, so it aborted long-running waiters (wait_until_complete, wait_and_download) at the cap even when they were polling fine or the caller passed a larger timeout_secs.

This removes the cap and bounds calls at the transport layer instead: gRPC keeps its 60s deadline, and REST gets a default 60s timeout it never had (an inter-byte read timeout, so stalled sockets fail fast but healthy long downloads don't). Waiters now honor their own timeout_secs.

REST requests now default to a 60s timeout, configurable via RestConfig(request_timeout=...) and overridable per call.

Test plan

  • Unit tests (REST timeout default/override/disable, plus existing sync-wrapper
    and gRPC deadline tests).
  • Manual: ran a data import that takes a few minutes and confirmed the sync
    wait_until_complete() runs to completion instead of being cut off at the old cap.

@wei-qlu wei-qlu requested a review from alexluck-sift June 3, 2026 20:05
@wei-qlu wei-qlu marked this pull request as ready for review June 3, 2026 20:05
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 3, 2026

Python docs preview: https://sift-stack.github.io/sift/python/pr-613/

Deployed from a378263. The link may take up to a minute to become live as GitHub Pages propagates.

@wei-qlu wei-qlu merged commit b31423e into main Jun 3, 2026
23 checks passed
@wei-qlu wei-qlu deleted the python/fix-waiter-timeout branch June 3, 2026 21:01
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.

2 participants