Skip to content

Commit

Permalink
feat: add flag for gapic metadata (#795)
Browse files Browse the repository at this point in the history
As per the design doc, gapic metadata should not be generated by
default, and should be gated by the 'metadata' flag.

When invoking the generator via protoc, toggle the metadata flag like so:
--python_gapic_opt="metadata"

Subsequent Bazel integration is WIP.
  • Loading branch information
software-dov authored Mar 3, 2021
1 parent fa2fdc0 commit 9cd7664
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 26 deletions.
15 changes: 10 additions & 5 deletions gapic/generator/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,12 @@ def get_response(
template_name, api_schema=api_schema, opts=opts)
)

output_files.update(
self._generate_samples_and_manifest(
api_schema, self._env.get_template(sample_templates[0]),
)
)
sample_output = self._generate_samples_and_manifest(
api_schema,
self._env.get_template(sample_templates[0]),
) if sample_templates else {}

output_files.update(sample_output)

# Return the CodeGeneratorResponse output.
res = CodeGeneratorResponse(
Expand Down Expand Up @@ -233,6 +234,10 @@ def _render_template(
answer: Dict[str, CodeGeneratorResponse.File] = OrderedDict()
skip_subpackages = False

# Very, very special case. This flag exists to gate this one file.
if not opts.metadata and template_name.endswith("gapic_metadata.json.j2"):
return answer

# Sanity check: Rendering per service and per proto would be a
# combinatorial explosion and is almost certainly not what anyone
# ever wants. Error colorfully on it.
Expand Down
9 changes: 6 additions & 3 deletions gapic/utils/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,22 @@ class Options:
lazy_import: bool = False
old_naming: bool = False
add_iam_methods: bool = False
metadata: bool = False
# TODO(yon-mg): should there be an enum for transport type?
transport: List[str] = dataclasses.field(default_factory=lambda: [])

# Class constants
PYTHON_GAPIC_PREFIX: str = 'python-gapic-'
OPT_FLAGS: FrozenSet[str] = frozenset((
'add-iam-methods', # microgenerator implementation for `reroute_to_grpc_interface`
'lazy-import', # requires >= 3.7
'metadata', # generate GAPIC metadata JSON file
'old-naming', # TODO(dovs): Come up with a better comment
'retry-config', # takes a path
'samples', # output dir
'lazy-import', # requires >= 3.7
'add-iam-methods', # microgenerator implementation for `reroute_to_grpc_interface`
# transport type(s) delineated by '+' (i.e. grpc, rest, custom.[something], etc?)
'transport',
'warehouse-package-name' # change the package name on PyPI
'warehouse-package-name', # change the package name on PyPI
))

@classmethod
Expand Down Expand Up @@ -143,6 +145,7 @@ def tweak_path(p):
lazy_import=bool(opts.pop('lazy-import', False)),
old_naming=bool(opts.pop('old-naming', False)),
add_iam_methods=bool(opts.pop('add-iam-methods', False)),
metadata=bool(opts.pop('metadata', False)),
# transport should include desired transports delimited by '+', e.g. transport='grpc+rest'
transport=opts.pop('transport', ['grpc'])[0].split('+')
)
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/generator/test_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,22 @@ def test_get_response_fails_invalid_file_paths():
assert "%proto" in ex_str and "%service" in ex_str


def test_get_response_ignore_gapic_metadata():
g = make_generator()
with mock.patch.object(jinja2.FileSystemLoader, "list_templates") as lt:
lt.return_value = ["gapic/gapic_metadata.json.j2"]
with mock.patch.object(jinja2.Environment, "get_template") as gt:
gt.return_value = jinja2.Template(
"This is not something we want to see")
res = g.get_response(
api_schema=make_api(),
opts=Options.build(""),
)

# We don't expect any files because opts.metadata is not set.
assert res.file == CodeGeneratorResponse().file


def test_get_response_ignores_unwanted_transports_and_clients():
g = make_generator()
with mock.patch.object(jinja2.FileSystemLoader, "list_templates") as lt:
Expand Down
40 changes: 22 additions & 18 deletions tests/unit/generator/test_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import os
import pytest
import re
from unittest import mock
import warnings

Expand Down Expand Up @@ -139,21 +140,24 @@ def test_options_service_config(fs):
assert opts.retry == expected_cfg


def test_options_lazy_import():
opts = Options.build('lazy-import')
assert opts.lazy_import


def test_options_old_naming():
opts = Options.build('old-naming')
assert opts.old_naming


def test_options_add_iam_methods():
opts = Options.build('add-iam-methods')
assert opts.add_iam_methods


def test_options_warehouse_package_name():
opts = Options.build('warehouse-package-name')
assert opts.warehouse_package_name
def test_options_bool_flags():
# All these options are default False.
# If new options violate this assumption,
# this test may need to be tweaked.
# New options should follow the dash-case/snake_case convention.
opt_str_to_attr_name = {
name: re.sub(r"-", "_", name)
for name in
["lazy-import",
"old-naming",
"add-iam-methods",
"metadata",
"warehouse-package-name",
]}

for opt, attr in opt_str_to_attr_name.items():
options = Options.build("")
assert not getattr(options, attr)

options = Options.build(opt)
assert getattr(options, attr)

0 comments on commit 9cd7664

Please sign in to comment.