Skip to content

refactor(core): route reqsign send via accessor http client#7234

Open
Xuanwo wants to merge 2 commits intomainfrom
xuanwo/reqsign-http-send-via-accessor-http-client
Open

refactor(core): route reqsign send via accessor http client#7234
Xuanwo wants to merge 2 commits intomainfrom
xuanwo/reqsign-http-send-via-accessor-http-client

Conversation

@Xuanwo
Copy link
Member

@Xuanwo Xuanwo commented Mar 2, 2026

Which issue does this PR close?

Closes #.

Rationale for this change

Services that use reqsign currently wire ReqwestHttpSend with a global default reqwest client. This bypasses accessor-level HTTP client injection and makes reqsign traffic inconsistent with OpenDAL's HTTP abstraction.

What changes are included in this PR?

  • Add AccessorInfoHttpSend in opendal-core HTTP utilities.
  • Implement reqsign_core::HttpSend for:
    • HttpClient
    • AccessorInfoHttpSend (delegates to AccessorInfo::http_client() at send time)
  • Migrate reqsign users to this adapter in:
    • s3, gcs, oss, obs, cos, azblob, azdls, azfile
  • Remove reqsign-http-send-reqwest from migrated services.
  • Switch S3 endpoint normalization from reqwest::Url to url::Url, and remove the direct reqwest dependency from opendal-service-s3.
  • Update core/Cargo.lock accordingly.

Validation:

  • cargo fmt --check
  • cargo check --all-features
  • cargo clippy --all-targets --all-features -- -D warnings

Are there any user-facing changes?

Yes.

Services using reqsign now send signing/credential HTTP requests through the accessor's HTTP client configuration, so injected clients are respected consistently. No public API changes.

AI Usage Statement

Implemented with Codex (GPT-5) in the local repository, with human review before PR submission.

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor" labels Mar 2, 2026
@tisonkun tisonkun mentioned this pull request Mar 2, 2026
Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

Follow up a bit at #7235.

Generally LGTM. Thanks!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 2, 2026
@Xuanwo
Copy link
Member Author

Xuanwo commented Mar 2, 2026

Need to address wasm build failure

Signed-off-by: tison <wander4096@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor" size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants