search

Byron / google-apis-rs

A binding and CLI generator for all Google APIs
http://byron.github.io/google-apis-rs
Other
1.01k stars 131 forks source link

Support HTTP proxies #337

Closed kylegentle closed 2 years ago

kylegentle commented 2 years ago

As written, Hubs require clients to use a hyper_rustls::HttpsConnector:

https://github.com/Byron/google-apis-rs/blob/93de38dddd36ab21eb99f782a3a6dbdc4173220f/src/mako/api/api.rs.mako#L52

Unfortunately, this connector doesn't support HTTP proxies (see related issue).

Can we make Hub::new() more generic, to enable customized connectors like those of hyper_proxy? This would also follow the convention of hyper's own builder.

Byron commented 2 years ago

Thanks for bringing this up! I didn't look into the details yet, but if you think this can be solved by following hypers own conventions, the google crates should certainly follow suit.

PRs are welcome. In case you want to take it on, please only submit changes to src/ without regenerating the APIs. Thank you.

kylegentle commented 2 years ago

Thanks for the quick response, @Byron!

I'm pretty new to Rust, and totally new to Mako, but I'll share what I've got so far. From there, if you can provide some guidance I'd be happy to write a fix, or if you decide it would be more efficient to do it yourself, of course that's great too.

The approach I have in mind is to update Hub's client type to: hyper::Client<tower_service::Service<http::Uri>, hyper::body::Body>

This is more or less what hyper does with the connector parameter of their client builder (code pointer).

Link to the tower_service::Service trait for reference.

So my thinking is that we:

  1. Add tower_service as a dependency in Cargo.toml.
  2. Add use tower_service::Service and use http:Uri to src/rust/api/client.rs.
  3. Update the mako templates to appropriately generate these type annotations in the Hub code. a. Update src/mako/api/api.rs.mako, for the struct members and new function. b. Update the hub type parameters.

3b is the part I feel least equipped to tackle at this point, but I'd love a sanity check on the rest too. I'm sure I don't have a complete picture yet. :smile:

Edit: tower::Service --> tower_service::Service, since that's how hyper_rustls does it.

Byron commented 2 years ago

Thanks a lot for the summary!

It looks like tower_service adds the much needed abstraction layer to not bind so closely to the underlying transport, which sounds like the right approach.

Maybe there should also be a point 4) which updates the CLI code as it will instantiate an API hub as well.

As to how to proceed from here, maybe it helps to share my typical workflow. Usually I use the groups_migration codebase to try things. You could prototype the changes there and fiddle with the types until it works. The same could be done for the CLI of groups_migration for completeness. Once this works, these changes 'just' have to be transferred to the mako source, which usually is quite mechanical. make can be used to repeatedly make groupsmigration1-cargo ARGS=check for validation, along with make groupsmigration1-cli-cargo ARGS=check for the cli related code.

I hope that helps.

kylegentle commented 2 years ago

I discovered that this is also an issue in yup-oauth2; submitted a PR to fix it there as well.