Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Connection management support (with optional grpc_gcp dependency) #5553

Merged
merged 8 commits into from
Jul 27, 2018
Merged

Connection management support (with optional grpc_gcp dependency) #5553

merged 8 commits into from
Jul 27, 2018

Conversation

WeiranFang
Copy link
Contributor

Add optional grpc_gcp dependency for api_core and spanner_v1. If user has installed grpcio-gcp, then the connection management feature will be enabled, and with the configuration specified in spanner.grpc.config . Otherwise, grpc_gcp won't be imported and everything works as usual.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 29, 2018
@theacodes theacodes removed the request for review from tseaver June 29, 2018 15:54
@theacodes theacodes self-assigned this Jun 29, 2018
@tseaver tseaver added api: core api: spanner Issues related to the Spanner API. labels Jun 29, 2018
theacodes
theacodes suggested changes Jun 29, 2018
View reviewed changes
api_core/google/api_core/grpc_helpers.py Outdated
@@ -161,7 +212,7 @@ def create_channel(target, credentials=None, scopes=None, **kwargs):
service. These are only used when credentials are not specified and
are passed to :func:`google.auth.default`.
kwargs: Additional key-word args passed to
:func:`google.auth.transport.grpc.secure_authorized_channel`.
:func:`_create_secure_channel`.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

api_core/google/api_core/grpc_helpers.py Outdated
@@ -149,6 +154,52 @@ def wrap_errors(callable_):
return _wrap_unary_errors(callable_)


def _create_secure_channel(

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

spanner/google/cloud/spanner_v1/gapic/spanner_client.py
options = None

if HAS_GRPC_GCP:
# Initialize grpc gcp config for spanner api.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@ZhouyihaiDing ZhouyihaiDing mentioned this pull request Jun 29, 2018
Closed
@WeiranFang
Copy link
Contributor Author

Hi @theacodes , I have made some changes as requested. Please take a look, thanks!

theacodes
theacodes suggested changes Jul 12, 2018
View reviewed changes
Copy link
Contributor

@theacodes theacodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking almost ready. Sorry for the delay on reviewing.

api_core/google/api_core/grpc_helpers.py
def create_channel(target,
credentials=None,
scopes=None,
ssl_credentials=None,

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

spanner/google/cloud/spanner_v1/gapic/spanner_client.py
options = None

if HAS_GRPC_GCP:
# Initialize grpc gcp config for spanner api.

This comment was marked as spam.

Sign in to view

@WeiranFang
Copy link
Contributor Author

Hi @theacodes , can you take a look at the new changes?

theacodes
theacodes suggested changes Jul 24, 2018
View reviewed changes
Copy link
Contributor

@theacodes theacodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks almost good.

api_core/tests/unit/test_grpc_helpers.py Outdated
@mock.patch(
'google.auth.default',
return_value=(mock.sentinel.credentials, mock.sentinel.projet))
@mock.patch('google.auth.transport.grpc.secure_authorized_channel')
def test_create_channel_implicit(secure_authorized_channel, default):
@mock.patch('grpc._channel.Channel')

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@theacodes theacodes merged commit 55d11b9 into googleapis:master Jul 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theacodes theacodes theacodes approved these changes

Assignees

@theacodes theacodes

Labels
api: core api: spanner Issues related to the Spanner API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@WeiranFang @theacodes @tseaver @googlebot