ipfs-rust / ipfs-embed

A small embeddable ipfs implementation
348 stars 49 forks source link

ipfs-embed should be executor agnostic #68

Closed wngr closed 3 years ago

wngr commented 3 years ago

ipfs-embed is a library, which is to be used not standalone, but embedded in bigger applications. It should be the decision of the user, where and how the futures spawned by this library are executed.

The currently used async_global_executor offers the possibility to select the runtime, but uses a global resource to store a handle to said runtime, which can only be initialized once. This is restricting in a sense, such that users can't replace a runtime without restarting the process (users might want to do that in order to implement the Let It Crash pattern within process boundaries).

I see the following possibilities to provide that:

  1. Use a comprehensive crate that abstracts of the executor. I found the following, but none of them immediately fit the bill:
    • agnostik: Seems this is to be replaced, and considered buggy by its authors, see [0]
    • async_executors: this is a bit too opinionated for my case
    • async-spawner: like async_global_executor uses a global resource; not sure about your future intentions with this crate, too
    • ... ?
  2. Provide a very small trait users can implement (like rust-libp2p) does. Returning tasks handle might complicate it, but we could probably come up with a small API surface.

Now I reckon that the public API of ipfs-embed will probably not become nicer by exposing that, but we might guard it behind a feature flag, defaulting to async_global_executor like it is right now.

dvc94ch commented 3 years ago

I gave up on async-spawner and decided on just using async-global-executor in my projects.

dvc94ch commented 3 years ago

Not saying that that decision shouldn't be revisited. But option 2 seems like the most sensible approach. Are you interested in coming up with a PR? Otherwise I can take a look on Monday

wngr commented 3 years ago

Yes, I will give it a try!