-
Notifications
You must be signed in to change notification settings - Fork 88
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: allow credentials files to be passed for channel creation #50
Conversation
…hon-api-core into creds-and-scope-overrides
@@ -218,7 +227,7 @@ def _create_composite_credentials(credentials=None, scopes=None, ssl_credentials | |||
) | |||
|
|||
|
|||
def create_channel(target, credentials=None, scopes=None, ssl_credentials=None, **kwargs): | |||
def create_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 added credentials_file
to the end of the list to avoid breaking anyone who is using positional arguments.
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" in str(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
class DuplicateCredentialArgs(Exception):
pass
...
with pytest.raises(DuplicateCredentialArgs) as exc:
...
…hon-api-core into creds-and-scope-overrides
Allow credentials files to be passed to
create_channel
.For googleapis/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
.