Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default wasm32-unknown-unknown client #7310

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

H-Plus-Time
Copy link

Which issue does this PR close?

Closes #7227 .

What changes are included in this PR?

Target-gated wasm32-unknown-unknown reqwest-based HttpConnector (with explicit handling surrounding non-Send, non-Sync futures).

Are there any user-facing changes?

wasm32-unknown-unknown users will need to add the getrandom = { version = "*", features = ["js"] } cargo entry if they aren't already using it. The alternative would be to feature flag in a similar manner to chrono, getrandom, zstd (i.e. 'js').

…1st party HttpService implementation for wasm32-unknown-unknown.
@github-actions github-actions bot added the object-store Object Store Interface label Mar 19, 2025
@H-Plus-Time
Copy link
Author

We'll need a definitive position from maintainers on which of the following is preferred to resolve the ambiguity surrounding wasm32-unknown-unknown:

  1. Feature flag the code behind "js" + include the getrandom, web-time, wasm-bindgen-futures dependencies behind that same flag.
  2. Forego support for wasm32-unknown-unknown in non-JS embedded contexts, and either:
    a. include the getrandom cargo entry in the target-gated set of dependencies.
    b. document the requirement to add the getrandom cargo entry / point to the wasm entry of the getrandom docs.

@H-Plus-Time
Copy link
Author

Also, two things:

  1. The bump of the url crate (to 2.5.3) is required for wasip2 to compile (with or without the http feature). I'll remove it from this PR given it falls foul of the stated MSRV 1.64 (especially given there's no wasi client implementation anyway). This does mean though that any default wasi http client (wasip1 will likely never work) would need to bump the MSRV to 1.67 (looks like that'll be fine given the discussion in Adopt a MSRV policy #181).
  2. For 3rd party HttpConnector implementations, the ClientOptions struct will need default_headers accessible (in some form, presumably through get_config_value, though that does convert to a string. Perhaps just a pub field) outside this crate.

@alamb
Copy link
Contributor

alamb commented Mar 20, 2025

Thank you for this PR. We are in the process of moving the object_store code to its own repository. Would it be possible for you to create a PR in that repository instead?

(we will handle moving all existing issues to the new repository)

@alamb
Copy link
Contributor

alamb commented Mar 20, 2025

We'll need a definitive position from maintainers on which of the following is preferred to resolve the ambiguity surrounding wasm32-unknown-unknown:

I don't know enough about wasm to have a useful opinion here

Perhaps @kylebarron or @XiangpengHao can help out with an answer

@XiangpengHao
Copy link
Contributor

I think the get_random on wasm-unknown-unknown requires a feature gate and a RUSTFLAG: https://docs.rs/getrandom/latest/getrandom/#webassembly-support.

So it's already quite a hassle to use it on js-based platforms.

That said, I only used wasm-unknown-unknown on the JS platform and do not have enough knowledge to comment on other combinations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
object-store Object Store Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default WASM32 HttpConnector
3 participants