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.

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: add generator option proto-plus-dep #1596

Merged
merged 28 commits into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
5f5f581
feat: add generator option proto-plus-dep
parthea Feb 14, 2023
855e664
fix mypy
parthea Feb 28, 2023
9711dd6
fix style
parthea Mar 1, 2023
091356b
update goldens
parthea Mar 1, 2023
51a540b
Merge branch 'main' into fix-proto-plus-dependencies-bazel
parthea Mar 1, 2023
68e39cb
fix tests
parthea Mar 1, 2023
de80fc9
clean up
parthea Mar 1, 2023
15d3563
clean up
parthea Mar 1, 2023
4c10b61
add support for dependency google-cloud-kms
parthea Mar 2, 2023
1e8eb7f
Address review comments
parthea Mar 6, 2023
4012b67
add missing file
parthea Mar 6, 2023
da88d33
Merge branch 'main' into fix-proto-plus-dependencies-bazel
parthea Mar 8, 2023
31f5ffd
run formatting tool
parthea Mar 8, 2023
19cfea6
style
parthea Mar 8, 2023
ea60dc7
address review comments
parthea Mar 8, 2023
4b6b7f5
formatting
parthea Mar 8, 2023
be2dee9
address review feedback
parthea Mar 9, 2023
bd3a4da
style
parthea Mar 9, 2023
8f44f05
Merge branch 'main' into fix-proto-plus-dependencies-bazel
parthea Mar 14, 2023
7286e8a
add pypi package google-geo-type
parthea Mar 14, 2023
2dfda6c
bump google-cloud-documentai dependency to 2.0.0
parthea Mar 14, 2023
fa9e9b8
Merge branch 'main' into fix-proto-plus-dependencies-bazel
parthea Mar 14, 2023
346e291
Merge branch 'main' into fix-proto-plus-dependencies-bazel
parthea Mar 15, 2023
99a1ac8
address review comment
parthea Mar 17, 2023
47a2067
add docstring
parthea Mar 17, 2023
3f95c0a
address review comment
parthea Mar 17, 2023
dfe77d7
fix docs
parthea Mar 17, 2023
7a0198d
Merge branch 'main' into fix-proto-plus-dependencies-bazel
parthea Mar 17, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -322,10 +322,10 @@ class {{service.name}}RestTransport({{service.name}}Transport):
{% endfor %}{# rule in method.http_options #}
]
request, metadata = self._interceptor.pre_{{ method.name|snake_case }}(request, metadata)
{% if method.input.ident.is_external_type %}
pb_request = request
{% else %}
{% if method.input.ident.is_proto_plus_type %}
pb_request = {{method.input.ident}}.pb(request)
{% else %}
pb_request = request
{% endif %}
transcoded_request = path_template.transcode(http_options, pb_request)

Expand Down Expand Up @@ -384,10 +384,10 @@ class {{service.name}}RestTransport({{service.name}}Transport):
resp = rest_streaming.ResponseIterator(response, {{method.output.ident}})
{% else %}
resp = {{method.output.ident}}()
{% if method.output.ident.is_external_type %}
pb_resp = resp
{% else %}
{% if method.output.ident.is_proto_plus_type %}
pb_resp = {{method.output.ident}}.pb(resp)
{% else %}
pb_resp = resp
{% endif %}

