Skip to content

Commit ed21464

Browse files
authored
chore: use template excludes in generation config (#2453)
In this PR: - Modify `generate_prerequisite_files` to render `owlbot.py` using template excludes from `GenerationConfig`, rather than hard code the excludes. - Add unit tests to verify `from_yaml` method.
1 parent 33ba423 commit ed21464

20 files changed

+235
-25
lines changed

library_generation/generate_composed_library.py

+3-2
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ def generate_composed_library(
5454
:param library_path: the path to which the generated file goes
5555
:param library: a LibraryConfig object contained inside config, passed here
5656
for convenience and to prevent all libraries to be processed
57-
:param output_folder:
58-
:param versions_file:
57+
:param output_folder: the folder to where tools go
58+
:param versions_file: the file containing version of libraries
5959
:return None
6060
"""
6161
util.pull_api_definition(
@@ -74,6 +74,7 @@ def generate_composed_library(
7474
# owlbot.py) here because transport is parsed from BUILD.bazel,
7575
# which lives in a versioned proto_path.
7676
util.generate_prerequisite_files(
77+
config=config,
7778
library=library,
7879
proto_path=util.remove_version_from(gapic.proto_path),
7980
transport=gapic_inputs.transport,

library_generation/model/generation_config.py

+5-4
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ def __init__(
3131
googleapis_commitish: str,
3232
owlbot_cli_image: str,
3333
synthtool_commitish: str,
34-
template_excludes: str,
34+
template_excludes: List[str],
3535
path_to_yaml: str,
3636
libraries: List[LibraryConfig],
3737
grpc_version: Optional[str] = None,
@@ -48,10 +48,11 @@ def __init__(
4848
self.protobuf_version = protobuf_version
4949

5050

51-
def from_yaml(path_to_yaml: str):
51+
def from_yaml(path_to_yaml: str) -> GenerationConfig:
5252
"""
53-
Parses a yaml located in path_to_yaml. Returns the parsed configuration
54-
represented by the "model" classes
53+
Parses a yaml located in path_to_yaml.
54+
:param path_to_yaml: the path to the configuration file
55+
:return the parsed configuration represented by the "model" classes
5556
"""
5657
with open(path_to_yaml, "r") as file_stream:
5758
config = yaml.safe_load(file_stream)

library_generation/requirements.in

+1
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@ platformdirs==4.1.0
1515
PyYAML==6.0.1
1616
smmap==5.0.1
1717
typing==3.7.4.3
18+
parameterized==0.9.0 # used in parameterized test

library_generation/test/resources/integration/google-cloud-java/generation_config.yaml

-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ protobuf_version: 25.2
33
googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026
44
owlbot_cli_image: sha256:623647ee79ac605858d09e60c1382a716c125fb776f69301b72de1cd35d49409
55
synthtool_commitish: 6612ab8f3afcd5e292aecd647f0fa68812c9f5b5
6-
destination_path: google-cloud-java
76
template_excludes:
87
- ".github/*"
98
- ".kokoro/*"

library_generation/test/resources/integration/java-bigtable/generation_config.yaml

-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ protobuf_version: 25.2
44
googleapis_commitish: 40203ca1880849480bbff7b8715491060bbccdf1
55
owlbot_cli_image: sha256:623647ee79ac605858d09e60c1382a716c125fb776f69301b72de1cd35d49409
66
synthtool_commitish: 6612ab8f3afcd5e292aecd647f0fa68812c9f5b5
7-
destination_path: java-bigtable
87
template_excludes:
98
- ".gitignore"
109
- ".kokoro/presubmit/integration.cfg"
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
gapic_generator_version: 2.34.0
2+
libraries:
3+
- api_shortname: apigeeconnect
4+
GAPICs:
5+
- proto_path: google/cloud/apigeeconnect/v1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
gapic_generator_version: 2.34.0
2+
libraries:
3+
- GAPICs:
4+
- proto_path: google/cloud/apigeeconnect/v1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
gapic_generator_version: 2.34.0
2+
libraries:
3+
random_key:
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
libraries:
2+
- api_shortname: apigeeconnect
3+
name_pretty: Apigee Connect
4+
api_description: "allows the Apigee hybrid management"
5+
product_documentation: "https://cloud.google.com/apigee/docs/hybrid/v1.3/apigee-connect/"
6+
GAPICs:
7+
- proto_path: google/cloud/apigeeconnect/v1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
gapic_generator_version: 2.34.0
2+
libraries:
3+
- api_shortname: apigeeconnect
4+
name_pretty: Apigee Connect
5+
api_description: "allows the Apigee hybrid management"
6+
product_documentation: "https://cloud.google.com/apigee/docs/hybrid/v1.3/apigee-connect/"
7+
GAPICs:
8+
- proto_path: google/cloud/apigeeconnect/v1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
gapic_generator_version: 2.34.0
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
gapic_generator_version: 2.34.0
2+
libraries:
3+
- api_shortname: apigeeconnect
4+
api_description: "allows the Apigee hybrid management"
5+
GAPICs:
6+
- proto_path: google/cloud/apigeeconnect/v1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
gapic_generator_version: 2.34.0
2+
googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026
3+
libraries:
4+
- api_shortname: apigeeconnect
5+
name_pretty: Apigee Connect
6+
api_description: "allows the Apigee hybrid management"
7+
product_documentation: "https://cloud.google.com/apigee/docs/hybrid/v1.3/apigee-connect/"
8+
GAPICs:
9+
- proto_path: google/cloud/apigeeconnect/v1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
gapic_generator_version: 2.34.0
2+
libraries:
3+
- api_shortname: apigeeconnect
4+
name_pretty: Apigee Connect
5+
api_description: "allows the Apigee hybrid management"
6+
GAPICs:
7+
- proto_path: google/cloud/apigeeconnect/v1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
gapic_generator_version: 2.34.0
2+
libraries:
3+
- GAPICs:
4+
- random_key:
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
gapic_generator_version: 2.34.0
2+
googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026
3+
owlbot_cli_image: sha256:623647ee79ac605858d09e60c1382a716c125fb776f69301b72de1cd35d49409
4+
libraries:
5+
- api_shortname: apigeeconnect
6+
name_pretty: Apigee Connect
7+
api_description: "allows the Apigee hybrid management"
8+
product_documentation: "https://cloud.google.com/apigee/docs/hybrid/v1.3/apigee-connect/"
9+
GAPICs:
10+
- proto_path: google/cloud/apigeeconnect/v1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
gapic_generator_version: 2.34.0
2+
googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026
3+
owlbot_cli_image: sha256:623647ee79ac605858d09e60c1382a716c125fb776f69301b72de1cd35d49409
4+
synthtool_commitish: 6612ab8f3afcd5e292aecd647f0fa68812c9f5b5
5+
libraries:
6+
- api_shortname: apigeeconnect
7+
name_pretty: Apigee Connect
8+
api_description: "allows the Apigee hybrid management"
9+
product_documentation: "https://cloud.google.com/apigee/docs/hybrid/v1.3/apigee-connect/"
10+
GAPICs:
11+
- proto_path: google/cloud/apigeeconnect/v1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
gapic_generator_version: 2.34.0
2+
protobuf_version: 25.2
3+
googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026
4+
owlbot_cli_image: sha256:623647ee79ac605858d09e60c1382a716c125fb776f69301b72de1cd35d49409
5+
synthtool_commitish: 6612ab8f3afcd5e292aecd647f0fa68812c9f5b5
6+
template_excludes:
7+
- ".github/*"
8+
- ".kokoro/*"
9+
- "samples/*"
10+
- "CODE_OF_CONDUCT.md"
11+
- "CONTRIBUTING.md"
12+
- "LICENSE"
13+
- "SECURITY.md"
14+
- "java.header"
15+
- "license-checks.xml"
16+
- "renovate.json"
17+
- ".gitignore"
18+
libraries:
19+
- api_shortname: cloudasset
20+
name_pretty: Cloud Asset Inventory
21+
product_documentation: "https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview"
22+
api_description: "provides inventory services based on a time series database."
23+
library_name: "asset"
24+
client_documentation: "https://cloud.google.com/java/docs/reference/google-cloud-asset/latest/overview"
25+
distribution_name: "com.google.cloud:google-cloud-asset"
26+
release_level: "stable"
27+
issue_tracker: "https://issuetracker.google.com/issues/new?component=187210&template=0"
28+
api_reference: "https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview"
29+
GAPICs:
30+
- proto_path: google/cloud/asset/v1
31+
- proto_path: google/cloud/asset/v1p1beta1
32+
- proto_path: google/cloud/asset/v1p2beta1
33+
- proto_path: google/cloud/asset/v1p5beta1
34+
- proto_path: google/cloud/asset/v1p7beta1

library_generation/test/unit_tests.py

+113-3
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,21 @@
2020
import os
2121
import io
2222
import contextlib
23-
import subprocess
2423
from pathlib import Path
2524
from difflib import unified_diff
2625
from typing import List
27-
26+
from parameterized import parameterized
2827
from library_generation import utilities as util
2928
from library_generation.model.gapic_config import GapicConfig
3029
from library_generation.model.generation_config import GenerationConfig
3130
from library_generation.model.gapic_inputs import parse as parse_build_file
31+
from library_generation.model.generation_config import from_yaml
3232
from library_generation.model.library_config import LibraryConfig
3333

3434
script_dir = os.path.dirname(os.path.realpath(__file__))
3535
resources_dir = os.path.join(script_dir, "resources")
3636
build_file = Path(os.path.join(resources_dir, "misc")).resolve()
37+
test_config_dir = Path(os.path.join(resources_dir, "test-config")).resolve()
3738
library_1 = LibraryConfig(
3839
api_shortname="baremetalsolution",
3940
name_pretty="Bare Metal Solution",
@@ -104,6 +105,101 @@ def test_eprint_valid_input_succeeds(self):
104105
# print() appends a `\n` each time it's called
105106
self.assertEqual(test_input + "\n", result)
106107

108+
# parameterized tests need to run from the class, see
109+
# https://github.com/wolever/parameterized/issues/37 for more info.
110+
# This test confirms that a ValueError with an error message about a
111+
# missing key (specified in the first parameter of each `parameterized`
112+
# tuple) when parsing a test configuration yaml (second parameter) will
113+
# be raised.
114+
@parameterized.expand(
115+
[
116+
("libraries", f"{test_config_dir}/config_without_libraries.yaml"),
117+
("GAPICs", f"{test_config_dir}/config_without_gapics.yaml"),
118+
("proto_path", f"{test_config_dir}/config_without_proto_path.yaml"),
119+
("api_shortname", f"{test_config_dir}/config_without_api_shortname.yaml"),
120+
(
121+
"api_description",
122+
f"{test_config_dir}/config_without_api_description.yaml",
123+
),
124+
("name_pretty", f"{test_config_dir}/config_without_name_pretty.yaml"),
125+
(
126+
"product_documentation",
127+
f"{test_config_dir}/config_without_product_docs.yaml",
128+
),
129+
(
130+
"gapic_generator_version",
131+
f"{test_config_dir}/config_without_generator.yaml",
132+
),
133+
(
134+
"googleapis_commitish",
135+
f"{test_config_dir}/config_without_googleapis.yaml",
136+
),
137+
("owlbot_cli_image", f"{test_config_dir}/config_without_owlbot.yaml"),
138+
("synthtool_commitish", f"{test_config_dir}/config_without_synthtool.yaml"),
139+
(
140+
"template_excludes",
141+
f"{test_config_dir}/config_without_temp_excludes.yaml",
142+
),
143+
]
144+
)
145+
def test_from_yaml_without_key_fails(self, error_message_contains, path_to_yaml):
146+
self.assertRaisesRegex(
147+
ValueError,
148+
error_message_contains,
149+
from_yaml,
150+
path_to_yaml,
151+
)
152+
153+
def test_from_yaml_succeeds(self):
154+
config = from_yaml(f"{test_config_dir}/generation_config.yaml")
155+
self.assertEqual("2.34.0", config.gapic_generator_version)
156+
self.assertEqual(25.2, config.protobuf_version)
157+
self.assertEqual(
158+
"1a45bf7393b52407188c82e63101db7dc9c72026", config.googleapis_commitish
159+
)
160+
self.assertEqual(
161+
"sha256:623647ee79ac605858d09e60c1382a716c125fb776f69301b72de1cd35d49409",
162+
config.owlbot_cli_image,
163+
)
164+
self.assertEqual(
165+
"6612ab8f3afcd5e292aecd647f0fa68812c9f5b5", config.synthtool_commitish
166+
)
167+
self.assertEqual(
168+
[
169+
".github/*",
170+
".kokoro/*",
171+
"samples/*",
172+
"CODE_OF_CONDUCT.md",
173+
"CONTRIBUTING.md",
174+
"LICENSE",
175+
"SECURITY.md",
176+
"java.header",
177+
"license-checks.xml",
178+
"renovate.json",
179+
".gitignore",
180+
],
181+
config.template_excludes,
182+
)
183+
library = config.libraries[0]
184+
self.assertEqual("cloudasset", library.api_shortname)
185+
self.assertEqual("Cloud Asset Inventory", library.name_pretty)
186+
self.assertEqual(
187+
"https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview",
188+
library.product_documentation,
189+
)
190+
self.assertEqual(
191+
"provides inventory services based on a time series database.",
192+
library.api_description,
193+
)
194+
self.assertEqual("asset", library.library_name)
195+
gapics = library.gapic_configs
196+
self.assertEqual(5, len(gapics))
197+
self.assertEqual("google/cloud/asset/v1", gapics[0].proto_path)
198+
self.assertEqual("google/cloud/asset/v1p1beta1", gapics[1].proto_path)
199+
self.assertEqual("google/cloud/asset/v1p2beta1", gapics[2].proto_path)
200+
self.assertEqual("google/cloud/asset/v1p5beta1", gapics[3].proto_path)
201+
self.assertEqual("google/cloud/asset/v1p7beta1", gapics[4].proto_path)
202+
107203
def test_gapic_inputs_parse_grpc_only_succeeds(self):
108204
parsed = parse_build_file(build_file, "", "BUILD_grpc.bazel")
109205
self.assertEqual("grpc", parsed.transport)
@@ -252,9 +348,11 @@ def test_generate_prerequisite_files_success(self):
252348
f"{library_path}/owlbot.py",
253349
]
254350
self.__cleanup(files)
351+
config = self.__get_a_gen_config(1)
255352
proto_path = "google/cloud/baremetalsolution/v2"
256353
transport = "grpc"
257354
util.generate_prerequisite_files(
355+
config=config,
258356
library=library_1,
259357
proto_path=proto_path,
260358
transport=transport,
@@ -357,7 +455,19 @@ def __get_a_gen_config(num: int):
357455
googleapis_commitish="",
358456
owlbot_cli_image="",
359457
synthtool_commitish="",
360-
template_excludes=[],
458+
template_excludes=[
459+
".github/*",
460+
".kokoro/*",
461+
"samples/*",
462+
"CODE_OF_CONDUCT.md",
463+
"CONTRIBUTING.md",
464+
"LICENSE",
465+
"SECURITY.md",
466+
"java.header",
467+
"license-checks.xml",
468+
"renovate.json",
469+
".gitignore",
470+
],
361471
path_to_yaml=".",
362472
libraries=libraries,
363473
)

library_generation/utilities.py

+4-14
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,7 @@ def pull_api_definition(
320320

321321

322322
def generate_prerequisite_files(
323+
config: GenerationConfig,
323324
library: LibraryConfig,
324325
proto_path: str,
325326
transport: str,
@@ -331,6 +332,8 @@ def generate_prerequisite_files(
331332
Generate prerequisite files for a library.
332333
333334
Note that the version, if any, in the proto_path will be removed.
335+
:param config: a GenerationConfig object representing a parsed configuration
336+
yaml
334337
:param library: the library configuration
335338
:param proto_path: the proto path
336339
:param transport: transport supported by the library
@@ -417,24 +420,11 @@ def generate_prerequisite_files(
417420
# generate owlbot.py
418421
py_file = "owlbot.py"
419422
if not os.path.exists(f"{library_path}/{py_file}"):
420-
template_excludes = [
421-
".github/*",
422-
".kokoro/*",
423-
"samples/*",
424-
"CODE_OF_CONDUCT.md",
425-
"CONTRIBUTING.md",
426-
"LICENSE",
427-
"SECURITY.md",
428-
"java.header",
429-
"license-checks.xml",
430-
"renovate.json",
431-
".gitignore",
432-
]
433423
__render(
434424
template_name="owlbot.py.j2",
435425
output_name=f"{library_path}/{py_file}",
436426
should_include_templates=True,
437-
template_excludes=template_excludes,
427+
template_excludes=config.template_excludes,
438428
)
439429

440430

0 commit comments

Comments
 (0)