From ed21464d75eaa97743a63285be64fcca6e8f4cf2 Mon Sep 17 00:00:00 2001 From: Joe Wang <106995533+JoeWang1127@users.noreply.github.com> Date: Fri, 9 Feb 2024 15:58:12 +0000 Subject: [PATCH] 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. --- .../generate_composed_library.py | 5 +- library_generation/model/generation_config.py | 9 +- library_generation/requirements.in | 1 + .../google-cloud-java/generation_config.yaml | 1 - .../java-bigtable/generation_config.yaml | 1 - .../config_without_api_description.yaml | 5 + .../config_without_api_shortname.yaml | 4 + .../test-config/config_without_gapics.yaml | 3 + .../test-config/config_without_generator.yaml | 7 ++ .../config_without_googleapis.yaml | 8 ++ .../test-config/config_without_libraries.yaml | 1 + .../config_without_name_pretty.yaml | 6 + .../test-config/config_without_owlbot.yaml | 9 ++ .../config_without_product_docs.yaml | 7 ++ .../config_without_proto_path.yaml | 4 + .../test-config/config_without_synthtool.yaml | 10 ++ .../config_without_temp_excludes.yaml | 11 ++ .../test-config/generation_config.yaml | 34 +++++ library_generation/test/unit_tests.py | 116 +++++++++++++++++- library_generation/utilities.py | 18 +-- 20 files changed, 235 insertions(+), 25 deletions(-) create mode 100644 library_generation/test/resources/test-config/config_without_api_description.yaml create mode 100644 library_generation/test/resources/test-config/config_without_api_shortname.yaml create mode 100644 library_generation/test/resources/test-config/config_without_gapics.yaml create mode 100644 library_generation/test/resources/test-config/config_without_generator.yaml create mode 100644 library_generation/test/resources/test-config/config_without_googleapis.yaml create mode 100644 library_generation/test/resources/test-config/config_without_libraries.yaml create mode 100644 library_generation/test/resources/test-config/config_without_name_pretty.yaml create mode 100644 library_generation/test/resources/test-config/config_without_owlbot.yaml create mode 100644 library_generation/test/resources/test-config/config_without_product_docs.yaml create mode 100644 library_generation/test/resources/test-config/config_without_proto_path.yaml create mode 100644 library_generation/test/resources/test-config/config_without_synthtool.yaml create mode 100644 library_generation/test/resources/test-config/config_without_temp_excludes.yaml create mode 100644 library_generation/test/resources/test-config/generation_config.yaml diff --git a/library_generation/generate_composed_library.py b/library_generation/generate_composed_library.py index 1c2e1a4a91..37a1e75e75 100755 --- 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 index 7317edc932..b6c82f6e26 100644 --- 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 index 2bd5a0b0a8..1c1f476434 100644 --- 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 index a7ca6bec7b..349c10385c 100644 --- 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 index 7e62bea404..997e2c14c7 100644 --- 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 new file mode 100644 index 0000000000..79ff135067 --- /dev/null +++ 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 new file mode 100644 index 0000000000..ec8206be61 --- /dev/null +++ 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 new file mode 100644 index 0000000000..0f0c49fc48 --- /dev/null +++ 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 new file mode 100644 index 0000000000..c78b8b9700 --- /dev/null +++ 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 new file mode 100644 index 0000000000..e5a00ca4ee --- /dev/null +++ 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 new file mode 100644 index 0000000000..dbbe2ea318 --- /dev/null +++ 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 new file mode 100644 index 0000000000..f8612ad9ca --- /dev/null +++ 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 new file mode 100644 index 0000000000..7921f68bd2 --- /dev/null +++ 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 new file mode 100644 index 0000000000..e3921d2c0d --- /dev/null +++ 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 new file mode 100644 index 0000000000..e37b0cef63 --- /dev/null +++ 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 new file mode 100644 index 0000000000..8907f96bf7 --- /dev/null +++ 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 new file mode 100644 index 0000000000..9def2f3be6 --- /dev/null +++ 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 new file mode 100644 index 0000000000..b73fa4d65d --- /dev/null +++ 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 index 268b3ff0c8..04bcd46ccb 100644 --- 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 index 1eff1ae947..014b95ae8e 100755 --- 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, )