Skip to content

Use the client timeout in run_sync and run - #105

Merged
yutkin merged 1 commit into
masterfrom
fix/run-timeout
Oct 30, 2026
Merged

yutkin merged 1 commit into
masterfrom
fix/run-timeout

Conversation

@yutkin

@yutkin yutkin commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Summary

InferenceClient.run_sync and run used 300 seconds when the caller gave no timeout_seconds. The client timeout_seconds had no effect on them. The HTTP verb helpers (get, post and the others) already use the client value.

 InferenceClient(timeout_seconds=900)
   post(path)                  # 900 s
-  run_sync(data)              # 300 s
-  run(data)                   # 300 s
+  run_sync(data)              # 900 s
+  run(data)                   # 900 s
   run_sync(data, timeout_seconds=7)   # 7 s, no change
 def run_sync(self, data, path='', timeout_seconds=..., ...):
-    timeout_seconds: int = 60 * 5
+    timeout_seconds: int | None = None   # _make_request uses self.timeout_seconds

Deployment.run_sync and Deployment.run now also default to None. They send the value to the inference client, and the client value applies.

The old timeout tests used a client with the default of 300 seconds. That value is the same as the old hard-coded value, so the tests passed with the bug. The new test uses a client with 900 seconds for each entry point.

Evidence

  • Before: the new test fails on master source.
    client = InferenceClient(..., timeout_seconds=900)
    client.run_sync(...)  -> request timeout == 900   FAILED (300)
    client.run(...)       -> request timeout == 900   FAILED (300)
    client.run(..., no_response=True)                 FAILED (300)
    pytest -k timeout: 3 failed, 15 passed
    
    After: all unit tests pass.
    665 passed
    ruff check: All checks passed!
    ruff format --check: 144 files already formatted
    

Merge Danger

Door: two-way

A revert brings back the old default.

Blast Radius: small

Only a caller that sets timeout_seconds on the client and does not set it on run_sync or run sees a change. That caller now gets the client value, not 300 seconds. A client with a value lower than 300 now stops these calls earlier. Deployment creates its client with the default of 300, so Deployment calls do not change.

🤖 Generated with Claude Code

InferenceClient.run_sync and run always used a timeout of 300 seconds
when the caller gave none. The client timeout_seconds had no effect on
them. With InferenceClient(..., timeout_seconds=900), post() waited 900
seconds and run_sync() stopped after 300 seconds.

run_sync and run now use the client timeout_seconds when the caller
gives none, like the HTTP verb helpers. Deployment.run_sync and run now
pass None to the inference client, so the client setting applies.

The old timeout tests used a client with the default of 300 seconds,
so they passed with the bug. The new test uses a client with 900
seconds for each entry point.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@yutkin
yutkin marked this pull request as ready for review October 30, 2026 11:09
@yutkin
yutkin merged commit a96b637 into master Oct 30, 2026
7 checks passed
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