From 168b4c4977036907ca991d8ac1ac862485d4f09e Mon Sep 17 00:00:00 2001 From: Bu Sun Kim <8822365+busunkim96@users.noreply.github.com> Date: Wed, 27 Oct 2021 13:51:46 -0600 Subject: [PATCH 1/8] feat: generate code snippets by default --- gapic/utils/options.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gapic/utils/options.py b/gapic/utils/options.py index d7bbe2473d..cb2fbc3db6 100644 --- a/gapic/utils/options.py +++ b/gapic/utils/options.py @@ -37,7 +37,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 @@ -143,7 +143,7 @@ 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=bool(opts.pop("autogen-snippets", True)), 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)), From 2bf98224f52e972497030835978e5d9a70bbf615 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Fri, 29 Oct 2021 20:11:25 +0000 Subject: [PATCH 2/8] test: fix broken unit tests --- gapic/utils/options.py | 10 +++++++++- tests/unit/generator/test_generator.py | 15 +++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/gapic/utils/options.py b/gapic/utils/options.py index cb2fbc3db6..4bbeb5e92b 100644 --- a/gapic/utils/options.py +++ b/gapic/utils/options.py @@ -16,6 +16,7 @@ from os import path from typing import Any, DefaultDict, Dict, FrozenSet, List, Optional, Tuple +import distutils.util import dataclasses import json import os @@ -132,6 +133,13 @@ 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` + if opts.get("autogen-snippets"): + autogen_snippets = bool(distutils.util.strtobool(opts.pop("autogen-snippets")[0])) + else: + autogen_snippets = True + answer = Options( name=opts.pop('name', ['']).pop(), namespace=tuple(opts.pop('namespace', [])), @@ -143,7 +151,7 @@ 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", True)), + 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)), diff --git a/tests/unit/generator/test_generator.py b/tests/unit/generator/test_generator.py index d068250e97..5d6dfdbf7b 100644 --- a/tests/unit/generator/test_generator.py +++ b/tests/unit/generator/test_generator.py @@ -242,7 +242,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( @@ -277,7 +280,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", @@ -683,7 +686,11 @@ def test_dont_generate_in_code_samples(mock_gmtime, mock_generate_sample, fs): ), ) - generator = make_generator(f"samples={config_fpath}") + # NOTE: autogen-snippets is intentionally disabled for this test + # The API schema below is incomplete and will result in errors when the + # snippetgen logic attempts to parse it. + generator = make_generator(f"samples={config_fpath},autogen-snippets=False") + print(generator) generator._env.loader = jinja2.DictLoader({"sample.py.j2": ""}) api_schema = make_api( make_proto( @@ -743,7 +750,7 @@ def test_dont_generate_in_code_samples(mock_gmtime, mock_generate_sample, fs): expected.supported_features |= CodeGeneratorResponse.Feature.FEATURE_PROTO3_OPTIONAL actual = generator.get_response( - api_schema=api_schema, opts=Options.build("") + api_schema=api_schema, opts=Options.build("autogen-snippets=False") ) assert actual == expected From 66bf6daa6bdd1e87f9852ec43631fc35010b3ec8 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Fri, 29 Oct 2021 20:22:46 +0000 Subject: [PATCH 3/8] fix: don't use deprecated distutils.util --- gapic/utils/options.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/gapic/utils/options.py b/gapic/utils/options.py index 4bbeb5e92b..203a5ffcd8 100644 --- a/gapic/utils/options.py +++ b/gapic/utils/options.py @@ -16,7 +16,6 @@ from os import path from typing import Any, DefaultDict, Dict, FrozenSet, List, Optional, Tuple -import distutils.util import dataclasses import json import os @@ -135,8 +134,8 @@ def tweak_path(p): # autogen-snippets is True by default, so make sure users can disable # by passing `autogen-snippets=false` - if opts.get("autogen-snippets"): - autogen_snippets = bool(distutils.util.strtobool(opts.pop("autogen-snippets")[0])) + if opts.get("autogen-snippets") and opts.pop("autogen-snippets")[0] in ("False", "false"): + autogen_snippets = False else: autogen_snippets = True From b229fba5a92437f6d661df1211e9f018257598ab Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Fri, 29 Oct 2021 20:33:50 +0000 Subject: [PATCH 4/8] chore: also disable for ads templates --- gapic/utils/options.py | 5 +++++ tests/unit/generator/test_generator.py | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/gapic/utils/options.py b/gapic/utils/options.py index 203a5ffcd8..db2b8508c7 100644 --- a/gapic/utils/options.py +++ b/gapic/utils/options.py @@ -138,6 +138,11 @@ def tweak_path(p): autogen_snippets = False else: autogen_snippets = True + + # NOTE: Snippets are not currently correct for the alternative (Ads) templates + # so always disable snippetgen in that case + if opts.get("old-naming"): + autogen_snippets = False answer = Options( name=opts.pop('name', ['']).pop(), diff --git a/tests/unit/generator/test_generator.py b/tests/unit/generator/test_generator.py index 5d6dfdbf7b..26873b1d33 100644 --- a/tests/unit/generator/test_generator.py +++ b/tests/unit/generator/test_generator.py @@ -243,7 +243,7 @@ def test_get_response_enumerates_proto(): def test_get_response_divides_subpackages(): # NOTE: autogen-snippets is intentionally disabled for this test - # The API schema below is incomplete and will result in errors when the + # 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( @@ -687,9 +687,10 @@ def test_dont_generate_in_code_samples(mock_gmtime, mock_generate_sample, fs): ) # NOTE: autogen-snippets is intentionally disabled for this test - # The API schema below is incomplete and will result in errors when the + # The API schema below is incomplete and will result in errors when the # snippetgen logic attempts to parse it. - generator = make_generator(f"samples={config_fpath},autogen-snippets=False") + generator = make_generator( + f"samples={config_fpath},autogen-snippets=False") print(generator) generator._env.loader = jinja2.DictLoader({"sample.py.j2": ""}) api_schema = make_api( From 77f4b983949bf06d34ce980bc9354b96fe5f7062 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Fri, 29 Oct 2021 20:36:37 +0000 Subject: [PATCH 5/8] chore: format --- gapic/utils/options.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapic/utils/options.py b/gapic/utils/options.py index db2b8508c7..4a024cfb9c 100644 --- a/gapic/utils/options.py +++ b/gapic/utils/options.py @@ -138,7 +138,7 @@ def tweak_path(p): autogen_snippets = False else: autogen_snippets = True - + # NOTE: Snippets are not currently correct for the alternative (Ads) templates # so always disable snippetgen in that case if opts.get("old-naming"): From 5ac664bdc5047c297a3c776852db568a1e003ce6 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Fri, 29 Oct 2021 20:44:23 +0000 Subject: [PATCH 6/8] chore: add link to issue --- gapic/utils/options.py | 1 + 1 file changed, 1 insertion(+) diff --git a/gapic/utils/options.py b/gapic/utils/options.py index 4a024cfb9c..4fbe1d20c9 100644 --- a/gapic/utils/options.py +++ b/gapic/utils/options.py @@ -141,6 +141,7 @@ def tweak_path(p): # 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 if opts.get("old-naming"): autogen_snippets = False From 3466a5cc3b0c5ce0ea6b4e3a9b0ea07163fae860 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Fri, 29 Oct 2021 21:48:17 +0000 Subject: [PATCH 7/8] fix: simplify logic for fetching autogen-snippets status --- gapic/utils/options.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/gapic/utils/options.py b/gapic/utils/options.py index 4fbe1d20c9..ef0316160b 100644 --- a/gapic/utils/options.py +++ b/gapic/utils/options.py @@ -134,10 +134,7 @@ def tweak_path(p): # autogen-snippets is True by default, so make sure users can disable # by passing `autogen-snippets=false` - if opts.get("autogen-snippets") and opts.pop("autogen-snippets")[0] in ("False", "false"): - autogen_snippets = False - else: - autogen_snippets = True + 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 From ac066b5162ea320232a03135243b46c727ec12b7 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Fri, 29 Oct 2021 21:49:22 +0000 Subject: [PATCH 8/8] chore: format --- gapic/utils/options.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gapic/utils/options.py b/gapic/utils/options.py index ef0316160b..d6c692c8fe 100644 --- a/gapic/utils/options.py +++ b/gapic/utils/options.py @@ -134,7 +134,8 @@ def tweak_path(p): # 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") + 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