From 8c19609d6244930bd91fd5f40ef9b5b65584c4a5 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Thu, 1 Sep 2022 16:22:35 -0400 Subject: [PATCH] fix: restore support for grpcio-gcp (#418) docs: add a note that grpcio-gcp is only supported in environments with protobuf < 4.x.x docs: raise DeprecationWarning when 'grpcio-gcp' is used fix(deps): require protobuf >= 3.20.1 --- .github/sync-repo-settings.yaml | 4 ++ .github/workflows/unittest.yml | 2 +- google/api_core/grpc_helpers.py | 28 ++++++++++- noxfile.py | 15 ++++++ setup.py | 4 +- testing/constraints-3.7.txt | 3 +- testing/constraints-3.8.txt | 2 + tests/unit/test_grpc_helpers.py | 88 ++++++++++++++++++++++++++++----- 8 files changed, 129 insertions(+), 17 deletions(-) diff --git a/.github/sync-repo-settings.yaml b/.github/sync-repo-settings.yaml index e2d70f90..6d2c2a0e 100644 --- a/.github/sync-repo-settings.yaml +++ b/.github/sync-repo-settings.yaml @@ -11,6 +11,10 @@ branchProtectionRules: # No Kokoro: the following are Github actions - 'lint' - 'mypy' + - 'unit_grpc_gcp-3.7' + - 'unit_grpc_gcp-3.8' + - 'unit_grpc_gcp-3.9' + - 'unit_grpc_gcp-3.10' - 'unit-3.7' - 'unit-3.8' - 'unit-3.9' diff --git a/.github/workflows/unittest.yml b/.github/workflows/unittest.yml index c3027cee..cd1d4d60 100644 --- a/.github/workflows/unittest.yml +++ b/.github/workflows/unittest.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - option: ["", "_wo_grpc"] + option: ["", "_grpc_gcp", "_wo_grpc"] python: - "3.7" - "3.8" diff --git a/google/api_core/grpc_helpers.py b/google/api_core/grpc_helpers.py index 47d27726..bf04ae4c 100644 --- a/google/api_core/grpc_helpers.py +++ b/google/api_core/grpc_helpers.py @@ -16,6 +16,7 @@ import collections import functools +import warnings import grpc @@ -24,6 +25,27 @@ import google.auth.credentials import google.auth.transport.grpc import google.auth.transport.requests +import google.protobuf + +PROTOBUF_VERSION = google.protobuf.__version__ + +# The grpcio-gcp package only has support for protobuf < 4 +if PROTOBUF_VERSION[0:2] == "3.": + try: + import grpc_gcp + + warnings.warn( + """Support for grpcio-gcp is deprecated. This feature will be + removed from `google-api-core` after January 1, 2024. If you need to + continue to use this feature, please pin to a specific version of + `google-api-core`.""", + DeprecationWarning, + ) + HAS_GRPC_GCP = True + except ImportError: + HAS_GRPC_GCP = False +else: + HAS_GRPC_GCP = False # The list of gRPC Callable interfaces that return iterators. @@ -275,7 +297,9 @@ def create_channel( default_scopes (Sequence[str]): Default scopes passed by a Google client library. Use 'scopes' for user-defined scopes. default_host (str): The default endpoint. e.g., "pubsub.googleapis.com". - kwargs: Additional key-word args passed to :func:`grpc.secure_channel`. + kwargs: Additional key-word args passed to + :func:`grpc_gcp.secure_channel` or :func:`grpc.secure_channel`. + Note: `grpc_gcp` is only supported in environments with protobuf < 4.0.0. Returns: grpc.Channel: The created channel. @@ -294,6 +318,8 @@ def create_channel( default_host=default_host, ) + if HAS_GRPC_GCP: + return grpc_gcp.secure_channel(target, composite_credentials, **kwargs) return grpc.secure_channel(target, composite_credentials, **kwargs) diff --git a/noxfile.py b/noxfile.py index d0db9914..2d8f1e02 100644 --- a/noxfile.py +++ b/noxfile.py @@ -32,6 +32,7 @@ # 'docfx' is excluded since it only needs to run in 'docs-presubmit' nox.options.sessions = [ "unit", + "unit_grpc_gcp", "unit_wo_grpc", "cover", "pytype", @@ -142,6 +143,20 @@ def unit(session): default(session) +@nox.session(python=["3.6", "3.7", "3.8", "3.9", "3.10"]) +def unit_grpc_gcp(session): + """Run the unit test suite with grpcio-gcp installed.""" + constraints_path = str( + CURRENT_DIRECTORY / "testing" / f"constraints-{session.python}.txt" + ) + # Install grpcio-gcp + session.install("-e", ".[grpcgcp]", "-c", constraints_path) + # Install protobuf < 4.0.0 + session.install("protobuf<4.0.0") + + default(session) + + @nox.session(python=["3.8", "3.10"]) def unit_wo_grpc(session): """Run the unit test suite w/o grpcio installed""" diff --git a/setup.py b/setup.py index 4919e5f8..2dd2a0cd 100644 --- a/setup.py +++ b/setup.py @@ -30,12 +30,14 @@ release_status = "Development Status :: 5 - Production/Stable" dependencies = [ "googleapis-common-protos >= 1.56.2, < 2.0dev", - "protobuf >= 3.15.0, <5.0.0dev", + "protobuf >= 3.20.1, <5.0.0dev", "google-auth >= 1.25.0, < 3.0dev", "requests >= 2.18.0, < 3.0.0dev", ] extras = { "grpc": ["grpcio >= 1.33.2, < 2.0dev", "grpcio-status >= 1.33.2, < 2.0dev"], + "grpcgcp": "grpcio-gcp >= 0.2.2, < 1.0dev", + "grpcio-gcp": "grpcio-gcp >= 0.2.2, < 1.0dev", } diff --git a/testing/constraints-3.7.txt b/testing/constraints-3.7.txt index c3e6ad74..fe671145 100644 --- a/testing/constraints-3.7.txt +++ b/testing/constraints-3.7.txt @@ -6,9 +6,10 @@ # e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev", # Then this file should have foo==1.14.0 googleapis-common-protos==1.56.2 -protobuf==3.15.0 +protobuf==3.20.1 google-auth==1.25.0 requests==2.18.0 packaging==14.3 grpcio==1.33.2 grpcio-status==1.33.2 +grpcio-gcp==0.2.2 diff --git a/testing/constraints-3.8.txt b/testing/constraints-3.8.txt index e69de29b..8d760bbd 100644 --- a/testing/constraints-3.8.txt +++ b/testing/constraints-3.8.txt @@ -0,0 +1,2 @@ +googleapis-common-protos==1.56.3 +protobuf==4.21.5 \ No newline at end of file diff --git a/tests/unit/test_grpc_helpers.py b/tests/unit/test_grpc_helpers.py index 649072f0..8b9fd9f1 100644 --- a/tests/unit/test_grpc_helpers.py +++ b/tests/unit/test_grpc_helpers.py @@ -365,7 +365,10 @@ def test_create_channel_implicit(grpc_secure_channel, default, composite_creds_c default.assert_called_once_with(scopes=None, default_scopes=None) - grpc_secure_channel.assert_called_once_with(target, composite_creds) + if grpc_helpers.HAS_GRPC_GCP: + grpc_secure_channel.assert_called_once_with(target, composite_creds, None) + else: + grpc_secure_channel.assert_called_once_with(target, composite_creds) @mock.patch("google.auth.transport.grpc.AuthMetadataPlugin", autospec=True) @@ -397,7 +400,10 @@ def test_create_channel_implicit_with_default_host( mock.sentinel.credentials, mock.sentinel.Request, default_host=default_host ) - grpc_secure_channel.assert_called_once_with(target, composite_creds) + if grpc_helpers.HAS_GRPC_GCP: + grpc_secure_channel.assert_called_once_with(target, composite_creds, None) + else: + grpc_secure_channel.assert_called_once_with(target, composite_creds) @mock.patch("grpc.composite_channel_credentials") @@ -420,7 +426,11 @@ def test_create_channel_implicit_with_ssl_creds( composite_creds_call.assert_called_once_with(ssl_creds, mock.ANY) composite_creds = composite_creds_call.return_value - grpc_secure_channel.assert_called_once_with(target, composite_creds) + + if grpc_helpers.HAS_GRPC_GCP: + grpc_secure_channel.assert_called_once_with(target, composite_creds, None) + else: + grpc_secure_channel.assert_called_once_with(target, composite_creds) @mock.patch("grpc.composite_channel_credentials") @@ -442,7 +452,10 @@ def test_create_channel_implicit_with_scopes( default.assert_called_once_with(scopes=["one", "two"], default_scopes=None) - grpc_secure_channel.assert_called_once_with(target, composite_creds) + if grpc_helpers.HAS_GRPC_GCP: + grpc_secure_channel.assert_called_once_with(target, composite_creds, None) + else: + grpc_secure_channel.assert_called_once_with(target, composite_creds) @mock.patch("grpc.composite_channel_credentials") @@ -464,7 +477,10 @@ def test_create_channel_implicit_with_default_scopes( default.assert_called_once_with(scopes=None, default_scopes=["three", "four"]) - grpc_secure_channel.assert_called_once_with(target, composite_creds) + if grpc_helpers.HAS_GRPC_GCP: + grpc_secure_channel.assert_called_once_with(target, composite_creds, None) + else: + grpc_secure_channel.assert_called_once_with(target, composite_creds) def test_create_channel_explicit_with_duplicate_credentials(): @@ -492,7 +508,11 @@ def test_create_channel_explicit(grpc_secure_channel, auth_creds, composite_cred ) assert channel is grpc_secure_channel.return_value - grpc_secure_channel.assert_called_once_with(target, composite_creds) + + if grpc_helpers.HAS_GRPC_GCP: + grpc_secure_channel.assert_called_once_with(target, composite_creds, None) + else: + grpc_secure_channel.assert_called_once_with(target, composite_creds) @mock.patch("grpc.composite_channel_credentials") @@ -512,7 +532,11 @@ def test_create_channel_explicit_scoped(grpc_secure_channel, composite_creds_cal credentials.with_scopes.assert_called_once_with(scopes, default_scopes=None) assert channel is grpc_secure_channel.return_value - grpc_secure_channel.assert_called_once_with(target, composite_creds) + + if grpc_helpers.HAS_GRPC_GCP: + grpc_secure_channel.assert_called_once_with(target, composite_creds, None) + else: + grpc_secure_channel.assert_called_once_with(target, composite_creds) @mock.patch("grpc.composite_channel_credentials") @@ -536,7 +560,11 @@ def test_create_channel_explicit_default_scopes( ) assert channel is grpc_secure_channel.return_value - grpc_secure_channel.assert_called_once_with(target, composite_creds) + + if grpc_helpers.HAS_GRPC_GCP: + grpc_secure_channel.assert_called_once_with(target, composite_creds, None) + else: + grpc_secure_channel.assert_called_once_with(target, composite_creds) @mock.patch("grpc.composite_channel_credentials") @@ -558,7 +586,11 @@ def test_create_channel_explicit_with_quota_project( credentials.with_quota_project.assert_called_once_with("project-foo") assert channel is grpc_secure_channel.return_value - grpc_secure_channel.assert_called_once_with(target, composite_creds) + + if grpc_helpers.HAS_GRPC_GCP: + grpc_secure_channel.assert_called_once_with(target, composite_creds, None) + else: + grpc_secure_channel.assert_called_once_with(target, composite_creds) @mock.patch("grpc.composite_channel_credentials") @@ -583,7 +615,11 @@ def test_create_channel_with_credentials_file( ) assert channel is grpc_secure_channel.return_value - grpc_secure_channel.assert_called_once_with(target, composite_creds) + + if grpc_helpers.HAS_GRPC_GCP: + grpc_secure_channel.assert_called_once_with(target, composite_creds, None) + else: + grpc_secure_channel.assert_called_once_with(target, composite_creds) @mock.patch("grpc.composite_channel_credentials") @@ -611,7 +647,11 @@ def test_create_channel_with_credentials_file_and_scopes( ) assert channel is grpc_secure_channel.return_value - grpc_secure_channel.assert_called_once_with(target, composite_creds) + + if grpc_helpers.HAS_GRPC_GCP: + grpc_secure_channel.assert_called_once_with(target, composite_creds, None) + else: + grpc_secure_channel.assert_called_once_with(target, composite_creds) @mock.patch("grpc.composite_channel_credentials") @@ -639,11 +679,33 @@ def test_create_channel_with_credentials_file_and_default_scopes( ) assert channel is grpc_secure_channel.return_value - grpc_secure_channel.assert_called_once_with(target, composite_creds) + + if grpc_helpers.HAS_GRPC_GCP: + grpc_secure_channel.assert_called_once_with(target, composite_creds, None) + else: + grpc_secure_channel.assert_called_once_with(target, composite_creds) + + +@pytest.mark.skipif( + not grpc_helpers.HAS_GRPC_GCP, reason="grpc_gcp module not available" +) +@mock.patch("grpc_gcp.secure_channel") +def test_create_channel_with_grpc_gcp(grpc_gcp_secure_channel): + target = "example.com:443" + scopes = ["test_scope"] + + credentials = mock.create_autospec(google.auth.credentials.Scoped, instance=True) + credentials.requires_scopes = True + + grpc_helpers.create_channel(target, credentials=credentials, scopes=scopes) + grpc_gcp_secure_channel.assert_called() + + credentials.with_scopes.assert_called_once_with(scopes, default_scopes=None) +@pytest.mark.skipif(grpc_helpers.HAS_GRPC_GCP, reason="grpc_gcp module not available") @mock.patch("grpc.secure_channel") -def test_create_channel(grpc_secure_channel): +def test_create_channel_without_grpc_gcp(grpc_secure_channel): target = "example.com:443" scopes = ["test_scope"]