Honor configured timeouts instead of capping transfers at 30s (fixes #253)#263
Open
swimmer-303 wants to merge 2 commits into
Open
Honor configured timeouts instead of capping transfers at 30s (fixes #253)#263swimmer-303 wants to merge 2 commits into
swimmer-303 wants to merge 2 commits into
Conversation
The session-level timeoutIntervalForResource=30 silently overrode the transcription_timeout_seconds, post_processing_timeout_seconds, and context_request_timeout_seconds settings, killing long transfers to slow local models. Sessions are now cached per resource timeout derived from each request's configured timeout, preserving connection reuse. Uploads keep their fresh-session-per-call behavior. Fixes zachlatta#253 (cherry picked from commit 9e89b6e)
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesTimeout-aware transport
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out from #254 so the small bug fix can land independently.
LLMAPITransportpinnedtimeoutIntervalForResource = 30at the session level, silently overriding the README-documentedtranscription_timeout_seconds/post_processing_timeout_seconds/context_request_timeout_secondsoverrides — any transfer over 30s was killed regardless of settings, breaking slow local models.Sessions now derive their whole-transfer budget from each request's configured timeout (30s floor), cached per timeout value so TLS connection reuse is preserved. Uploads keep their fresh-session-per-call poisoning defense.
Fixes #253
🤖 Generated with Claude Code
Summary by CodeRabbit