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

fix: don't enable snippetgen by default #1078

Merged
merged 2 commits into from
Nov 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 2 additions & 13 deletions gapic/utils/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = True
autogen_snippets: bool = False
templates: Tuple[str, ...] = dataclasses.field(default=('DEFAULT',))
lazy_import: bool = False
old_naming: bool = False
Expand Down Expand Up @@ -132,17 +132,6 @@ 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
if opts.get("old-naming"):
autogen_snippets = False

answer = Options(
name=opts.pop('name', ['']).pop(),
namespace=tuple(opts.pop('namespace', [])),
Expand All @@ -154,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=autogen_snippets,
autogen_snippets=bool(opts.pop("autogen-snippets", False)),
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)),
Expand Down
16 changes: 4 additions & 12 deletions tests/unit/generator/test_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,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
# snippetgen logic tries to parse it.
g = make_generator("autogen-snippets=false")
g = make_generator()
api_schema = api.API.build(
[
descriptor_pb2.FileDescriptorProto(
Expand Down Expand Up @@ -280,7 +277,7 @@ def test_get_response_divides_subpackages():
""".strip()
)
cgr = g.get_response(api_schema=api_schema,
opts=Options.build("autogen-snippets=false"))
opts=Options.build(""))
assert len(cgr.file) == 6
assert {i.name for i in cgr.file} == {
"foo/types/top.py",
Expand Down Expand Up @@ -686,12 +683,7 @@ 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
# snippetgen logic attempts to parse it.
generator = make_generator(
f"samples={config_fpath},autogen-snippets=False")
print(generator)
generator = make_generator(f"samples={config_fpath}")
generator._env.loader = jinja2.DictLoader({"sample.py.j2": ""})
api_schema = make_api(
make_proto(
Expand Down Expand Up @@ -751,7 +743,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("autogen-snippets=False")
api_schema=api_schema, opts=Options.build("")
)
assert actual == expected

Expand Down
23 changes: 3 additions & 20 deletions tests/unit/generator/test_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,9 @@ def test_options_service_config(fs):


def test_options_bool_flags():
# Most options are default False.
# 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)
Expand All @@ -159,22 +161,3 @@ 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