Skip to content

feat(gax-internal): builder for raw HTTP requests#4755

Merged
coryan merged 6 commits intogoogleapis:mainfrom
coryan:feat-gax-internal-http-requests
Feb 23, 2026
Merged

feat(gax-internal): builder for raw HTTP requests#4755
coryan merged 6 commits intogoogleapis:mainfrom
coryan:feat-gax-internal-http-requests

Conversation

@coryan
Copy link
Collaborator

@coryan coryan commented Feb 21, 2026

Most client libraries use ReqwestClient to make HTTP+JSON RPCs. In the Storage client (and maybe in others) we need to make raw HTTP requests.

These are different in seveal dimensions:

  • The retry loop is typically implemented outside the ReqwestClient, so calling execute() is a problem.
  • The URL may target an endpoint that is different from the ReqwestClient's endpoint, so the host header needs adjustment.
  • The request or response may be streamed instead of parsed from/to a JSON value.

It was getting too hard to keep these things in mind, adding a new type makes it harder to use these incorrectly.

I also refactored some of the tests to remove duplicated code.

Towards #3178

Most client libraries use `ReqwestClient` to make HTTP+JSON RPCs. In the
Storage client (and maybe in others) we need to make raw HTTP requests.

These are different in seveal dimensions:

- The retry loop is typically implemented outside the `ReqwestClient`, so
  calling `execute()` is a problem.
- The URL may target an endpoint that is different from the `ReqwestClient`'s
  endpoint, so the host header needs adjustment.
- The request or response may be streamed instead of parsed from/to a JSON
  value.

It was getting too hard to keep these things in mind, adding a new type makes
it harder to use these incorrectly.

I also refactored some of the tests to remove duplicated code.
@codecov
Copy link

codecov bot commented Feb 21, 2026

Codecov Report

❌ Patch coverage is 98.18182% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.09%. Comparing base (1a18a46) to head (44436bd).
⚠️ Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
src/gax-internal/tests/http_request_builder.rs 95.23% 1 Missing ⚠️
src/gax-internal/tests/mock_credentials.rs 93.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4755      +/-   ##
==========================================
+ Coverage   95.01%   95.09%   +0.07%     
==========================================
  Files         199      204       +5     
  Lines        7764     7865     +101     
==========================================
+ Hits         7377     7479     +102     
+ Misses        387      386       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coryan coryan marked this pull request as ready for review February 21, 2026 02:37
@coryan coryan requested a review from a team as a code owner February 21, 2026 02:37
@coryan coryan marked this pull request as draft February 21, 2026 02:52
@coryan coryan marked this pull request as ready for review February 21, 2026 03:38
Copy link
Collaborator

@PhongChuong PhongChuong left a comment

Choose a reason for hiding this comment

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

PR looks LGTM but I would be more ocnfident if there was a second set of eyes.

@coryan
Copy link
Collaborator Author

coryan commented Feb 23, 2026

@dbolduc do you want to be the second (or third) pair of eyes on this?

Comment on lines +86 to +87
remaining_time: Option<Duration>,
attempt_count: u32,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should be a struct so it can grow.

I know we can always plumb things via "internal" fields in the RequestOptions.... but I don't like that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@coryan coryan enabled auto-merge (squash) February 23, 2026 17:54
@coryan coryan merged commit ca410a7 into googleapis:main Feb 23, 2026
35 checks passed
@coryan coryan deleted the feat-gax-internal-http-requests branch February 23, 2026 18:04
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.

3 participants