fix: remove auth, policy, and options from the reserved names list (#851

) Fixes #835. Breakdown by name that was originally added in #824 - `auth`: `from google import auth` -> `import google.auth` - `credentials`: `from google.auth import credentials` -> `from google.auth import credentials as ga_credentials` - `exceptions`: `from google.api_core import exceptions` -> `from google.api_core import exceptions as core_exceptions` - `future`: skipped, as it is only used in the [generated tests](https://github.com/googleapis/gapic-generator-python/search?q=%22import+future%22) and has a low chance of colliding - `options` `from google.iam.v1 import options_pb2 as options` -> `from google.iam.v1 import options_pb2` - `policy` `from google.iam.v1 import policy_pb2 as policy` -> `from google.iam.v1 import policy_pb2` - `math` skipped as it is only used in [generated tests](https://github.com/googleapis/gapic-generator-python/search?q=%22import+math%22) For `options` and `policy` there is a small change to `gapic/schema/metadata.py` to not alias `_pb2` types
googleapis · Apr 30, 2021 · d3f31a0 · d3f31a0
1 parent 1b77107
commit d3f31a0
Show file tree

Hide file tree

Showing 14 changed files with 280 additions and 276 deletions.
diff --git a/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2 b/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2
@@ -10,10 +10,10 @@ from typing import Callable, Dict, Optional, {% if service.any_server_streaming
 import pkg_resources
 
 from google.api_core import client_options as client_options_lib  # type: ignore
-from google.api_core import exceptions                            # type: ignore
+from google.api_core import exceptions as core_exceptions         # type: ignore
 from google.api_core import gapic_v1                              # type: ignore
 from google.api_core import retry as retries                      # type: ignore
-from google.auth import credentials                               # type: ignore
+from google.auth import credentials as ga_credentials             # type: ignore
 from google.auth.transport import mtls                            # type: ignore
 from google.auth.transport.grpc import SslCredentials             # type: ignore
 from google.auth.exceptions import MutualTLSChannelError          # type: ignore
@@ -174,7 +174,7 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
     {% endfor %} {# common resources #}
 
     def __init__(self, *,
-            credentials: Optional[credentials.Credentials] = None,
+            credentials: Optional[ga_credentials.Credentials] = None,
             transport: Union[str, {{ service.name }}Transport, None] = None,
             client_options: Optional[client_options_lib.ClientOptions] = None,
             client_info: gapic_v1.client_info.ClientInfo = DEFAULT_CLIENT_INFO,

diff --git a/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/transports/base.py.j2 b/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/transports/base.py.j2
@@ -6,13 +6,13 @@ import abc
 import typing
 import pkg_resources
 
-from google import auth
+import google.auth  # type: ignore
 from google.api_core import gapic_v1    # type: ignore
 from google.api_core import retry as retries  # type: ignore
 {% if service.has_lro %}
 from google.api_core import operations_v1  # type: ignore
 {% endif %}
-from google.auth import credentials  # type: ignore
+from google.auth import credentials as ga_credentials  # type: ignore
 
 {% filter sort_lines %}
 {% for method in service.methods.values() %}
@@ -43,7 +43,7 @@ class {{ service.name }}Transport(metaclass=abc.ABCMeta):
     def __init__(
             self, *,
             host: str{% if service.host %} = '{{ service.host }}'{% endif %},
-            credentials: credentials.Credentials = None,
+            credentials: ga_credentials.Credentials = None,
             client_info: gapic_v1.client_info.ClientInfo = DEFAULT_CLIENT_INFO,
             ) -> None:
         """Instantiate the transport.
@@ -70,7 +70,7 @@ class {{ service.name }}Transport(metaclass=abc.ABCMeta):
         # If no credentials are provided, then determine the appropriate
         # defaults.
         if credentials is None:
-            credentials, _ = auth.default(scopes=self.AUTH_SCOPES)
+            credentials, _ = google.auth.default(scopes=self.AUTH_SCOPES)
 
         # Save the credentials.
         self._credentials = credentials

diff --git a/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/transports/grpc.py.j2 b/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/transports/grpc.py.j2
@@ -10,8 +10,8 @@ from google.api_core import grpc_helpers   # type: ignore
 from google.api_core import operations_v1  # type: ignore
 {% endif %}
 from google.api_core import gapic_v1       # type: ignore
-from google import auth                    # type: ignore
-from google.auth import credentials        # type: ignore
+import google.auth                         # type: ignore
+from google.auth import credentials as ga_credentials  # type: ignore
 from google.auth.transport.grpc import SslCredentials  # type: ignore
 
 import grpc  # type: ignore
@@ -39,7 +39,7 @@ class {{ service.name }}GrpcTransport({{ service.name }}Transport):
     """
     def __init__(self, *,
             host: str{% if service.host %} = '{{ service.host }}'{% endif %},
-            credentials: credentials.Credentials = None,
+            credentials: ga_credentials.Credentials = None,
             credentials_file: str = None,
             scopes: Sequence[str] = None,
             channel: grpc.Channel = None,
@@ -105,7 +105,7 @@ class {{ service.name }}GrpcTransport({{ service.name }}Transport):
             host = api_mtls_endpoint if ":" in api_mtls_endpoint else api_mtls_endpoint + ":443"
 
             if credentials is None:
-                credentials, _ = auth.default(scopes=self.AUTH_SCOPES, quota_project_id=quota_project_id)
+                credentials, _ = google.auth.default(scopes=self.AUTH_SCOPES, quota_project_id=quota_project_id)
 
             # Create SSL credentials with client_cert_source or application
             # default SSL credentials.
@@ -135,7 +135,7 @@ class {{ service.name }}GrpcTransport({{ service.name }}Transport):
             host = host if ":" in host else host + ":443"
 
             if credentials is None:
-                credentials, _ = auth.default(scopes=self.AUTH_SCOPES)
+                credentials, _ = google.auth.default(scopes=self.AUTH_SCOPES)
 
             # create a new channel. The provided one is ignored.
             self._grpc_channel = type(self).create_channel(
@@ -162,7 +162,7 @@ class {{ service.name }}GrpcTransport({{ service.name }}Transport):
     @classmethod
     def create_channel(cls,
                        host: str{% if service.host %} = '{{ service.host }}'{% endif %},
-                       credentials: credentials.Credentials = None,
+                       credentials: ga_credentials.Credentials = None,
                        scopes: Optional[Sequence[str]] = None,
                        **kwargs) -> grpc.Channel:
         """Create and return a gRPC channel object.

diff --git a/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 b/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2
@@ -12,8 +12,8 @@ from proto.marshal.rules.dates import DurationRule, TimestampRule
 
 {# Import the service itself as well as every proto module that it imports. -#}
 {% filter sort_lines %}
-from google import auth
-from google.auth import credentials
+import google.auth
+from google.auth import credentials as ga_credentials
 from google.auth.exceptions import MutualTLSChannelError
 from google.oauth2 import service_account
 from {{ (api.naming.module_namespace + (api.naming.versioned_module_name,) + service.meta.address.subpackage)|join(".") }}.services.{{ service.name|snake_case }} import {{ service.client_name }}
@@ -63,7 +63,7 @@ def test__get_default_mtls_endpoint():
 
 
 def test_{{ service.client_name|snake_case }}_from_service_account_info():
-    creds = credentials.AnonymousCredentials()
+    creds = ga_credentials.AnonymousCredentials()
     with mock.patch.object(service_account.Credentials, 'from_service_account_info') as factory:
         factory.return_value = creds
         info = {"valid": True}
@@ -76,7 +76,7 @@ def test_{{ service.client_name|snake_case }}_from_service_account_info():
 
 
 def test_{{ service.client_name|snake_case }}_from_service_account_file():
-    creds = credentials.AnonymousCredentials()
+    creds = ga_credentials.AnonymousCredentials()
     with mock.patch.object(service_account.Credentials, 'from_service_account_file') as factory:
         factory.return_value = creds
         client = {{ service.client_name }}.from_service_account_file("dummy/file/path.json")
@@ -103,7 +103,7 @@ def test_{{ service.client_name|snake_case }}_client_options():
     # Check that if channel is provided we won't create a new one.
     with mock.patch('{{ (api.naming.module_namespace + (api.naming.versioned_module_name,) + service.meta.address.subpackage)|join(".") }}.services.{{ service.name|snake_case }}.{{ service.client_name }}.get_transport_class') as gtc:
         transport = transports.{{ service.name }}GrpcTransport(
-            credentials=credentials.AnonymousCredentials()
+            credentials=ga_credentials.AnonymousCredentials()
         )
         client = {{ service.client_name }}(transport=transport)
         gtc.assert_not_called()
@@ -254,7 +254,7 @@ def test_{{ service.client_name|snake_case }}_client_options_from_dict():
 {% for method in service.methods.values() %}
 def test_{{ method.name|snake_case }}(transport: str = 'grpc', request_type={{ method.input.ident }}):
     client = {{ service.client_name }}(
-        credentials=credentials.AnonymousCredentials(),
+        credentials=ga_credentials.AnonymousCredentials(),
         transport=transport,
     )
 
@@ -340,7 +340,7 @@ def test_{{ method.name|snake_case }}_from_dict():
 {% if method.field_headers and not method.client_streaming %}
 def test_{{ method.name|snake_case }}_field_headers():
     client = {{ service.client_name }}(
-        credentials=credentials.AnonymousCredentials(),
+        credentials=ga_credentials.AnonymousCredentials(),
     )
 
     # Any value that is part of the HTTP/1.1 URI should be sent as
@@ -385,7 +385,7 @@ def test_{{ method.name|snake_case }}_field_headers():
 {% if method.ident.package != method.input.ident.package %}
 def test_{{ method.name|snake_case }}_from_dict():
     client = {{ service.client_name }}(
-        credentials=credentials.AnonymousCredentials(),
+        credentials=ga_credentials.AnonymousCredentials(),
     )
     # Mock the actual call within the gRPC stub, and fake the request.
     with mock.patch.object(
@@ -414,7 +414,7 @@ def test_{{ method.name|snake_case }}_from_dict():
 {% if method.flattened_fields %}
 def test_{{ method.name|snake_case }}_flattened():
     client = {{ service.client_name }}(
-        credentials=credentials.AnonymousCredentials(),
+        credentials=ga_credentials.AnonymousCredentials(),
     )
 
     # Mock the actual call within the gRPC stub, and fake the request.
@@ -461,7 +461,7 @@ def test_{{ method.name|snake_case }}_flattened():
 
 def test_{{ method.name|snake_case }}_flattened_error():
     client = {{ service.client_name }}(
-        credentials=credentials.AnonymousCredentials(),
+        credentials=ga_credentials.AnonymousCredentials(),
     )
 
     # Attempting to call a method with both a request object and flattened
@@ -479,7 +479,7 @@ def test_{{ method.name|snake_case }}_flattened_error():
 {% if method.paged_result_field %}
 def test_{{ method.name|snake_case }}_pager():
     client = {{ service.client_name }}(
-        credentials=credentials.AnonymousCredentials,
+        credentials=ga_credentials.AnonymousCredentials,
     )
 
     # Mock the actual call within the gRPC stub, and fake the request.
@@ -538,7 +538,7 @@ def test_{{ method.name|snake_case }}_pager():
 
 def test_{{ method.name|snake_case }}_pages():
     client = {{ service.client_name }}(
-        credentials=credentials.AnonymousCredentials,
+        credentials=ga_credentials.AnonymousCredentials,
     )
 
     # Mock the actual call within the gRPC stub, and fake the request.
@@ -587,19 +587,19 @@ def test_{{ method.name|snake_case }}_raw_page_lro():
 def test_credentials_transport_error():
     # It is an error to provide credentials and a transport instance.
     transport = transports.{{ service.name }}GrpcTransport(
-        credentials=credentials.AnonymousCredentials(),
+        credentials=ga_credentials.AnonymousCredentials(),
     )
     with pytest.raises(ValueError):
         client = {{ service.client_name }}(
-            credentials=credentials.AnonymousCredentials(),
+            credentials=ga_credentials.AnonymousCredentials(),
             transport=transport,
         )
 
 
 def test_transport_instance():
     # A client may be instantiated with a custom transport instance.
     transport = transports.{{ service.name }}GrpcTransport(
-        credentials=credentials.AnonymousCredentials(),
+        credentials=ga_credentials.AnonymousCredentials(),
     )
     client = {{ service.client_name }}(transport=transport)
     assert client.transport is transport
@@ -608,7 +608,7 @@ def test_transport_instance():
 def test_transport_get_channel():
     # A client may be instantiated with a custom transport instance.
     transport = transports.{{ service.name }}GrpcTransport(
-        credentials=credentials.AnonymousCredentials(),
+        credentials=ga_credentials.AnonymousCredentials(),
     )
     channel = transport.grpc_channel
     assert channel
@@ -617,7 +617,7 @@ def test_transport_get_channel():
 def test_transport_grpc_default():
     # A client should use the gRPC transport by default.
     client = {{ service.client_name }}(
-        credentials=credentials.AnonymousCredentials(),
+        credentials=ga_credentials.AnonymousCredentials(),
     )
     assert isinstance(
         client.transport,
@@ -629,8 +629,8 @@ def test_transport_grpc_default():
 ])
 def test_transport_adc(transport_class):
     # Test default credentials are used if not provided.
-    with mock.patch.object(auth, 'default') as adc:
-        adc.return_value = (credentials.AnonymousCredentials(), None)
+    with mock.patch.object(google.auth, 'default') as adc:
+        adc.return_value = (ga_credentials.AnonymousCredentials(), None)
         transport_class()
         adc.assert_called_once()
 
@@ -640,7 +640,7 @@ def test_{{ service.name|snake_case }}_base_transport():
     with mock.patch('{{ (api.naming.module_namespace + (api.naming.versioned_module_name,) + service.meta.address.subpackage)|join(".") }}.services.{{ service.name|snake_case }}.transports.{{ service.name }}Transport.__init__') as Transport:
         Transport.return_value = None
         transport = transports.{{ service.name }}Transport(
-            credentials=credentials.AnonymousCredentials(),
+            credentials=ga_credentials.AnonymousCredentials(),
         )
 
     # Every method on the transport should just blindly
@@ -664,17 +664,17 @@ def test_{{ service.name|snake_case }}_base_transport():
 
 def test_{{ service.name|snake_case }}_base_transport_with_adc():
     # Test the default credentials are used if credentials and credentials_file are None.
-    with mock.patch.object(auth, 'default') as adc, mock.patch('{{ (api.naming.module_namespace + (api.naming.versioned_module_name,) + service.meta.address.subpackage)|join(".") }}.services.{{ service.name|snake_case }}.transports.{{ service.name }}Transport._prep_wrapped_messages') as Transport:
+    with mock.patch.object(google.auth, 'default') as adc, mock.patch('{{ (api.naming.module_namespace + (api.naming.versioned_module_name,) + service.meta.address.subpackage)|join(".") }}.services.{{ service.name|snake_case }}.transports.{{ service.name }}Transport._prep_wrapped_messages') as Transport:
         Transport.return_value = None
-        adc.return_value = (credentials.AnonymousCredentials(), None)
+        adc.return_value = (ga_credentials.AnonymousCredentials(), None)
         transport = transports.{{ service.name }}Transport()
         adc.assert_called_once()
 
 
 def test_{{ service.name|snake_case }}_auth_adc():
     # If no credentials are provided, we should use ADC credentials.
-    with mock.patch.object(auth, 'default') as adc:
-        adc.return_value = (credentials.AnonymousCredentials(), None)
+    with mock.patch.object(google.auth, 'default') as adc:
+        adc.return_value = (ga_credentials.AnonymousCredentials(), None)
         {{ service.client_name }}()
         adc.assert_called_once_with(scopes=(
             {% for scope in service.oauth_scopes %}
@@ -686,8 +686,8 @@ def test_{{ service.name|snake_case }}_auth_adc():
 def test_{{ service.name|snake_case }}_transport_auth_adc():
     # If credentials and host are not provided, the transport class should use
     # ADC credentials.
-    with mock.patch.object(auth, 'default') as adc:
-        adc.return_value = (credentials.AnonymousCredentials(), None)
+    with mock.patch.object(google.auth, 'default') as adc:
+        adc.return_value = (ga_credentials.AnonymousCredentials(), None)
         transports.{{ service.name }}GrpcTransport(host="squid.clam.whelk")
         adc.assert_called_once_with(scopes=(
             {% for scope in service.oauth_scopes %}
@@ -699,7 +699,7 @@ def test_{{ service.name|snake_case }}_transport_auth_adc():
 def test_{{ service.name|snake_case }}_host_no_port():
     {% with host = (service.host|default('localhost', true)).split(':')[0] %}
     client = {{ service.client_name }}(
-        credentials=credentials.AnonymousCredentials(),
+        credentials=ga_credentials.AnonymousCredentials(),
         client_options=client_options.ClientOptions(api_endpoint='{{ host }}'),
     )
     assert client.transport._host == '{{ host }}:443'
@@ -709,7 +709,7 @@ def test_{{ service.name|snake_case }}_host_no_port():
 def test_{{ service.name|snake_case }}_host_with_port():
     {% with host = (service.host|default('localhost', true)).split(':')[0] %}
     client = {{ service.client_name }}(
-        credentials=credentials.AnonymousCredentials(),
+        credentials=ga_credentials.AnonymousCredentials(),
         client_options=client_options.ClientOptions(api_endpoint='{{ host }}:8000'),
     )
     assert client.transport._host == '{{ host }}:8000'
@@ -741,9 +741,9 @@ def test_{{ service.name|snake_case }}_transport_channel_mtls_with_client_cert_s
             mock_grpc_channel = mock.Mock()
             grpc_create_channel.return_value = mock_grpc_channel
 
-            cred = credentials.AnonymousCredentials()
+            cred = ga_credentials.AnonymousCredentials()
             with pytest.warns(DeprecationWarning):
-                with mock.patch.object(auth, 'default') as adc:
+                with mock.patch.object(google.auth, 'default') as adc:
                     adc.return_value = (cred, None)
                     transport = transport_class(
                         host="squid.clam.whelk",
@@ -820,7 +820,7 @@ def test_{{ service.name|snake_case }}_transport_channel_mtls_with_adc(
 {% if service.has_lro %}
 def test_{{ service.name|snake_case }}_grpc_lro_client():
     client = {{ service.client_name }}(
-        credentials=credentials.AnonymousCredentials(),
+        credentials=ga_credentials.AnonymousCredentials(),
         transport='grpc',
     )
     transport = client.transport
@@ -890,15 +890,15 @@ def test_client_withDEFAULT_CLIENT_INFO():
 
     with mock.patch.object(transports.{{ service.name }}Transport, '_prep_wrapped_messages') as prep:
         client = {{ service.client_name }}(
-            credentials=credentials.AnonymousCredentials(),
+            credentials=ga_credentials.AnonymousCredentials(),
             client_info=client_info,
         )
         prep.assert_called_once_with(client_info)
 
     with mock.patch.object(transports.{{ service.name }}Transport, '_prep_wrapped_messages') as prep:
         transport_class = {{ service.client_name }}.get_transport_class()
         transport = transport_class(
-            credentials=credentials.AnonymousCredentials(),
+            credentials=ga_credentials.AnonymousCredentials(),
             client_info=client_info,
         )
         prep.assert_called_once_with(client_info)

diff --git a/gapic/schema/metadata.py b/gapic/schema/metadata.py
@@ -74,9 +74,16 @@ def __str__(self) -> str:
         """
         # Most (but not all) types are in a module.
         if self.module:
+            module_name = self.module
+
+            # This module is from a different proto package
+            # Most commonly happens for a common proto
+            # https://pypi.org/project/googleapis-common-protos/
+            if not self.proto_package.startswith(self.api_naming.proto_package):
+                module_name = f'{self.module}_pb2'
+
             # If collisions are registered and conflict with our module,
             # use the module alias instead.
-            module_name = self.module
             if self.module_alias:
                 module_name = self.module_alias
 
@@ -170,7 +177,6 @@ def python_import(self) -> imp.Import:
         return imp.Import(
             package=self.package,
             module=f'{self.module}_pb2',
-            alias=self.module_alias if self.module_alias else self.module,
         )
 
     @property