Skip to content

Upgrade to object_store 0.12.0 RC, tinker with upstreamable version #45

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

Open
H-Plus-Time opened this issue Mar 5, 2025 · 2 comments
Open

Comments

@H-Plus-Time
Copy link
Owner

Depending on whether a full form of this is upstreamable, this could be an opportunity to sunset this repository. Let's see.

@H-Plus-Time
Copy link
Owner Author

Alright, findings so far.

First, showstoppers:

  • RetryExt involves a call to std::time::Instant, which triggers a panic (by design) on wasm32-unknown-unknown (wasi p1 and p2 seem to have valid monotonic clocks, the emscripten target is probably (haven't checked) subject to the same limitation as wasm32-uu). This can't be worked around through the http_connector mechanism, upstream will need a fix.
    • The easy (and genuinely correct) solve: conditional use on web-time as a drop-in replacement for std::time, with a target-dependent dependency on it. Note: this must be in the object_store crate, it cannot be dealt with downstream (not without swapping out the majority of object_store::http).

Strongly recommended for upstream:

  • Since the above showstoppers require a target_arch dependency, pulling in getrandom{features = ["js"]} is much less of a hurdle (already in the dep tree, all its' dependencies are essentially mandatory for wasm32.

1st party HttpConnector for wasm32 targets, upstream:

  • The client method of ClientOptions essentially just needs a bunch (read: almost all) options gated behind target_arch != wasm32 (typical example: browsers don't allow guests to know/control http protocol versions). Given how little is left over (user agent, headers, and finalizing the builder) in the wasm32 target, probably better to swap out the entire function on wasm32 (as opposed to per-block usage of the cfg macro). no_gzip and so on likely require the equivalent Accept-Encoding (caveats apply).
  • impl HttpService for reqwest::Client - swapping out the whole fn call is necessary, and involved more or less identical spawn_local + mpsc::channel + oneshot::channel mediated (for want of a better word) smuggling; it may look ugly, appear slightly unsound, but there's really no way around this (all interactions with JS promises (which includes touching the Fetch API indirectly) are subject to their inherent non-Send, non-Sync nature). Minor tweaks upstream in reqwest would make some of the response manipulations a little neater.

Remaining sticking points:

  • HttpRequestBody::into_request - the PutPayload needs a moderate amount of fiddling (as yet unsolved), but probably not Send/Sync related.

Ongoing role of this crate

The JS bindings (WasmObjectStore, WasmObjectMeta, WasmGetOptions, and the object store adapter methods) don't make a lot of sense to upstream (certainly not in the object_store crate, and there isn't really a suitable workspace slot for it), unless people are eager for it.

The current method adapters (specifically the conversion of rs streams to equivalent JS ReadableStreams) are fairly uncontroversial, but don't amount to much on their own.

The direction I'd like to take this crate in is quite specific to wasm32 environments, specifically the panoply of non-remote object store or filesystems:

  • Actual local filesystems (Origin-Private or non-sandboxed FS access).
  • IndexedDB
  • Cache Storage (shares a number of things in common with Fetch, but must go through the Cache API)
  • The various (flat, somewhat boring) key-value stores.
  • Background Fetch and Sync - probably the single most useful, and difficult to adapt without detailed knowledge

Most of that is irrelevant (and frequently has no equivalent) on other targets.

@kylebarron
Copy link
Contributor

I'd encourage you to write this comment here too apache/arrow-rs#6818

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

No branches or pull requests

2 participants