-
Notifications
You must be signed in to change notification settings - Fork 72
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
auth: add client_json and client_json_dict #206
Conversation
|
||
if not keyfile_dict and keyfile_json: | ||
# Compensating for missing ServiceAccountCredentials.from_json_keyfile | ||
keyfile_dict = json.loads(keyfile_json) |
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.
Note, from_json_keyfile_name
is naively loading the json too and any errors pass right through. Same here.
client_json_file_path: {{str}} | ||
client_json_dict: {{dict}} | ||
client_json: {{str}} |
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 understand that having all three might seem excessive, but it is really useful and in sync with the flexibility that ServiceAccountCredentials.from_json*
provides.
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.
looks good to me, only minor questions
client_creds_dict
was introduced in experimental mode before (a few days back) and wasn't documented. Renaming it intoclient_json_dict
to be consistent with other options.Introducing
client_json
for convenience to syncronize with oauth2 storage options so that users can use either dict or json and not be forced to use both of them when authenticating.