-
Notifications
You must be signed in to change notification settings - Fork 521
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
Credential improvements #1077
Credential improvements #1077
Conversation
Create credentials from file path; and allow scopes to be specified in all relevant credential static factory methods.
@jskeet Please check especially for backward compatibility. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely happy with FromFile. Not sure about the scoping parts... maybe see what @SurferJeffAtGoogle thinks?
{ | ||
return defaultCredentialProvider.GetDefaultCredentialAsync(); | ||
} | ||
public static async Task<GoogleCredential> GetApplicationDefaultAsync(IEnumerable<string> scopes) => |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
{ | ||
return defaultCredentialProvider.CreateDefaultCredentialFromStream(stream); | ||
using (var f = File.OpenRead(path)) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the scoping part is useful, as it shows that it's important to think about scopes. Currently it's very easy to miss them completely.
Although I agree that as they're only relevant with Service Credentials, it may be that most users don't ever need to think about them!
{ | ||
return defaultCredentialProvider.GetDefaultCredentialAsync(); | ||
} | ||
public static async Task<GoogleCredential> GetApplicationDefaultAsync(IEnumerable<string> scopes) => |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
{ | ||
return defaultCredentialProvider.CreateDefaultCredentialFromStream(stream); | ||
using (var f = File.OpenRead(path)) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Yes, let's see what @SurferJeffAtGoogle thinks. |
public static GoogleCredential FromStream(Stream stream, string scope1, params string[] scopes) => | ||
FromStream(stream, new[] { scope1.ThrowIfNull(nameof(scope1)) }.Concat(scopes.ThrowIfNull(nameof(scopes)))); | ||
|
||
public static GoogleCredential FromFile(string path, IEnumerable<string> scopes) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
||
public static Task<GoogleCredential> GetApplicationDefaultAsync() => GetApplicationDefaultAsync(Enumerable.Empty<string>()); | ||
|
||
public static Task<GoogleCredential> GetApplicationDefaultAsync(string scope1, params string[] scopes) => |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Create credentials from file path; and allow scopes to be specified in all relevant credential static factory methods.
If this looks OK, I'll add xmldocs.
Helps with googleapis/google-cloud-dotnet#1398