Azure / azure-sdk-for-rust

This repository is for active development of the *unofficial* Azure SDK for Rust. This repository is *not* supported by the Azure SDK team.
MIT License
676 stars 231 forks source link

switch to tokio::process #1654

Open conradludgate opened 2 months ago

conradludgate commented 2 months ago

Resolves #1652.

As mentioned in the issue, reqwest already depends on tokio, so this removes a large section of the new dependency tree. tokio::process also uses non-blocking IO for windows, which async-process does not

conradludgate commented 2 months ago

@microsoft-github-policy-service agree company="Neon, Inc."

svix-jplatte commented 1 month ago

For non-wasm (which is the only thing this touches), you are already depending on tokio through reqwest and AFAIK there are really no good alternatives in the Rust ecosystem right now:

arpad-m commented 1 month ago

Yeah there is already an (optional) dependency on tokio through reqwest but most people will probably enable reqwest anyways, so they already pull in tokio. At least that's what we are doing as users of the azure SDK.

The current state pulls in the async-io crate unconditionally, which is IMO worse because that way, a lot of users end up depending on both async-io as well as tokio.

If you really want, it is possible to make the dependency optional by adding a crate-private module that either contains a reexport of tokio's Command struct, or if the tokio dependency is not enabled, contains a struct called Command with all the APIs we use, being a wrapper of std's Command (we cannot reexport std's Command directly because some APIs are async).

heaths commented 1 month ago

We actually do have use cases and budding implementation that will not use tokio but monoio in our feature branch. tokio will still be our default async runtime via reqwest.

That said, this branch will eventually be replaced but community-supported for some time. While no one can say whether the whole community is using tokio, it may be a safe assumption until proven otherwise and then we could implement @arpad-m's idea.

@ctaggart @demoray @jamesbell what do you think? For the stated reasons I'm all for replacing unmaintained async-process as mentioned in #1652. At this time for this branch, I wonder how strict we want to maintain a no-hard-dependency stance on tokio. Do you know of anyone using a different HTTP stack and/or async runtime?