ferristseng / rust-ipfs-api

IPFS HTTP client in Rust
Apache License 2.0
247 stars 68 forks source link

Reqwest support #70

Closed SionoiS closed 2 years ago

SionoiS commented 3 years ago

Copy-pasta everywhere without thinking too much. Doesn't compile!

Next step would be to actually think about how to stream into multipart forms.

edit: this PR would resolve #69 #58

SionoiS commented 3 years ago
pub async fn add_with_options<R>(
        &self,
        add: request::Add<'_>,
        data: R,
    ) -> Result<response::AddResponse, reqwest::Error>
    where
        R: 'static + AsyncRead + Send + Sync,
    {
        //TODO fix allocations

        use reqwest::Body;
        use tokio_util::codec::BytesCodec;

        let url = format!("{}{}", self.base, "/add");

        let stream = FramedRead::new(data, BytesCodec::new());
        let body = Body::wrap_stream(stream);
        let part = reqwest::multipart::Part::stream(body);

        let form = multipart::Form::new().part("path", part);

        let res = self
            .client
            .post(&url)
            .query(&add)
            .multipart(form)
            .send()
            .await?;

        res.json::<response::AddResponse>().await
    }

add_file example works!

Next, I'm trying to master the streaming response flow but it's a big chunk of code.

I kinda understand what need to be done.

  1. Send request
  2. Read response chunk
  3. Deserialize chunk to json
  4. Wrap in stream of Result<Json, Error> And also handle errors but how to do it still escape me...

Staring at your code for couple days should do the trick. :P

SionoiS commented 3 years ago

Changed the layout a little.

Also, not sure if keeping hyper make sense, I could remove hyper but keep TLS options since reqwest has them.

Let me know what you think!

Thanks.

ferristseng commented 3 years ago

I think this mostly looks good at a first glance, and I'd want to keep hyper just to preserve compatibility. I think I'd prefer handling #58 in a separate PR because I'd like to try to keep the APIs consistent for all of the implementations...which would require modifying the multipart crate first.

The feature flags are getting a bit out of hand though and I'd like to also explore ways to organize things to prevent the code from getting difficult to maintain.

ferristseng commented 3 years ago

Can you see if this will work on top of #72? I'm moving to separate crates to preserve my sanity in maintaining this

SionoiS commented 3 years ago

One of the problem is not having reqwest multipart available but otherwise it should work fine.

Don't stress, do the crate split then we can talk about async multipart and then reqwest.

edit: ok so I was finally able to test wasm support and it does not work. So I will also add reqwest-wasm-backend at some point.