chore: use template excludes in generation config (#2453)

JoeWang1127 · web-flow · commit ed21464d75ea · 2024-02-09T10:58:12.000-05:00
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.
diff --git a/library_generation/generate_composed_library.py b/library_generation/generate_composed_library.py
@@ -54,8 +54,8 @@ def generate_composed_library(
     :param library_path: the path to which the generated file goes
     :param library: a LibraryConfig object contained inside config, passed here
     for convenience and to prevent all libraries to be processed
-    :param output_folder:
-    :param versions_file:
+    :param output_folder: the folder to where tools go
+    :param versions_file: the file containing version of libraries
     :return None
     """
     util.pull_api_definition(
@@ -74,6 +74,7 @@ def generate_composed_library(
         # owlbot.py) here because transport is parsed from BUILD.bazel,
         # which lives in a versioned proto_path.
         util.generate_prerequisite_files(
+            config=config,
             library=library,
             proto_path=util.remove_version_from(gapic.proto_path),
             transport=gapic_inputs.transport,
diff --git a/library_generation/model/generation_config.py b/library_generation/model/generation_config.py
@@ -31,7 +31,7 @@ def __init__(
         googleapis_commitish: str,
         owlbot_cli_image: str,
         synthtool_commitish: str,
-        template_excludes: str,
+        template_excludes: List[str],
         path_to_yaml: str,
         libraries: List[LibraryConfig],
         grpc_version: Optional[str] = None,
@@ -48,10 +48,11 @@ def __init__(
         self.protobuf_version = protobuf_version
 
 
-def from_yaml(path_to_yaml: str):
+def from_yaml(path_to_yaml: str) -> GenerationConfig:
     """
-    Parses a yaml located in path_to_yaml. Returns the parsed configuration
-    represented by the "model" classes
+    Parses a yaml located in path_to_yaml.
+    :param path_to_yaml: the path to the configuration file
+    :return the parsed configuration represented by the "model" classes
     """
     with open(path_to_yaml, "r") as file_stream:
         config = yaml.safe_load(file_stream)
diff --git a/library_generation/requirements.in b/library_generation/requirements.in
@@ -15,3 +15,4 @@ platformdirs==4.1.0
 PyYAML==6.0.1
 smmap==5.0.1
 typing==3.7.4.3
+parameterized==0.9.0 # used in parameterized test
diff --git a/library_generation/test/resources/integration/google-cloud-java/generation_config.yaml b/library_generation/test/resources/integration/google-cloud-java/generation_config.yaml
@@ -3,7 +3,6 @@ protobuf_version: 25.2
 googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026
 owlbot_cli_image: sha256:623647ee79ac605858d09e60c1382a716c125fb776f69301b72de1cd35d49409
 synthtool_commitish: 6612ab8f3afcd5e292aecd647f0fa68812c9f5b5
-destination_path: google-cloud-java
 template_excludes:
   - ".github/*"
   - ".kokoro/*"
diff --git a/library_generation/test/resources/integration/java-bigtable/generation_config.yaml b/library_generation/test/resources/integration/java-bigtable/generation_config.yaml
@@ -4,7 +4,6 @@ protobuf_version: 25.2
 googleapis_commitish: 40203ca1880849480bbff7b8715491060bbccdf1
 owlbot_cli_image: sha256:623647ee79ac605858d09e60c1382a716c125fb776f69301b72de1cd35d49409
 synthtool_commitish: 6612ab8f3afcd5e292aecd647f0fa68812c9f5b5
-destination_path: java-bigtable
 template_excludes:
   - ".gitignore"
   - ".kokoro/presubmit/integration.cfg"
diff --git a/library_generation/test/resources/test-config/config_without_api_description.yaml b/library_generation/test/resources/test-config/config_without_api_description.yaml
@@ -0,0 +1,5 @@
+gapic_generator_version: 2.34.0
+libraries:
+  - api_shortname: apigeeconnect
+    GAPICs:
+    - proto_path: google/cloud/apigeeconnect/v1
diff --git a/library_generation/test/resources/test-config/config_without_api_shortname.yaml b/library_generation/test/resources/test-config/config_without_api_shortname.yaml
@@ -0,0 +1,4 @@
+gapic_generator_version: 2.34.0
+libraries:
+  - GAPICs:
+    - proto_path: google/cloud/apigeeconnect/v1
diff --git a/library_generation/test/resources/test-config/config_without_gapics.yaml b/library_generation/test/resources/test-config/config_without_gapics.yaml
@@ -0,0 +1,3 @@
+gapic_generator_version: 2.34.0
+libraries:
+  random_key:
diff --git a/library_generation/test/resources/test-config/config_without_generator.yaml b/library_generation/test/resources/test-config/config_without_generator.yaml
@@ -0,0 +1,7 @@
+libraries:
+  - api_shortname: apigeeconnect
+    name_pretty: Apigee Connect
+    api_description: "allows the Apigee hybrid management"
+    product_documentation: "https://cloud.google.com/apigee/docs/hybrid/v1.3/apigee-connect/"
+    GAPICs:
+    - proto_path: google/cloud/apigeeconnect/v1
diff --git a/library_generation/test/resources/test-config/config_without_googleapis.yaml b/library_generation/test/resources/test-config/config_without_googleapis.yaml
@@ -0,0 +1,8 @@
+gapic_generator_version: 2.34.0
+libraries:
+  - api_shortname: apigeeconnect
+    name_pretty: Apigee Connect
+    api_description: "allows the Apigee hybrid management"
+    product_documentation: "https://cloud.google.com/apigee/docs/hybrid/v1.3/apigee-connect/"
+    GAPICs:
+    - proto_path: google/cloud/apigeeconnect/v1
diff --git a/library_generation/test/resources/test-config/config_without_libraries.yaml b/library_generation/test/resources/test-config/config_without_libraries.yaml
@@ -0,0 +1 @@
+gapic_generator_version: 2.34.0
diff --git a/library_generation/test/resources/test-config/config_without_name_pretty.yaml b/library_generation/test/resources/test-config/config_without_name_pretty.yaml
@@ -0,0 +1,6 @@
+gapic_generator_version: 2.34.0
+libraries:
+  - api_shortname: apigeeconnect
+    api_description: "allows the Apigee hybrid management"
+    GAPICs:
+    - proto_path: google/cloud/apigeeconnect/v1
diff --git a/library_generation/test/resources/test-config/config_without_owlbot.yaml b/library_generation/test/resources/test-config/config_without_owlbot.yaml
@@ -0,0 +1,9 @@
+gapic_generator_version: 2.34.0
+googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026
+libraries:
+  - api_shortname: apigeeconnect
+    name_pretty: Apigee Connect
+    api_description: "allows the Apigee hybrid management"
+    product_documentation: "https://cloud.google.com/apigee/docs/hybrid/v1.3/apigee-connect/"
+    GAPICs:
+    - proto_path: google/cloud/apigeeconnect/v1
diff --git a/library_generation/test/resources/test-config/config_without_product_docs.yaml b/library_generation/test/resources/test-config/config_without_product_docs.yaml
@@ -0,0 +1,7 @@
+gapic_generator_version: 2.34.0
+libraries:
+  - api_shortname: apigeeconnect
+    name_pretty: Apigee Connect
+    api_description: "allows the Apigee hybrid management"
+    GAPICs:
+    - proto_path: google/cloud/apigeeconnect/v1
diff --git a/library_generation/test/resources/test-config/config_without_proto_path.yaml b/library_generation/test/resources/test-config/config_without_proto_path.yaml
@@ -0,0 +1,4 @@
+gapic_generator_version: 2.34.0
+libraries:
+  - GAPICs:
+    - random_key:
diff --git a/library_generation/test/resources/test-config/config_without_synthtool.yaml b/library_generation/test/resources/test-config/config_without_synthtool.yaml
@@ -0,0 +1,10 @@
+gapic_generator_version: 2.34.0
+googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026
+owlbot_cli_image: sha256:623647ee79ac605858d09e60c1382a716c125fb776f69301b72de1cd35d49409
+libraries:
+  - api_shortname: apigeeconnect
+    name_pretty: Apigee Connect
+    api_description: "allows the Apigee hybrid management"
+    product_documentation: "https://cloud.google.com/apigee/docs/hybrid/v1.3/apigee-connect/"
+    GAPICs:
+    - proto_path: google/cloud/apigeeconnect/v1
diff --git a/library_generation/test/resources/test-config/config_without_temp_excludes.yaml b/library_generation/test/resources/test-config/config_without_temp_excludes.yaml
@@ -0,0 +1,11 @@
+gapic_generator_version: 2.34.0
+googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026
+owlbot_cli_image: sha256:623647ee79ac605858d09e60c1382a716c125fb776f69301b72de1cd35d49409
+synthtool_commitish: 6612ab8f3afcd5e292aecd647f0fa68812c9f5b5
+libraries:
+  - api_shortname: apigeeconnect
+    name_pretty: Apigee Connect
+    api_description: "allows the Apigee hybrid management"
+    product_documentation: "https://cloud.google.com/apigee/docs/hybrid/v1.3/apigee-connect/"
+    GAPICs:
+    - proto_path: google/cloud/apigeeconnect/v1
diff --git a/library_generation/test/resources/test-config/generation_config.yaml b/library_generation/test/resources/test-config/generation_config.yaml
@@ -0,0 +1,34 @@
+gapic_generator_version: 2.34.0
+protobuf_version: 25.2
+googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026
+owlbot_cli_image: sha256:623647ee79ac605858d09e60c1382a716c125fb776f69301b72de1cd35d49409
+synthtool_commitish: 6612ab8f3afcd5e292aecd647f0fa68812c9f5b5
+template_excludes:
+  - ".github/*"
+  - ".kokoro/*"
+  - "samples/*"
+  - "CODE_OF_CONDUCT.md"
+  - "CONTRIBUTING.md"
+  - "LICENSE"
+  - "SECURITY.md"
+  - "java.header"
+  - "license-checks.xml"
+  - "renovate.json"
+  - ".gitignore"
+libraries:
+  - api_shortname: cloudasset
+    name_pretty: Cloud Asset Inventory
+    product_documentation: "https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview"
+    api_description: "provides inventory services based on a time series database."
+    library_name: "asset"
+    client_documentation: "https://cloud.google.com/java/docs/reference/google-cloud-asset/latest/overview"
+    distribution_name: "com.google.cloud:google-cloud-asset"
+    release_level: "stable"
+    issue_tracker: "https://issuetracker.google.com/issues/new?component=187210&template=0"
+    api_reference: "https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview"
+    GAPICs:
+      - proto_path: google/cloud/asset/v1
+      - proto_path: google/cloud/asset/v1p1beta1
+      - proto_path: google/cloud/asset/v1p2beta1
+      - proto_path: google/cloud/asset/v1p5beta1
+      - proto_path: google/cloud/asset/v1p7beta1
diff --git a/library_generation/test/unit_tests.py b/library_generation/test/unit_tests.py
@@ -20,20 +20,21 @@
 import os
 import io
 import contextlib
-import subprocess
 from pathlib import Path
 from difflib import unified_diff
 from typing import List
-
+from parameterized import parameterized
 from library_generation import utilities as util
 from library_generation.model.gapic_config import GapicConfig
 from library_generation.model.generation_config import GenerationConfig
 from library_generation.model.gapic_inputs import parse as parse_build_file
+from library_generation.model.generation_config import from_yaml
 from library_generation.model.library_config import LibraryConfig
 
 script_dir = os.path.dirname(os.path.realpath(__file__))
 resources_dir = os.path.join(script_dir, "resources")
 build_file = Path(os.path.join(resources_dir, "misc")).resolve()
+test_config_dir = Path(os.path.join(resources_dir, "test-config")).resolve()
 library_1 = LibraryConfig(
     api_shortname="baremetalsolution",
     name_pretty="Bare Metal Solution",
@@ -104,6 +105,101 @@ def test_eprint_valid_input_succeeds(self):
         # print() appends a `\n` each time it's called
         self.assertEqual(test_input + "\n", result)
 
+    # parameterized tests need to run from the class, see
+    # https://github.com/wolever/parameterized/issues/37 for more info.
+    # This test confirms that a ValueError with an error message about a
+    # missing key (specified in the first parameter of each `parameterized`
+    # tuple) when parsing a test configuration yaml (second parameter) will
+    # be raised.
+    @parameterized.expand(
+        [
+            ("libraries", f"{test_config_dir}/config_without_libraries.yaml"),
+            ("GAPICs", f"{test_config_dir}/config_without_gapics.yaml"),
+            ("proto_path", f"{test_config_dir}/config_without_proto_path.yaml"),
+            ("api_shortname", f"{test_config_dir}/config_without_api_shortname.yaml"),
+            (
+                "api_description",
+                f"{test_config_dir}/config_without_api_description.yaml",
+            ),
+            ("name_pretty", f"{test_config_dir}/config_without_name_pretty.yaml"),
+            (
+                "product_documentation",
+                f"{test_config_dir}/config_without_product_docs.yaml",
+            ),
+            (
+                "gapic_generator_version",
+                f"{test_config_dir}/config_without_generator.yaml",
+            ),
+            (
+                "googleapis_commitish",
+                f"{test_config_dir}/config_without_googleapis.yaml",
+            ),
+            ("owlbot_cli_image", f"{test_config_dir}/config_without_owlbot.yaml"),
+            ("synthtool_commitish", f"{test_config_dir}/config_without_synthtool.yaml"),
+            (
+                "template_excludes",
+                f"{test_config_dir}/config_without_temp_excludes.yaml",
+            ),
+        ]
+    )
+    def test_from_yaml_without_key_fails(self, error_message_contains, path_to_yaml):
+        self.assertRaisesRegex(
+            ValueError,
+            error_message_contains,
+            from_yaml,
+            path_to_yaml,
+        )
+
+    def test_from_yaml_succeeds(self):
+        config = from_yaml(f"{test_config_dir}/generation_config.yaml")
+        self.assertEqual("2.34.0", config.gapic_generator_version)
+        self.assertEqual(25.2, config.protobuf_version)
+        self.assertEqual(
+            "1a45bf7393b52407188c82e63101db7dc9c72026", config.googleapis_commitish
+        )
+        self.assertEqual(
+            "sha256:623647ee79ac605858d09e60c1382a716c125fb776f69301b72de1cd35d49409",
+            config.owlbot_cli_image,
+        )
+        self.assertEqual(
+            "6612ab8f3afcd5e292aecd647f0fa68812c9f5b5", config.synthtool_commitish
+        )
+        self.assertEqual(
+            [
+                ".github/*",
+                ".kokoro/*",
+                "samples/*",
+                "CODE_OF_CONDUCT.md",
+                "CONTRIBUTING.md",
+                "LICENSE",
+                "SECURITY.md",
+                "java.header",
+                "license-checks.xml",
+                "renovate.json",
+                ".gitignore",
+            ],
+            config.template_excludes,
+        )
+        library = config.libraries[0]
+        self.assertEqual("cloudasset", library.api_shortname)
+        self.assertEqual("Cloud Asset Inventory", library.name_pretty)
+        self.assertEqual(
+            "https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview",
+            library.product_documentation,
+        )
+        self.assertEqual(
+            "provides inventory services based on a time series database.",
+            library.api_description,
+        )
+        self.assertEqual("asset", library.library_name)
+        gapics = library.gapic_configs
+        self.assertEqual(5, len(gapics))
+        self.assertEqual("google/cloud/asset/v1", gapics[0].proto_path)
+        self.assertEqual("google/cloud/asset/v1p1beta1", gapics[1].proto_path)
+        self.assertEqual("google/cloud/asset/v1p2beta1", gapics[2].proto_path)
+        self.assertEqual("google/cloud/asset/v1p5beta1", gapics[3].proto_path)
+        self.assertEqual("google/cloud/asset/v1p7beta1", gapics[4].proto_path)
+
     def test_gapic_inputs_parse_grpc_only_succeeds(self):
         parsed = parse_build_file(build_file, "", "BUILD_grpc.bazel")
         self.assertEqual("grpc", parsed.transport)
@@ -252,9 +348,11 @@ def test_generate_prerequisite_files_success(self):
             f"{library_path}/owlbot.py",
         ]
         self.__cleanup(files)
+        config = self.__get_a_gen_config(1)
         proto_path = "google/cloud/baremetalsolution/v2"
         transport = "grpc"
         util.generate_prerequisite_files(
+            config=config,
             library=library_1,
             proto_path=proto_path,
             transport=transport,
@@ -357,7 +455,19 @@ def __get_a_gen_config(num: int):
             googleapis_commitish="",
             owlbot_cli_image="",
             synthtool_commitish="",
-            template_excludes=[],
+            template_excludes=[
+                ".github/*",
+                ".kokoro/*",
+                "samples/*",
+                "CODE_OF_CONDUCT.md",
+                "CONTRIBUTING.md",
+                "LICENSE",
+                "SECURITY.md",
+                "java.header",
+                "license-checks.xml",
+                "renovate.json",
+                ".gitignore",
+            ],
             path_to_yaml=".",
             libraries=libraries,
         )
diff --git a/library_generation/utilities.py b/library_generation/utilities.py
@@ -320,6 +320,7 @@ def pull_api_definition(
 
 
 def generate_prerequisite_files(
+    config: GenerationConfig,
     library: LibraryConfig,
     proto_path: str,
     transport: str,
@@ -331,6 +332,8 @@ def generate_prerequisite_files(
     Generate prerequisite files for a library.
 
     Note that the version, if any, in the proto_path will be removed.
+    :param config: a GenerationConfig object representing a parsed configuration
+    yaml
     :param library: the library configuration
     :param proto_path: the proto path
     :param transport: transport supported by the library
@@ -417,24 +420,11 @@ def generate_prerequisite_files(
     # generate owlbot.py
     py_file = "owlbot.py"
     if not os.path.exists(f"{library_path}/{py_file}"):
-        template_excludes = [
-            ".github/*",
-            ".kokoro/*",
-            "samples/*",
-            "CODE_OF_CONDUCT.md",
-            "CONTRIBUTING.md",
-            "LICENSE",
-            "SECURITY.md",
-            "java.header",
-            "license-checks.xml",
-            "renovate.json",
-            ".gitignore",
-        ]
         __render(
             template_name="owlbot.py.j2",
             output_name=f"{library_path}/{py_file}",
             should_include_templates=True,
-            template_excludes=template_excludes,
+            template_excludes=config.template_excludes,
         )
 
 

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+gapic_generator_version: 2.34.0`
	`2`	`+libraries:`
	`3`	`+ random_key:`