Skip to content

Add configurable timeout for passthrough endpoints#2

Closed
tanmaykm wants to merge 2 commits intov1.82.1_with_jh_fixesfrom
tan/timeoutconfig
Closed

Add configurable timeout for passthrough endpoints#2
tanmaykm wants to merge 2 commits intov1.82.1_with_jh_fixesfrom
tan/timeoutconfig

Conversation

@tanmaykm
Copy link
Member

@tanmaykm tanmaykm commented Mar 16, 2026 •
edited
Loading

  • Fix hardcoded 600-second timeout on pass-through endpoints that caused TimeoutError for long-running streaming requests (e.g. Anthropic API calls with extended thinking/tool use)
  • Add timeout field to PassThroughGenericEndpoint config, allowing per-endpoint timeout configuration
  • Added tests

Ref: https://github.com/JuliaComputing/JuliaHub/issues/21622

@tanmaykm tanmaykm requested review from lamdor and mortenpi March 16, 2026 13:55
@tanmaykm tanmaykm marked this pull request as ready for review March 16, 2026 13:56
@tanmaykm
Copy link
Member Author

tanmaykm commented Mar 16, 2026

@claude review this

mortenpi
mortenpi reviewed Mar 17, 2026
View reviewed changes
Copy link
Member

@mortenpi mortenpi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the timeout value passed through a lot of the functions, but where do we actually get the initial value? It should be consistent with the existing timeouts, right?

tests/proxy_admin_ui_tests/ui_unit_tests/package-lock.json
Copy link
Member

@mortenpi mortenpi Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the package-lock.json changes?

@mortenpi
Copy link
Member

mortenpi commented Mar 17, 2026

I would also suggest contributing this to the upstream.

  • Fix aiohttp compatibility issue with ConnectionTimeoutError/SocketTimeoutError on older versions (not necessary for this PRs purpose, but good to have)

If it's unrelated, then probably better to have it as a separate PR, with its own motivation.

@tanmaykm
Copy link
Member Author

tanmaykm commented Mar 17, 2026

Yes, will remove the ConnectionTimeoutError/SocketTimeoutError and the package-lock changes.

The initial timeout value is retained as default in _timeout = timeout if timeout is not None else 600, line 737 of pass_through_endpoints.py

@mortenpi
Copy link
Member

mortenpi commented Mar 17, 2026

The initial timeout value is retained as default in _timeout = timeout if timeout is not None else 600, line 737 of pass_through_endpoints.py

But we'd like it to use the --request_timeout 1800 (probably, or some other option/config). Is it picking it up?

@tanmaykm
Add configurable timeout for passthrough endpoints
946465d 
- Fix hardcoded 600-second timeout on pass-through endpoints that caused TimeoutError for long-running streaming requests (e.g. Anthropic API calls with extended thinking/tool use)
- Add timeout field to PassThroughGenericEndpoint config, allowing per-endpoint timeout configuration
- Added tests
@tanmaykm tanmaykm force-pushed the tan/timeoutconfig branch from 8f1341e to 946465d Compare March 17, 2026 12:17
@tanmaykm
have Anthropic adapter path respect timeout config
4e54b62 
Makes the Anthropic adapter path respect --request_timeout, litellm_settings.request_timeout, and per-deployment timeout settings.
This matches the pattern used by every other handler in the file (responses API, completion, embedding, etc.
@tanmaykm
Copy link
Member Author

tanmaykm commented Mar 17, 2026

Closing and working on a different fix

@tanmaykm tanmaykm closed this Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mortenpi mortenpi mortenpi left review comments

@lamdor lamdor Awaiting requested review from lamdor

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tanmaykm @mortenpi