json_format.Parse(response.content, pb_resp, ignore_unknown_fields=True)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1118,10 +1118,10 @@ def test_{{ method_name }}_rest_required_fields(request_type={{ method.input.ide
{% endif %}{# default is str #}
{% endfor %}
request = request_type(**request_init)
{% if method.input.ident.is_external_type %}
pb_request = request
{% else %}
{% if method.input.ident.is_proto_plus_type %}
pb_request = request_type.pb(request)
{% else %}
pb_request = request
{% endif %}
jsonified_request = json.loads(json_format.MessageToJson(
pb_request,
Expand Down Expand Up @@ -1189,10 +1189,10 @@ def test_{{ method_name }}_rest_required_fields(request_type={{ method.input.ide
with mock.patch.object(path_template, 'transcode') as transcode:
# A uri without fields and an empty body will force all the
# request fields to show up in the query_params.
{% if method.input.ident.is_external_type %}
pb_request = request
{% else %}
{% if method.input.ident.is_proto_plus_type %}
pb_request = request_type.pb(request)
{% else %}
pb_request = request
{% endif %}
transcode_result = {
'uri': 'v1/sample_method',
Expand All @@ -1212,10 +1212,10 @@ def test_{{ method_name }}_rest_required_fields(request_type={{ method.input.ide
json_return_value = json_format.MessageToJson(return_value)
{% else %}

{% if method.output.ident.is_external_type %}
pb_return_value = return_value
{% else %}
{% if method.output.ident.is_proto_plus_type %}
pb_return_value = {{ method.output.ident }}.pb(return_value)
{% else %}
pb_return_value = return_value
{% endif %}
json_return_value = json_format.MessageToJson(pb_return_value)
{% endif %}
Expand Down Expand Up @@ -1287,10 +1287,10 @@ def test_{{ method_name }}_rest_interceptors(null_interceptor):
{% if not method.void %}
post.assert_not_called()
{% endif %}
{% if method.input.ident.is_external_type %}
pb_message = {{ method.input.ident }}()
{% else %}
{% if method.input.ident.is_proto_plus_type %}
pb_message = {{ method.input.ident }}.pb({{ method.input.ident }}())
{% else %}
pb_message = {{ method.input.ident }}()
{% endif %}
transcode.return_value = {
"method": "post",
Expand Down
38 changes: 32 additions & 6 deletions gapic/schema/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"""

import dataclasses
import re
from typing import FrozenSet, Tuple, Optional

from google.protobuf import descriptor_pb2
Expand Down Expand Up @@ -89,10 +90,8 @@ def __str__(self) -> str:
if self.module_alias:
module_name = self.module_alias

# This module is from a different proto package
# Most commonly happens for a common proto
# https://pypi.org/project/googleapis-common-protos/
if self.is_external_type:
# Add _pb2 suffix except when it is a proto-plus type
if not self.is_proto_plus_type:
module_name = f'{self.module}_pb2'

# Return the dot-separated Python identifier.
Expand All @@ -103,8 +102,13 @@ def __str__(self) -> str:
return '.'.join(self.parent + (self.name,))

@property
def is_external_type(self):
return not self.proto_package.startswith(self.api_naming.proto_package)
def is_proto_plus_type(self):
parthea marked this conversation as resolved.
Show resolved Hide resolved
if self.proto_package.startswith(self.api_naming.proto_package) or (
parthea marked this conversation as resolved.
Show resolved Hide resolved
hasattr(self.api_naming, "proto_plus_deps")
and self.proto_package in self.api_naming.proto_plus_deps
):
return True
return False

@cached_property
def __cached_string_repr(self):
Expand Down Expand Up @@ -188,6 +192,28 @@ def python_import(self) -> imp.Import:
alias=self.module_alias,
)

if self.is_proto_plus_type:
# We need to change the import statement to use an
# underscore between the module and the version. For example,
# change google.cloud.documentai.v1 to google.cloud.documentai_v1.
# Check if the package name contains a version.
version_regex = "^v\d(alpha|beta)?\d?"
parthea marked this conversation as resolved.
Show resolved Hide resolved
regex_match = re.match(version_regex, self.package[-1])

if regex_match:
parthea marked this conversation as resolved.
Show resolved Hide resolved
versioned_module = f"{self.package[-2]}_{regex_match[0]}"
parthea marked this conversation as resolved.
Show resolved Hide resolved
return imp.Import(
package=self.package[:-2] +
(versioned_module, 'types'),
module=self.module,
alias=self.module_alias,
)
else:
return imp.Import(
package=self.package + ('types',),
module=self.module,
)

# Return the standard import.
return imp.Import(
package=self.package,
Expand Down
6 changes: 6 additions & 0 deletions gapic/schema/naming.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class Naming(abc.ABC):
product_name: str = ''
proto_package: str = ''
_warehouse_package_name: str = ''
proto_plus_deps: Tuple[str, ...] = dataclasses.field(default_factory=tuple)

def __post_init__(self):
if not self.product_name:
Expand Down Expand Up @@ -146,6 +147,11 @@ def build(
package_info = dataclasses.replace(package_info,
_warehouse_package_name=opts.warehouse_package_name
)
if opts.proto_plus_deps:
package_info = dataclasses.replace(
package_info,
proto_plus_deps=opts.proto_plus_deps,
)

# Done; return the naming information.
return package_info
Expand Down
2 changes: 1 addition & 1 deletion gapic/schema/wrappers.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def __hash__(self):
def name(self) -> str:
"""Used to prevent collisions with python keywords"""
name = self.field_pb.name
return name + "_" if name in utils.RESERVED_NAMES and not self.meta.address.is_external_type else name
return name + "_" if name in utils.RESERVED_NAMES and self.meta.address.is_proto_plus_type else name

@utils.cached_property
def ident(self) -> metadata.FieldIdentifier:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,10 +356,10 @@ class {{service.name}}RestTransport({{service.name}}Transport):
{% endfor %}{# rule in method.http_options #}
]
request, metadata = self._interceptor.pre_{{ method.name|snake_case }}(request, metadata)
{% if method.input.ident.is_external_type %}
pb_request = request
{% else %}
{% if method.input.ident.is_proto_plus_type %}
pb_request = {{method.input.ident}}.pb(request)
{% else %}
pb_request = request
{% endif %}
transcoded_request = path_template.transcode(http_options, pb_request)

Expand Down Expand Up @@ -418,10 +418,10 @@ class {{service.name}}RestTransport({{service.name}}Transport):
resp = rest_streaming.ResponseIterator(response, {{method.output.ident}})
{% else %}
resp = {{method.output.ident}}()
{% if method.output.ident.is_external_type %}
pb_resp = resp
{% else %}
{% if method.output.ident.is_proto_plus_type %}
pb_resp = {{method.output.ident}}.pb(resp)
{% else %}
pb_resp = resp
{% endif %}

json_format.Parse(response.content, pb_resp, ignore_unknown_fields=True)
Expand Down
12 changes: 9 additions & 3 deletions gapic/templates/setup.py.j2
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,16 @@ dependencies = [
"protobuf>=3.19.5,<5.0.0dev,!=3.20.0,!=3.20.1,!=4.21.0,!=4.21.1,!=4.21.2,!=4.21.3,!=4.21.4,!=4.21.5",
{# TODO: Remove after https://github.com/googleapis/gapic-generator-python/pull/1240 is merged. #}
{% if api.requires_package(('google', 'iam', 'v1')) or opts.add_iam_methods or api.has_iam_mixin %}
'grpc-google-iam-v1 >= 0.12.4, < 1.0.0dev',
"grpc-google-iam-v1 >= 0.12.4, < 1.0.0dev",
parthea marked this conversation as resolved.
Show resolved Hide resolved
{% endif %}
{% if api.requires_package(('google', 'cloud', 'documentai', 'v1')) %}
'google-cloud-documentai >= 1.2.1, < 3.0.0dev',
{% if api.requires_package(('google', 'cloud', 'documentai', 'v1')) and api.naming.warehouse_package_name != 'google-cloud-documentai' %}
parthea marked this conversation as resolved.
Show resolved Hide resolved
"google-cloud-documentai >= 1.2.1, < 3.0.0dev",
{% endif %}
{% if api.requires_package(('google', 'apps', 'script', 'type')) and api.naming.warehouse_package_name != 'google-apps-script-type' %}
"google-apps-script-type>=0.2.0, <1.0.0dev",
{% endif %}
{% if api.requires_package(('google', 'cloud', 'kms', 'v1')) and api.naming.warehouse_package_name != 'google-cloud-kms' %}
"google-cloud-kms>=2.3.0, <3.0.0dev",
{% endif %}
]
url = "https://github.com/googleapis/python-{{ api.naming.warehouse_package_name|replace("google-cloud-", "")|replace("google-", "") }}"
Expand Down
3 changes: 3 additions & 0 deletions gapic/templates/testing/constraints-3.7.txt.j2
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,6 @@ grpc-google-iam-v1==0.12.4
{% if api.requires_package(('google', 'cloud', 'documentai', 'v1')) %}
google-cloud-documentai==1.2.1
{% endif %}
{% if api.requires_package(('google', 'cloud', 'kms', 'v1')) %}
parthea marked this conversation as resolved.
Show resolved Hide resolved
google-cloud-kms==2.3.0
{% endif %}
Original file line number Diff line number Diff line change
Expand Up @@ -929,10 +929,10 @@ def test_{{ method_name }}_rest(request_type):
{% elif method.lro %}
json_return_value = json_format.MessageToJson(return_value)
{% else %}
{% if method.output.ident.is_external_type %}
pb_return_value = return_value
{% else %}
{% if method.output.ident.is_proto_plus_type %}
pb_return_value = {{ method.output.ident }}.pb(return_value)
{% else %}
pb_return_value = return_value
{% endif %}
json_return_value = json_format.MessageToJson(pb_return_value)
{% endif %}
Expand Down Expand Up @@ -1009,10 +1009,10 @@ def test_{{ method_name }}_rest_required_fields(request_type={{ method.input.ide
{% endif %}{# default is str #}
{% endfor %}
request = request_type(**request_init)
{% if method.input.ident.is_external_type %}
pb_request = request
{% else %}
{% if method.input.ident.is_proto_plus_type %}
pb_request = request_type.pb(request)
{% else %}
pb_request = request
{% endif %}
jsonified_request = json.loads(json_format.MessageToJson(
pb_request,
Expand Down Expand Up @@ -1080,10 +1080,10 @@ def test_{{ method_name }}_rest_required_fields(request_type={{ method.input.ide
with mock.patch.object(path_template, 'transcode') as transcode:
# A uri without fields and an empty body will force all the
# request fields to show up in the query_params.
{% if method.input.ident.is_external_type %}
pb_request = request
{% else %}
{% if method.input.ident.is_proto_plus_type %}
pb_request = request_type.pb(request)
{% else %}
pb_request = request
{% endif %}
transcode_result = {
'uri': 'v1/sample_method',
Expand All @@ -1103,10 +1103,10 @@ def test_{{ method_name }}_rest_required_fields(request_type={{ method.input.ide
json_return_value = json_format.MessageToJson(return_value)
{% else %}

{% if method.output.ident.is_external_type %}
pb_return_value = return_value
{% else %}
{% if method.output.ident.is_proto_plus_type %}
pb_return_value = {{ method.output.ident }}.pb(return_value)
{% else %}
pb_return_value = return_value
{% endif %}
json_return_value = json_format.MessageToJson(pb_return_value)
{% endif %}
Expand Down Expand Up @@ -1178,10 +1178,10 @@ def test_{{ method_name }}_rest_interceptors(null_interceptor):
{% if not method.void %}
post.assert_not_called()
{% endif %}
{% if method.input.ident.is_external_type %}
pb_message = {{ method.input.ident }}()
{% else %}
{% if method.input.ident.is_proto_plus_type %}
pb_message = {{ method.input.ident }}.pb({{ method.input.ident }}())
{% else %}
pb_message = {{ method.input.ident }}()
{% endif %}
transcode.return_value = {
"method": "post",
Expand Down Expand Up @@ -1295,10 +1295,10 @@ def test_{{ method_name }}_rest_flattened():
{% elif method.lro %}
json_return_value = json_format.MessageToJson(return_value)
{% else %}
{% if method.output.ident.is_external_type %}
pb_return_value = return_value
{% else %}
{% if method.output.ident.is_proto_plus_type %}
pb_return_value = {{ method.output.ident }}.pb(return_value)
{% else %}
pb_return_value = return_value
{% endif %}
json_return_value = json_format.MessageToJson(pb_return_value)
{% endif %}
Expand Down
9 changes: 9 additions & 0 deletions gapic/utils/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class Options:
service_yaml_config: Dict[str, Any] = dataclasses.field(
default_factory=dict)
rest_numeric_enums: bool = False
proto_plus_deps: Tuple[str, ...] = dataclasses.field(default=('',))

# Class constants
PYTHON_GAPIC_PREFIX: str = 'python-gapic-'
Expand All @@ -66,6 +67,9 @@ class Options:
'warehouse-package-name', # change the package name on PyPI
# when transport includes "rest", request that response enums be JSON-encoded as numbers
'rest-numeric-enums',
# proto plus dependencies delineated by '+'
# For example, 'google.cloud.api.v1+google.cloud.anotherapi.v2'
'proto-plus-deps',
))

@classmethod
Expand Down Expand Up @@ -161,6 +165,10 @@ def tweak_path(p):
if old_naming:
autogen_snippets = False

proto_plus_deps = tuple(opts.pop('proto-plus-deps', ''))
if len(proto_plus_deps):
proto_plus_deps = tuple(proto_plus_deps[0].split('+'))

answer = Options(
name=opts.pop('name', ['']).pop(),
namespace=tuple(opts.pop('namespace', [])),
Expand All @@ -182,6 +190,7 @@ def tweak_path(p):
transport=opts.pop('transport', ['grpc'])[0].split('+'),
service_yaml_config=service_yaml_config,
rest_numeric_enums=bool(opts.pop('rest-numeric-enums', False)),
proto_plus_deps=proto_plus_deps,
)

# Note: if we ever need to recursively check directories for sample
Expand Down
1 change: 0 additions & 1 deletion tests/integration/goldens/asset/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
"proto-plus >= 1.22.0, <2.0.0dev",
"proto-plus >= 1.22.2, <2.0.0dev; python_version>='3.11'",
"protobuf>=3.19.5,<5.0.0dev,!=3.20.0,!=3.20.1,!=4.21.0,!=4.21.1,!=4.21.2,!=4.21.3,!=4.21.4,!=4.21.5",
'grpc-google-iam-v1 >= 0.12.4, < 1.0.0dev',
]
url = "https://github.com/googleapis/python-asset"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,3 @@
google-api-core
proto-plus
protobuf
grpc-google-iam-v1
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,3 @@
google-api-core
proto-plus
protobuf
grpc-google-iam-v1
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,3 @@
google-api-core
proto-plus
protobuf
grpc-google-iam-v1
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,3 @@
google-api-core==1.34.0
proto-plus==1.22.0
protobuf==3.19.5
grpc-google-iam-v1==0.12.4
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,3 @@
google-api-core
proto-plus
protobuf
grpc-google-iam-v1
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,3 @@
google-api-core
proto-plus
protobuf
grpc-google-iam-v1
Loading