google-apis-rs / generator

A binding and CLI generator for all google APIs
Apache License 2.0
71 stars 14 forks source link

Add reqwest pass through features #18

Closed mwilliammyers closed 4 years ago

Byron commented 5 years ago

Thanks for your contribution! All looks good to me! What do you think, @ggriffiniii ?

ggriffiniii commented 5 years ago

First thanks for the PR. I'm curious what your personal use case is for the features as I could see a few different possibilities here. While adding pass-through features like in this commit, there are still several configuration settings that users may want to provide for a reqwest client that are configured via a Builder at runtime and I would like to provide a mechanism for users to be able to control both.

One possibility would be to provide a Client::with_reqwest function, that accepts a Reqwest::Client provided by the user. The user would then be able to provide a Client with whatever features they choose (both compile time and runtime). This would make cargo compile reqwest with the union of the features specified by the client library and whatever features were enabled by the end user. To allow users to avoid an openssl dependency I would opt the client to use the rustls feature of reqwest by default.

Would the above satisfy your requirements?

ggriffiniii commented 5 years ago

I just committed https://github.com/google-apis-rs/generator/commit/7504e310e8c4c361aca998ea14c309d5c46967cc which implements Client::with_reqwest_client as described above. Let me know if that solves your issue. I'm perfectly willing to reconsider that approach, but it was straightforward to implement so I figured it would be nice to have something more concrete than my description above. Does it satisfy your requirements?

mwilliammyers commented 5 years ago

I am using this in a project that already uses reqwest (with rustls) that has 350+ dependencies and takes ~3 minutes to cold compile (debug) on an 8 core machine with 32gb of RAM and 3-10 seconds to run check (which my editor does via rust-analyzer on save)... so I am just trying to pick low hanging fruit to reduce my dependencies to keep compile time manageable. It is an uphill battle though because not every pruned dependency actually reduces compile time... 😆

That being said, adding Client::with_reqwest is awesome to have regardless because then we can reuse the same client (for connection pooling etc...).

mwilliammyers commented 4 years ago

It would still be handy to turn rustls-tls on and off I think.