Expose requests module to library users for _with_options requests like add_with_options (fixes #26)

[sameer]

This will likely need a lot of discussion, since it is changing the way the library works.
There are several ways we could pursue fixing #26:

1. Accept a FnOnce argument that provides additional URL querystring parameters, since the IPFS API is v0 and may change (there are often experimental options people want to use)
2. Accept a request struct that is impl Default in an OPERATION_with_options method
3. Just require users specify all parameters as Option<T>, which is what the API currently does for most requests (since there hasn't been a request with this many non-required arguments added before)

This is style 2, so in a major version update, we should probably switch all variants of style 3 currently in the library to style 2.

[ferristseng]

Thanks! This looks great, but I added some small suggestions.

Also, I played around with some libraries to create builder structs for the request structs.

This one seems like a good option for our use case: https://github.com/idanarye/rust-typed-builder.

Another one was this: https://github.com/colin-kiegel/rust-derive-builder, but the build function it generates returns a Result, which we shouldn't need.

Not totally sure if adding builders is good idea for every request, but I think it is a cleaner API for the most part.

[ferristseng]

Maybe make this take in B: Borrow<request::Add> instead to allow it to take an owned request?

[sameer]

Ok, good idea.

[ferristseng]

I think this can derive Default to save some typing!

[sameer]

Fair point -- I completely forgot that the default for options is none when writing this.

[sameer]

Thanks! This looks great, but I added some small suggestions.

Also, I played around with some libraries to create builder structs for the request structs.

This one seems like a good option for our use case: https://github.com/idanarye/rust-typed-builder.

Another one was this: https://github.com/colin-kiegel/rust-derive-builder, but the build function it generates returns a Result, which we shouldn't need.

Not totally sure if adding builders is good idea for every request, but I think it is a cleaner API for the most part.

The first one looks nice, and I agree builders would be a lot nicer than having to manually mutate from the default.

What are your thoughts on extending this to the other API requests so we have a bunch of builder-style X_with_options?

[ferristseng]

I think it's a good way to deal with options for now, and seems like it will work nicely if any new options are added to the IPFS API.

[sameer]

I think it's a good way to deal with options for now, and seems like it will work nicely if any new options are added to the IPFS API.

Ok, should I expand this PR or postpone that to another one?

[ferristseng]

I think it's a good way to deal with options for now, and seems like it will work nicely if any new options are added to the IPFS API.

Ok, should I expand this PR or postpone that to another one?

Expanding on this one is fine.

[sameer]

Took a look at what work needs to be done and started using the TypedBuilder, it's pretty straightforward but I don't think I'll get around to doing all requests until sometime next week.

[ArdaXi]

Hello! I just came across this crate and would like to use it, but with similar requirements to be able to add parameters with requests. As this PR is quite old at this point, I'd like to ask whether this is still being worked on, or whether I could perhaps help out in providing the desired design. Thanks!

[ferristseng]

@ArdaXi As far as I know, this isn't being worked on. Feel free to take a stab at it! I think whatever design approach you take it should be easily repeatable, since I am aiming to keep this crate very low effort to maintain.

[ArdaXi]

@ferristseng Cool, thanks, I can definitely come up with something!

One thing that bothers me about this though, the easiest way is similar to this original PR where there's a new public struct inside a newly public module that has to be created manually and passed in. It is however the least invasive way of going about it.

A more ergonomic way to me would be to have methods like add_with_options(&self, data: R) which return an AddRequestBuilder that has a reference to the client, and which has a send method that ends up performing the request instead of a build method to yield an AddRequest. That would let you do, say, client.add_with_options(data).pin(false).send().await. This is similar to how reqwest works, for example.

That would mean moving some logic from the IpfsClient into the respective Request files and leave just shims in the client itself, which may or may not make sense.

If you have the time, I'd love to hear your thoughts on this. Either way I'll gladly develop the first design because it seems relatively unobtrusive, and a lot of the work is similar anyway.