- Notifications
You must be signed in to change notification settings - Fork95
feat: allow credentials files to be passed for channel creation#50
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…hon-api-core into creds-and-scope-overrides
| defcreate_channel(target,credentials=None,scopes=None,ssl_credentials=None,**kwargs): | ||
| defcreate_channel(target,credentials=None,scopes=None,ssl_credentials=None,credentials_file=None,**kwargs): |
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 addedcredentials_file to the end of the list to avoid breaking anyone who is using positional arguments.
software-dov left a comment
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, one style/preference concern with exception raising.
| credentials=mock.sentinel.credentials | ||
| ) | ||
| assert"mutually exclusive"instr(excinfo.value) |
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'm not a huge fan of checking for explicit strings in exception messages. If it's easy and within the style guides of the repo, I like to make one-off exception types and check for the specific exception type within tests, something like
classDuplicateCredentialArgs(Exception):pass...withpytest.raises(DuplicateCredentialArgs)asexc: ...
Uh oh!
There was an error while loading.Please reload this page.
Allow credentials files to be passed to
create_channel.Forgoogleapis/gapic-generator-python#432
While it's possible for the library transport to create a credentials object and pass it to
create_channel, I think it will be easier in the long run for the auth logic to remain in_create_composite_credentials.