You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "adriangb (via GitHub)" <gi...@apache.org> on 2026/04/27 20:43:16 UTC

[PR] feat: impl HttpService for reqwest_middleware::ClientWithMiddleware [arrow-rs-object-store]

adriangb opened a new pull request, #702:
URL: https://github.com/apache/arrow-rs-object-store/pull/702

   ## Summary
   
   Mirrors the existing `impl HttpService for reqwest::Client` so consumers who already build a middleware stack on `reqwest_middleware::ClientBuilder` can hand the resulting `ClientWithMiddleware` straight to `HttpClient::new` without writing adapter boilerplate.
   
   The most common use case this unblocks is W3C trace-context propagation via `reqwest_tracing::TracingMiddleware`, but any `reqwest_middleware::Middleware` (retry, caching, conditional, request-id, etc.) composes the same way.
   
   ## Design
   
   - Gated behind a new opt-in `reqwest-middleware` feature (off by default; implies `cloud`).
   - Pinned to `reqwest-middleware = "0.4"` because object_store's `reqwest = "0.12"` is incompatible with `reqwest-middleware = "0.5"` which requires `reqwest 0.13`. Bump together when object_store moves to reqwest 0.13.
   - The impl is non-wasm only: `reqwest-middleware`'s `Middleware` trait requires `Send + Sync` while the wasm `HttpService` plumbing uses `?Send`, and there's no clean way to reuse the channel-based wasm body logic for this convenience layer.
   - `HttpError::reqwest_middleware` is added next to the existing `HttpError::reqwest`. `Reqwest(_)` variants delegate to the existing classification; opaque `Middleware(_)` variants fall back to `HttpErrorKind::Unknown` (downstream middleware authors can construct richer kinds via `HttpError::new` if they want).
   
   ## Usage
   
   \`\`\`rust
   use object_store::client::HttpClient;
   use reqwest_middleware::ClientBuilder;
   use reqwest_tracing::TracingMiddleware;
   
   let inner = reqwest::Client::new();
   let client = ClientBuilder::new(inner)
       .with(TracingMiddleware::default())
       .build();
   let http = HttpClient::new(client);
   \`\`\`
   
   …or hand it via `HttpBuilder::with_http_connector` for the high-level builders.
   
   ## Independence
   
   This PR is one of three independent additions for better HTTP integration seams. The others are:
   
   - `ObjectStoreOperation` extension on outbound requests (per-call operation context for tracing wrappers).
   - `TowerHttpConnector` (general `tower::Service` adapter).
   
   Each PR is self-contained and behind its own feature; they can be reviewed in any order. A tracking issue will follow once all three drafts are CI-green.
   
   ## Test plan
   
   - [x] New unit test \`client_with_middleware_runs_middleware\` mirroring \`spawn.rs\`'s mock-server pattern, confirming middleware mutates the request the server sees
   - [x] \`cargo test --features=aws,azure,gcp,http,reqwest-middleware --lib\` (188 tests pass)
   - [x] \`cargo check --no-default-features --features reqwest-middleware\` (feature isolation)
   - [x] \`cargo fmt --all -- --check\`
   - [x] \`cargo clippy --all-features --lib --tests -- -D warnings\`
   - [ ] CI green (this draft is opened to validate CI)
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org