Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rearrange the where clause for generated Client::new() constructors #26

Merged

Conversation

Michael-F-Bryan
Copy link
Contributor

This PR rearranges the where clause on Client::new() to make it work with type inference and impl Trait better.


Clients currently explicitly require that an auth can be turned into a Box<dyn GetAccessToken>, but this doesn't play well with impl Trait and the impl GetAccessToken returned by google_api_auth::yup_oauth2::from_authenticator().

For example, I am trying to use the following code:

let app_secret: ApplicationSecret = args.load_app_secret()?;

let authenticator = DeviceFlowAuthenticator::builder(app_secret)
    .persist_tokens_to_disk(
        PROJECT_DIRS.cache_dir().join("token-cache.json"),
    )
    .build()
    .await
    .context("Unable to create the OAuth2 authenticator")?;


let scopes = vec!["https://www.googleapis.com/auth/cloud-translation"];
let get_access_token =
    google_api_auth::yup_oauth2::from_authenticator(authenticator, scopes);
let client = Client::new(get_access_token);

(original code)

And rustc fails with the unhelpful error message "the trait bound impl google_api_auth::GetAccessToken: std::convert::From<impl google_api_auth::GetAccessToken> is not satisfied"... Which doesn't make sense considering T: From<T>.

error[E0277]: the trait bound `impl google_api_auth::GetAccessToken: std::convert::From<impl google_api_auth::GetAccessToken>` is not satisfied
    --> cli\src\main.rs:27:30
     |
27   |     let client = Client::new(get_access_token);
     |                              ^^^^^^^^^^^^^^^^
     |                              |
     |                              expected an implementor of trait `std::convert::From<impl google_api_auth::GetAccessToken>`
     |                              help: consider borrowing here: `&get_access_token`
     |
    ::: C:\Users\mbryan\.cargo\git\checkouts\generated-f06055b20a4066fd\1f027cd\gen\translate\v3\lib\src\lib.rs:1237:12
     |
1237 |         A: Into<Box<dyn ::google_api_auth::GetAccessToken>>,
     |            ------------------------------------------------ required by this bound in `google_translate3::Client::new`
     |
     = note: required because of the requirements on the impl of `std::convert::From<impl google_api_auth::GetAccessToken>` for `std::boxed::Box<(dyn google_api_auth::GetAccessToken + 'static)>`
     = note: required because of the requirements on the impl of `std::convert::Into<std::boxed::Box<(dyn google_api_auth::GetAccessToken + 'static)>>` for `impl google_api_auth::GetAccessToken`

error: aborting due to previous error

This is linked to #24 in that the user experience for authentication isn't great.

@ggriffiniii ggriffiniii merged commit a835475 into google-apis-rs:master Jun 24, 2020
@Michael-F-Bryan
Copy link
Contributor Author

Thanks @ggriffiniii. Are you able to regenerate the files in google-apis-rs/generated with the new changes?

@Michael-F-Bryan Michael-F-Bryan deleted the client-new-where-clause branch June 25, 2020 05:38
@mwilliammyers
Copy link
Member

@Michael-F-Bryan, I will regenerate them.

@mwilliammyers
Copy link
Member

PS this will change (I think in a breaking way) when we merge in the async branch because yup-oauth has breaking changes in 4.0...

@mwilliammyers
Copy link
Member

@Michael-F-Bryan

I regenerated everything: google-apis-rs/generated@d53ef68

mwilliammyers added a commit that referenced this pull request Mar 14, 2021
* master:
  Repeated method parameters need to be encoded as `field1=value1&field1=value2`
  Fix predecessor link in README
  Rearrange the bounds on Client::new() (#26)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants