Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Open
Aaron1011 wants to merge2 commits intomain
base:main
Choose a base branch
Loading
fromaaron/response-wrapper

Conversation

@Aaron1011
Copy link
Member

@Aaron1011Aaron1011 commentedDec 17, 2025
edited by ellipsis-devbot
Loading

Previously, calling thesend method would return areqwest::Response and 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 aSpan to track the total HTTP duration (for the upcoming 'tensorzero_overhead' metric)

I've introduced a new wrapper type. It's similar toTensorzeroRequestBuilder - we hold on to ourLimitedClientTicket, and expose accessor methods that forward to the underlying type.


Important

IntroducesTensorzeroResponseWrapper to manage HTTP responses and retainLimitedClientTicket until fully consumed, updating response handling across the codebase.

  • Behavior:
    • IntroducesTensorzeroResponseWrapper inhttp.rs to manage HTTP responses and retainLimitedClientTicket until fully consumed.
    • Updatessend method inTensorzeroRequestBuilder to returnTensorzeroResponseWrapper instead ofreqwest::Response.
  • Modifications:
    • Changescheck_http_response inmod.rs to useTensorzeroResponseWrapper.
    • Updatescall method inaws_http_client.rs to convertTensorzeroResponseWrapper tohttp::Response<SdkBody>.
    • Modifiesinject_extra_request_data_and_send inhelpers.rs to returnTensorzeroResponseWrapper.
  • Misc:
    • AddsTensorzeroBodyWrapper inhttp.rs to manage body consumption and ticket retention.
    • Updates tests inhttp.rs to verify new response handling behavior.

This description was created byEllipsis for69a9fe3. You cancustomize this summary. It will automatically update as commits are pushed.

ellipsis-dev[bot] reacted with rocket emoji
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.
Copy link
Contributor

CopilotAI left a 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

  • AddedTensorzeroResponseWrapper struct that wrapsreqwest::Response and extends ticket lifetime
  • AddedTensorzeroBodyWrapper struct to preserve tickets when converting tohttp::Response<Body>
  • Updated all call sites to use the new wrapper type instead ofreqwest::Response

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

FileDescription
tensorzero-core/src/http.rsImplementsTensorzeroResponseWrapper andTensorzeroBodyWrapper with forwarding methods for common response operations (status(),headers(),text(),json(),bytes(), etc.)
tensorzero-core/src/providers/helpers.rsUpdates return type ofinject_extra_request_data_and_send fromreqwest::Response toTensorzeroResponseWrapper
tensorzero-core/src/providers/aws_http_client.rsCalls newinto_http_response() method to convert wrapper to AWS SDK-compatible response type while preserving ticket
tensorzero-core/src/client/mod.rsUpdatescheck_http_response method signature to accept and returnTensorzeroResponseWrapper

virajmehta
virajmehta previously approved these changesDec 17, 2025
@virajmehtavirajmehta added this pull request to themerge queueDec 17, 2025
@Aaron1011Aaron1011 removed this pull request from themerge queue due to a manual requestDec 17, 2025
@Aaron1011Aaron1011 added this pull request to themerge queueDec 17, 2025
@github-merge-queuegithub-merge-queuebot removed this pull request from themerge queue due to failed status checksDec 17, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

Copilot code reviewCopilotCopilot left review comments

@virajmehtavirajmehtavirajmehta approved these changes

Assignees

@virajmehtavirajmehta

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Aaron1011@virajmehta

[8]ページ先頭

©2009-2025 Movatter.jp