Skip to content

Commit

Permalink
fix: remove auth, policy, and options from the reserved names list (#851
Browse files Browse the repository at this point in the history
)

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
  • Loading branch information
busunkim96 authored Apr 30, 2021
1 parent 1b77107 commit d3f31a0
Show file tree
Hide file tree
Showing 14 changed files with 280 additions and 276 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() %}
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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(
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down Expand Up @@ -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}
Expand All @@ -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")
Expand All @@ -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()
Expand Down Expand Up @@ -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,
)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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()

Expand All @@ -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
Expand All @@ -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 %}
Expand All @@ -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 %}
Expand All @@ -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'
Expand All @@ -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'
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 8 additions & 2 deletions gapic/schema/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit d3f31a0

Please sign in to comment.