Skip to content

Commit

Permalink
feat: enable snippetgen for default templates (#1171)
Browse files Browse the repository at this point in the history
Enable snippetgen for the default (non-Ads) templates.

This reverts commit 8bdb709 (which was a revert of #1044 and #1055).

I've checked that the changes are OK (don't break generation for any APIs) by creating a [tag](https://github.com/googleapis/gapic-generator-python/commits/v0.62.0b1) and running the [presubmit](https://critique.corp.google.com/cl/424921742).
  • Loading branch information
busunkim96 authored Feb 3, 2022
1 parent 8c191a5 commit c1af051
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 10 deletions.
18 changes: 15 additions & 3 deletions gapic/utils/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class Options:
warehouse_package_name: str = ''
retry: Optional[Dict[str, Any]] = None
sample_configs: Tuple[str, ...] = dataclasses.field(default=())
autogen_snippets: bool = False
autogen_snippets: bool = True
templates: Tuple[str, ...] = dataclasses.field(default=('DEFAULT',))
lazy_import: bool = False
old_naming: bool = False
Expand Down Expand Up @@ -146,6 +146,18 @@ def tweak_path(p):
# Build the options instance.
sample_paths = opts.pop('samples', [])

# autogen-snippets is True by default, so make sure users can disable
# by passing `autogen-snippets=false`
autogen_snippets = opts.pop(
"autogen-snippets", ["True"])[0] in ("True", "true", "T", "t", "TRUE")

# NOTE: Snippets are not currently correct for the alternative (Ads) templates
# so always disable snippetgen in that case
# https://github.com/googleapis/gapic-generator-python/issues/1052
old_naming = bool(opts.pop('old-naming', False))
if old_naming:
autogen_snippets = False

answer = Options(
name=opts.pop('name', ['']).pop(),
namespace=tuple(opts.pop('namespace', [])),
Expand All @@ -157,10 +169,10 @@ def tweak_path(p):
for s in sample_paths
for cfg_path in samplegen_utils.generate_all_sample_fpaths(s)
),
autogen_snippets=bool(opts.pop("autogen-snippets", False)),
autogen_snippets=autogen_snippets,
templates=tuple(path.expanduser(i) for i in templates),
lazy_import=bool(opts.pop('lazy-import', False)),
old_naming=bool(opts.pop('old-naming', False)),
old_naming=old_naming,
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'
Expand Down
11 changes: 7 additions & 4 deletions tests/unit/generator/test_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,10 @@ def test_get_response_enumerates_proto():


def test_get_response_divides_subpackages():
g = make_generator()
# NOTE: autogen-snippets is intentionally disabled for this test
# The API schema below is incomplete and will result in errors when the
# snippetgen logic tries to parse it.
g = make_generator("autogen-snippets=false")
api_schema = api.API.build(
[
descriptor_pb2.FileDescriptorProto(
Expand Down Expand Up @@ -290,7 +293,7 @@ def test_get_response_divides_subpackages():
""".strip()
)
cgr = g.get_response(api_schema=api_schema,
opts=Options.build(""))
opts=Options.build("autogen-snippets=false"))
assert len(cgr.file) == 6
assert {i.name for i in cgr.file} == {
"foo/types/top.py",
Expand Down Expand Up @@ -466,7 +469,7 @@ def test_samplegen_config_to_output_files(mock_gmtime, fs):

with mock.patch("gapic.samplegen.samplegen.generate_sample", side_effect=mock_generate_sample):
actual_response = g.get_response(
api_schema, opts=Options.build(""))
api_schema, opts=Options.build("autogen-snippets=False"))

expected_snippet_index_json = {
"snippets": [
Expand Down Expand Up @@ -606,7 +609,7 @@ def test_samplegen_id_disambiguation(mock_gmtime, fs):
)
with mock.patch("gapic.samplegen.samplegen.generate_sample", side_effect=mock_generate_sample):
actual_response = g.get_response(api_schema,
opts=Options.build(""))
opts=Options.build("autogen-snippets=False"))

expected_snippet_metadata_json = {
"snippets": [
Expand Down
23 changes: 20 additions & 3 deletions tests/unit/generator/test_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,7 @@ def test_options_service_yaml_config(fs):


def test_options_bool_flags():
# All these options are default False.
# If new options violate this assumption,
# this test may need to be tweaked.
# Most options are default False.
# New options should follow the dash-case/snake_case convention.
opt_str_to_attr_name = {
name: re.sub(r"-", "_", name)
Expand All @@ -191,3 +189,22 @@ def test_options_bool_flags():

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

# Check autogen-snippets separately, as it is default True
options = Options.build("")
assert options.autogen_snippets

options = Options.build("autogen-snippets=False")
assert not options.autogen_snippets


def test_options_autogen_snippets_false_for_old_naming():
# NOTE: Snippets are not currently correct for the alternative (Ads) templates
# so always disable snippetgen in that case
# https://github.com/googleapis/gapic-generator-python/issues/1052
options = Options.build("old-naming")
assert not options.autogen_snippets

# Even if autogen-snippets is set to True, do not enable snippetgen
options = Options.build("old-naming,autogen-snippets=True")
assert not options.autogen_snippets

0 comments on commit c1af051

Please sign in to comment.