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

Per-request AddCredential not working with BatchRequest #2065

Open
mikequ-taggysoft opened this issue Mar 2, 2022 · 10 comments
Open

Per-request AddCredential not working with BatchRequest #2065

mikequ-taggysoft opened this issue Mar 2, 2022 · 10 comments
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@mikequ-taggysoft
Copy link

mikequ-taggysoft commented Mar 2, 2022

With the per-request credential example provided by @amanda-tarafa in #2064, I was able to get it working for services like Calendar and Oauth2. But I'm having trouble with GmailService.

I use GmailService via BatchRequest, like this

var cred = GoogleCredential.FromAccessToken(myToken);

var batchRequest = new BatchRequest(_gmailService);

foreach (var gmailMessage in gmailMessages)
{
    batchRequest.Queue<SendRequest>
        (_gmailService.Users.Messages.Send(gmailMessage, "me").AddCredential(cred), myCallback));
}

await batchRequest.ExecuteAsync();

I'm getting error:

Error 401, Request is missing required authentication credential. Expected OAuth 2 access token, login cookie or other valid authentication credential. See https://developers.google.com/identity/sign-in/web/devconsole-project.

@mikequ-taggysoft mikequ-taggysoft added priority: p3 Desirable enhancement or fix. May not be included in next release. type: question Request for information or clarification. Not an issue. labels Mar 2, 2022
@amanda-tarafa amanda-tarafa self-assigned this Mar 2, 2022
@mikequ-taggysoft mikequ-taggysoft changed the title Per-request AddCredential not working with GmailService's SendRequest via .Send(message, "me") Per-request AddCredential not working with BatchRequest Mar 2, 2022
@mikequ-taggysoft
Copy link
Author

mikequ-taggysoft commented Mar 2, 2022

Just did a test, using GmailService directly works, rather it's about the BatchRequest.

Now I think of it, BatchRequest must send all requests together via one request, so obviously it cannot use multiple credentials provided by each underlying request. The BatchRequest itself must need a credential somehow.

Previously credential was provided by me on the client level so it worked. Now the client no longer has a credential.

But I do not see an AddCredential method available on BatchRequest, as the class does not inherit ClientServiceRequest

@amanda-tarafa
Copy link
Contributor

Per call credential is not currently supported for batch (by the library) as far as I can see. I'll take a closer look tomorrow and confirm.

I'll also confirm what would be the fasted way to support it, whether to support it at the BatchRequest level or at the individual requests level. Would either work for you?

@amanda-tarafa
Copy link
Contributor

We crossed comments. It can be implemented either way, as individual batched requests may have their own credential. But possibly implementing at the BatchRequest level and maybe failing if the individual requests have a per call credentials is going to be fastest. Would having the credential at the BatchRequest level work for you?

All of this is pending confirmation when I've taken a closer look. In the meantime if you can use non-batch requests as a workaround that would allow your app to work.

@amanda-tarafa amanda-tarafa added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed type: question Request for information or clarification. Not an issue. labels Mar 2, 2022
@mikequ-taggysoft
Copy link
Author

Per call credential is not currently supported for batch (by the library) as far as I can see. I'll take a closer look tomorrow and confirm.

I'll also confirm what would be the fasted way to support it, whether to support it at the BatchRequest level or at the individual requests level. Would either work for you?

You're so fast. I barely finished my follow up comment :)

I'm not familiar on the internals of the Google API batch mechanism. I'd imagine the BatchRequest itself is a single request that has to carry a token of its own? Then once it gets to Google, does Google actually execute each inner request separately on their side? Or is this done from the client side (in which case the batch seems less useful?).

Yes for now, the BatchRequest level AddCredential support would be sufficient for our app, because we only batch requests from a single user at a time. Is multi-user batch even supported? I'm not sure how that'd work.

@amanda-tarafa
Copy link
Contributor

amanda-tarafa commented Mar 4, 2022

All requests in a batch are grouped together by the library and send as part of the content of a single request, the batch request. Headers on individual requests override headers in the batch request and this applies to the Authorization header as well. This applies to most REST Google APIS, and to Gmail in particular:

If you specify a given HTTP header in both the outer request and an individual call, then the individual call header's value overrides the outer batch request header's value. The headers for an individual call apply only to that call.

For example, if you provide an Authorization header for a specific call, then that header applies only to that call. If you provide an Authorization header for the outer request, then that header applies to all of the individual calls unless they override it with Authorization headers of their own.

I still need to take a closer look at the library and which is easier/faster/cleaner to support. I'll report back here when I know more, but just to set expectations, that probably won't be until next week.

@mikequ-taggysoft
Copy link
Author

@amanda-tarafa Thanks for the clarification. That makes sense. Either approach you mentioned would work for our use case. Thanks again!

@amanda-tarafa
Copy link
Contributor

Just an update, this is still something I'll be working on soon, but I haven't yet the change to get to it.

@mikequ-taggysoft
Copy link
Author

@amanda-tarafa By any chance this request can receive some love? :)

@jskeet
Copy link
Collaborator

jskeet commented May 20, 2022

@mikequ-taggysoft: Amanda is currently on vacation, and when she's back there are higher priorities. We haven't forgotten about this and will get to it, but I'm afraid we do need to prioritize work that will affect more customers.

@amanda-tarafa
Copy link
Contributor

@mikequ-taggysoft Yes, this is still on my radar, but as @jskeet said some higher priority work popped up. We'll work on this for sure.

@amanda-tarafa amanda-tarafa added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed priority: p3 Desirable enhancement or fix. May not be included in next release. labels May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

3 participants