-
Notifications
You must be signed in to change notification settings - Fork 310
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
feat: Introduce the functionality to override token_uri in credentials #1159
Conversation
@@ -254,6 +254,23 @@ def with_quota_project(self, quota_project_id): | |||
enable_reauth_refresh=self._enable_reauth_refresh, | |||
) | |||
|
|||
@_helpers.copy_docstring(credentials.CredentialsWithTokenUri) |
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.
Why this is needed for the credentials.py? What credentials types it affects?
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.
Python library uses helper methods to override things like quota project, scopes etc. checkout with_quota_project
method in many credentials.
This override method is added to all the credentials that have a token uri.
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.
That is the question - do we need it for every credential type?
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.
Why the auth credetials has it as not implemented exception and here we have a default implementation? It is preferable to have explicit implementation
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.
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.
credentials.py is user credential. It is not a base class for other credentials. Base class is the credentials.py in auth package.
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.
figured out that this is in fact user creds
Please add description for the change |
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.
LGTM
There are scenarios where a custom token endpoint needs to be used. Currently the only way to provide a custom token endpoint is if a credential is created directly from the constructor. For example if a service account is loaded from a file, then custom token endpoint cannot be provided. This change introduces a method that overrides token endpoint after credential creation.