-
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
Merged
Merged
Changes from 9 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
d72e2af
feat: let create_channel accept credentials files
busunkim96 21f6fac
test: add unit tests
busunkim96 2190ac5
Merge branch 'master' into creds-and-scope-overrides
busunkim96 2d55a3a
docs: update docstrings
busunkim96 937ea1f
Merge branch 'creds-and-scope-overrides' of github.com:googleapis/pyt…
busunkim96 f332cf3
chore: remove whitespace
busunkim96 b64b6e1
test: fix coverage
busunkim96 a34fab8
chore: delete pytype output
busunkim96 672e74d
chore: fix typo
busunkim96 2e01e23
Merge branch 'master' into creds-and-scope-overrides
busunkim96 6a3a9ac
Update .gitignore
software-dov 537130d
fix: add new exception type
busunkim96 659e787
Merge branch 'creds-and-scope-overrides' of github.com:googleapis/pyt…
busunkim96 a1d6402
docs: update error type in docstring
busunkim96 385a710
docs: fix in async too
busunkim96 cd60b1d
docs: one more docstring
busunkim96 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -317,6 +317,19 @@ def test_create_channel_implicit_with_scopes( | |
grpc_secure_channel.assert_called_once_with(target, composite_creds) | ||
|
||
|
||
def test_create_channel_explicit_with_duplicate_credentials(): | ||
target = "example:443" | ||
|
||
with pytest.raises(ValueError) as excinfo: | ||
grpc_helpers_async.create_channel( | ||
target, | ||
credentials_file="credentials.json", | ||
credentials=mock.sentinel.credentials | ||
) | ||
|
||
assert "mutually exclusive" in str(excinfo.value) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
... |
||
|
||
|
||
@mock.patch("grpc.composite_channel_credentials") | ||
@mock.patch("google.auth.credentials.with_scopes_if_required") | ||
@mock.patch("grpc.experimental.aio.secure_channel") | ||
|
@@ -350,6 +363,49 @@ def test_create_channel_explicit_scoped(grpc_secure_channel, composite_creds_cal | |
grpc_secure_channel.assert_called_once_with(target, composite_creds) | ||
|
||
|
||
@mock.patch("grpc.composite_channel_credentials") | ||
@mock.patch("grpc.experimental.aio.secure_channel") | ||
@mock.patch( | ||
"google.auth.load_credentials_from_file", | ||
return_value=(mock.sentinel.credentials, mock.sentinel.project) | ||
) | ||
def test_create_channnel_with_credentials_file(load_credentials_from_file, grpc_secure_channel, composite_creds_call): | ||
target = "example.com:443" | ||
|
||
credentials_file = "/path/to/credentials/file.json" | ||
composite_creds = composite_creds_call.return_value | ||
|
||
channel = grpc_helpers_async.create_channel( | ||
target, credentials_file=credentials_file | ||
) | ||
|
||
google.auth.load_credentials_from_file.assert_called_once_with(credentials_file, scopes=None) | ||
assert channel is grpc_secure_channel.return_value | ||
grpc_secure_channel.assert_called_once_with(target, composite_creds) | ||
|
||
|
||
@mock.patch("grpc.composite_channel_credentials") | ||
@mock.patch("grpc.experimental.aio.secure_channel") | ||
@mock.patch( | ||
"google.auth.load_credentials_from_file", | ||
return_value=(mock.sentinel.credentials, mock.sentinel.project) | ||
) | ||
def test_create_channel_with_credentials_file_and_scopes(load_credentials_from_file, grpc_secure_channel, composite_creds_call): | ||
target = "example.com:443" | ||
scopes = ["1", "2"] | ||
|
||
credentials_file = "/path/to/credentials/file.json" | ||
composite_creds = composite_creds_call.return_value | ||
|
||
channel = grpc_helpers_async.create_channel( | ||
target, credentials_file=credentials_file, scopes=scopes | ||
) | ||
|
||
google.auth.load_credentials_from_file.assert_called_once_with(credentials_file, scopes=scopes) | ||
assert channel is grpc_secure_channel.return_value | ||
grpc_secure_channel.assert_called_once_with(target, composite_creds) | ||
|
||
|
||
@pytest.mark.skipif(grpc_helpers_async.HAS_GRPC_GCP, reason="grpc_gcp module not available") | ||
@mock.patch("grpc.experimental.aio.secure_channel") | ||
def test_create_channel_without_grpc_gcp(grpc_secure_channel): | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.