- Notifications
You must be signed in to change notification settings - Fork746
Add response wrapper type toTensorzeroHttpClient#5253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
Previously, calling the `send` method would return a`reqwest::Response` and drop our `LimitedClientTicket`.This will incorrectly signal that we now have additionalspace in the pool (even though the HTTP connection is stillneeded to read the response body). It also will interfere withstoring a `Span` to track the total HTTP duration (for the upcoming'tensorzero_overhead' metric)I've introduced a new wrapper type. It's similar to`TensorzeroRequestBuilder` - we hold on to our `LimitedClientTicket`,and expose accessor methods that forward to the underlying type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull request overview
This PR introduces aTensorzeroResponseWrapper type to properly manage HTTP connection pool lifetimes by holding onto aLimitedClientTicket until response bodies are fully consumed. Previously, thesend() method would drop the ticket immediately after receiving headers, incorrectly signaling that space was available in the connection pool while the HTTP connection was still in use for reading the response body.
Key Changes
- Added
TensorzeroResponseWrapperstruct that wrapsreqwest::Responseand extends ticket lifetime - Added
TensorzeroBodyWrapperstruct to preserve tickets when converting tohttp::Response<Body> - Updated all call sites to use the new wrapper type instead of
reqwest::Response
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tensorzero-core/src/http.rs | ImplementsTensorzeroResponseWrapper andTensorzeroBodyWrapper with forwarding methods for common response operations (status(),headers(),text(),json(),bytes(), etc.) |
tensorzero-core/src/providers/helpers.rs | Updates return type ofinject_extra_request_data_and_send fromreqwest::Response toTensorzeroResponseWrapper |
tensorzero-core/src/providers/aws_http_client.rs | Calls newinto_http_response() method to convert wrapper to AWS SDK-compatible response type while preserving ticket |
tensorzero-core/src/client/mod.rs | Updatescheck_http_response method signature to accept and returnTensorzeroResponseWrapper |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Previously, calling the
sendmethod would return areqwest::Responseand drop ourLimitedClientTicket. This will incorrectly signal that we now have additional space in the pool (even though the HTTP connection is still needed to read the response body). It also will interfere with storing aSpanto track the total HTTP duration (for the upcoming 'tensorzero_overhead' metric)I've introduced a new wrapper type. It's similar to
TensorzeroRequestBuilder- we hold on to ourLimitedClientTicket, and expose accessor methods that forward to the underlying type.Important
Introduces
TensorzeroResponseWrapperto manage HTTP responses and retainLimitedClientTicketuntil fully consumed, updating response handling across the codebase.TensorzeroResponseWrapperinhttp.rsto manage HTTP responses and retainLimitedClientTicketuntil fully consumed.sendmethod inTensorzeroRequestBuilderto returnTensorzeroResponseWrapperinstead ofreqwest::Response.check_http_responseinmod.rsto useTensorzeroResponseWrapper.callmethod inaws_http_client.rsto convertTensorzeroResponseWrappertohttp::Response<SdkBody>.inject_extra_request_data_and_sendinhelpers.rsto returnTensorzeroResponseWrapper.TensorzeroBodyWrapperinhttp.rsto manage body consumption and ticket retention.http.rsto verify new response handling behavior.This description was created by
for69a9fe3. You cancustomize this summary. It will automatically update as commits are pushed.