From 0a97d55f82c4fb7dee150a81617874cfe4ec34af Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 26 Mar 2024 13:22:38 +0000 Subject: [PATCH 01/39] feat: add a generation config change class --- library_generation/model/config_change.py | 80 +++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 library_generation/model/config_change.py diff --git a/library_generation/model/config_change.py b/library_generation/model/config_change.py new file mode 100644 index 0000000000..67c52020de --- /dev/null +++ b/library_generation/model/config_change.py @@ -0,0 +1,80 @@ +# Copyright 2024 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from enum import Enum +from library_generation.model.library_config import LibraryConfig + + +class ChangeType(Enum): + GOOGLEAPIS_COMMIT = 1 + REPO_LEVEL_CHANGE = 2 + LIBRARIES_ADDITION = 3 + # As of Mar. 2024, we decide not to produce this type of change because we + # still need to manually remove the libray. + # LIBRARIES_REMOVAL = 4 + LIBRARY_LEVEL_CHANGE = 5 + GAPIC_ADDITION = 6 + # As of Mar. 2024, we decide not to produce this type of change because we + # still need to manually remove the libray. + # GAPIC_REMOVAL = 7 + + +class HashLibrary: + """ + Data class to group a LibraryConfig object and its hash value together. + """ + + def __init__(self, hash_value: int, library: LibraryConfig): + self.hash_value = hash_value + self.library = library + + +class LibraryChange: + def __init__(self, changed_param: str, latest_value: str, library_name: str = ""): + self.changed_param = changed_param + self.latest_value = latest_value + self.library_name = library_name + + +class ConfigChange: + def __init__(self, change_to_libraries: dict[ChangeType, list[LibraryChange]]): + self.change_to_libraries = change_to_libraries + + def get_changed_libraries(self): + pass + + + + + + + + + + + + + + + + + + + + + + + + + + From 233a105f8b01bb3b6a21bc22808fd33b62525a59 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 26 Mar 2024 13:23:00 +0000 Subject: [PATCH 02/39] refact config comparator --- .../utils/generation_config_comparator.py | 60 +++++-------------- 1 file changed, 15 insertions(+), 45 deletions(-) diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index 4fe8358230..b16f527c93 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -12,51 +12,21 @@ # See the License for the specific language governing permissions and # limitations under the License. from collections import defaultdict -from enum import Enum from typing import Any - from typing import Dict from typing import List - +from library_generation.model.config_change import ChangeType +from library_generation.model.config_change import ConfigChange +from library_generation.model.config_change import LibraryChange +from library_generation.model.config_change import HashLibrary from library_generation.model.gapic_config import GapicConfig from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig -class ChangeType(Enum): - GOOGLEAPIS_COMMIT = 1 - REPO_LEVEL_CHANGE = 2 - LIBRARIES_ADDITION = 3 - # As of Mar. 2024, we decide not to produce this type of change because we - # still need to manually remove the libray. - # LIBRARIES_REMOVAL = 4 - LIBRARY_LEVEL_CHANGE = 5 - GAPIC_ADDITION = 6 - # As of Mar. 2024, we decide not to produce this type of change because we - # still need to manually remove the libray. - # GAPIC_REMOVAL = 7 - - -class HashLibrary: - """ - Data class to group a LibraryConfig object and its hash value together. - """ - - def __init__(self, hash_value: int, library: LibraryConfig): - self.hash_value = hash_value - self.library = library - - -class ConfigChange: - def __init__(self, changed_param: str, latest_value: str, library_name: str = ""): - self.changed_param = changed_param - self.latest_value = latest_value - self.library_name = library_name - - def compare_config( baseline_config: GenerationConfig, latest_config: GenerationConfig -) -> Dict[ChangeType, list[ConfigChange]]: +) -> ConfigChange: """ Compare two GenerationConfig object and output a mapping from ConfigChange to a list of ConfigChange objects. @@ -65,9 +35,9 @@ def compare_config( :param baseline_config: the baseline GenerationConfig object :param latest_config: the latest GenerationConfig object - :return: a mapping from ConfigChange to a list of ConfigChange objects. + :return: a ConfigChange objects. """ - diff = defaultdict(list[ConfigChange]) + diff = defaultdict(list[LibraryChange]) baseline_params = __convert_params_to_sorted_list(baseline_config) latest_params = __convert_params_to_sorted_list(latest_config) for baseline_param, latest_param in zip(baseline_params, latest_params): @@ -76,7 +46,7 @@ def compare_config( if baseline_param[0] == "googleapis_commitish": diff[ChangeType.GOOGLEAPIS_COMMIT] = [] else: - config_change = ConfigChange( + config_change = LibraryChange( changed_param=latest_param[0], latest_value=latest_param[1], ) @@ -87,11 +57,11 @@ def compare_config( baseline_library_configs=baseline_config.libraries, latest_library_configs=latest_config.libraries, ) - return diff + return ConfigChange(change_to_libraries=diff) def __compare_libraries( - diff: Dict[ChangeType, list[ConfigChange]], + diff: Dict[ChangeType, list[LibraryChange]], baseline_library_configs: List[LibraryConfig], latest_library_configs: List[LibraryConfig], ) -> None: @@ -132,7 +102,7 @@ def __compare_libraries( # a library is added to latest_libraries if the library_name # is not in baseline_libraries. if library_name not in baseline_libraries: - config_change = ConfigChange( + config_change = LibraryChange( changed_param="", latest_value="", library_name=library_name ) diff[ChangeType.LIBRARIES_ADDITION].append(config_change) @@ -163,7 +133,7 @@ def __convert_to_hashed_library_dict( def __compare_changed_libraries( - diff: Dict[ChangeType, list[ConfigChange]], + diff: Dict[ChangeType, list[LibraryChange]], baseline_libraries: Dict[str, HashLibrary], latest_libraries: Dict[str, HashLibrary], changed_libraries: List[str], @@ -193,7 +163,7 @@ def __compare_changed_libraries( f"{library_name}: api_shortname must not change when library_name remains the same." ) else: - config_change = ConfigChange( + config_change = LibraryChange( changed_param=latest_param[0], latest_value=latest_param[1], library_name=library_name, @@ -212,7 +182,7 @@ def __compare_changed_libraries( def __compare_gapic_configs( - diff: Dict[ChangeType, list[ConfigChange]], + diff: Dict[ChangeType, list[LibraryChange]], library_name: str, baseline_gapic_configs: List[GapicConfig], latest_gapic_configs: List[GapicConfig], @@ -236,7 +206,7 @@ def __compare_gapic_configs( for proto_path in latest_proto_paths: if proto_path in baseline_proto_paths: continue - config_change = ConfigChange( + config_change = LibraryChange( changed_param="", latest_value=proto_path, library_name=library_name ) diff[ChangeType.GAPIC_ADDITION].append(config_change) From 86d911129a99e5dc542e6052f65846d2c67fb0eb Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 26 Mar 2024 13:23:15 +0000 Subject: [PATCH 03/39] fix unit tests --- ...generation_config_comparator_unit_tests.py | 176 ++++++++++++------ 1 file changed, 116 insertions(+), 60 deletions(-) diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py index c2c025c579..e526d6856b 100644 --- a/library_generation/test/utils/generation_config_comparator_unit_tests.py +++ b/library_generation/test/utils/generation_config_comparator_unit_tests.py @@ -64,7 +64,7 @@ def test_compare_config_not_change(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result) == 0) + self.assertTrue(len(result.change_to_libraries) == 0) def test_compare_config_googleapis_update(self): self.baseline_config.googleapis_commitish = ( @@ -77,7 +77,7 @@ def test_compare_config_googleapis_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.GOOGLEAPIS_COMMIT: []}, result) + self.assertEqual({ChangeType.GOOGLEAPIS_COMMIT: []}, result.change_to_libraries) def test_compare_config_generator_update(self): self.baseline_config.gapic_generator_version = "1.2.3" @@ -86,8 +86,10 @@ def test_compare_config_generator_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE][0] self.assertEqual("gapic_generator_version", config_change.changed_param) self.assertEqual("1.2.4", config_change.latest_value) @@ -98,8 +100,10 @@ def test_compare_config_owlbot_cli_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE][0] self.assertEqual("owlbot_cli_image", config_change.changed_param) self.assertEqual("image_version_456", config_change.latest_value) @@ -110,8 +114,10 @@ def test_compare_config_synthtool_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE][0] self.assertEqual("synthtool_commitish", config_change.changed_param) self.assertEqual("commit456", config_change.latest_value) @@ -122,8 +128,10 @@ def test_compare_protobuf_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE][0] self.assertEqual("protobuf_version", config_change.changed_param) self.assertEqual("3.27.0", config_change.latest_value) @@ -134,8 +142,10 @@ def test_compare_config_grpc_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE][0] self.assertEqual("grpc_version", config_change.changed_param) self.assertEqual("1.61.0", config_change.latest_value) @@ -151,8 +161,10 @@ def test_compare_config_template_excludes_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE][0] self.assertEqual("template_excludes", config_change.changed_param) self.assertEqual( [ @@ -178,8 +190,10 @@ def test_compare_config_library_addition(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARIES_ADDITION]) == 1) - config_change = result[ChangeType.LIBRARIES_ADDITION][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARIES_ADDITION]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARIES_ADDITION][0] self.assertEqual("new_library", config_change.library_name) def test_compare_config_api_shortname_update_without_library_name(self): @@ -188,8 +202,10 @@ def test_compare_config_api_shortname_update_without_library_name(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARIES_ADDITION]) == 1) - config_change = result[ChangeType.LIBRARIES_ADDITION][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARIES_ADDITION]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARIES_ADDITION][0] self.assertEqual("new_api_shortname", config_change.library_name) def test_compare_config_api_shortname_update_with_library_name_raise_error(self): @@ -210,8 +226,10 @@ def test_compare_config_library_name_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARIES_ADDITION]) == 1) - config_change = result[ChangeType.LIBRARIES_ADDITION][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARIES_ADDITION]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARIES_ADDITION][0] self.assertEqual("new_library_name", config_change.library_name) def test_compare_config_api_description_update(self): @@ -220,8 +238,10 @@ def test_compare_config_api_description_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("api_description", config_change.changed_param) self.assertEqual("updated description", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -232,8 +252,10 @@ def test_compare_config_name_pretty_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("name_pretty", config_change.changed_param) self.assertEqual("new name", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -244,8 +266,10 @@ def test_compare_config_product_docs_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("product_documentation", config_change.changed_param) self.assertEqual("new docs", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -256,8 +280,10 @@ def test_compare_config_library_type_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("library_type", config_change.changed_param) self.assertEqual("GAPIC_COMBO", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -268,8 +294,10 @@ def test_compare_config_release_level_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("release_level", config_change.changed_param) self.assertEqual("STABLE", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -280,8 +308,10 @@ def test_compare_config_api_id_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("api_id", config_change.changed_param) self.assertEqual("new_id", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -292,8 +322,10 @@ def test_compare_config_api_reference_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("api_reference", config_change.changed_param) self.assertEqual("new api_reference", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -304,8 +336,10 @@ def test_compare_config_code_owner_team_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("codeowner_team", config_change.changed_param) self.assertEqual("new team", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -316,8 +350,10 @@ def test_compare_config_excluded_deps_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("excluded_dependencies", config_change.changed_param) self.assertEqual("group:artifact", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -328,8 +364,10 @@ def test_compare_config_excluded_poms_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("excluded_poms", config_change.changed_param) self.assertEqual("pom.xml", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -340,8 +378,10 @@ def test_compare_config_client_docs_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("client_documentation", config_change.changed_param) self.assertEqual("new client docs", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -352,8 +392,10 @@ def test_compare_config_distribution_name_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("distribution_name", config_change.changed_param) self.assertEqual("new_group:new_artifact", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -364,8 +406,10 @@ def test_compare_config_group_id_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("group_id", config_change.changed_param) self.assertEqual("new_group", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -376,8 +420,10 @@ def test_compare_config_issue_tracker_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("issue_tracker", config_change.changed_param) self.assertEqual("new issue tracker", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -388,8 +434,10 @@ def test_compare_config_rest_docs_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("rest_documentation", config_change.changed_param) self.assertEqual("new rest docs", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -400,8 +448,10 @@ def test_compare_config_rpc_docs_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("rpc_documentation", config_change.changed_param) self.assertEqual("new rpc docs", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -412,8 +462,10 @@ def test_compare_config_cloud_api_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("cloud_api", config_change.changed_param) self.assertEqual(False, config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -424,8 +476,10 @@ def test_compare_config_requires_billing_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("requires_billing", config_change.changed_param) self.assertEqual(False, config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -436,8 +490,10 @@ def test_compare_config_extra_versioned_mod_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) - config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE][0] self.assertEqual("extra_versioned_modules", config_change.changed_param) self.assertEqual("extra module", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -450,8 +506,8 @@ def test_compare_config_version_addition(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.GAPIC_ADDITION]) == 1) - config_change = result[ChangeType.GAPIC_ADDITION][0] + self.assertTrue(len(result.change_to_libraries[ChangeType.GAPIC_ADDITION]) == 1) + config_change = result.change_to_libraries[ChangeType.GAPIC_ADDITION][0] self.assertEqual("", config_change.changed_param) self.assertEqual("google/new/library/v1", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) From 663bc069c1815b50f74557528f0d1ac3f8d5c637 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 26 Mar 2024 14:02:21 +0000 Subject: [PATCH 04/39] add proto_path_utils.py --- .../test/utils/proto_path_utils_unit_tests.py | 0 library_generation/utils/proto_path_utils.py | 65 +++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 library_generation/test/utils/proto_path_utils_unit_tests.py create mode 100644 library_generation/utils/proto_path_utils.py diff --git a/library_generation/test/utils/proto_path_utils_unit_tests.py b/library_generation/test/utils/proto_path_utils_unit_tests.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/library_generation/utils/proto_path_utils.py b/library_generation/utils/proto_path_utils.py new file mode 100644 index 0000000000..e8af7b55ca --- /dev/null +++ b/library_generation/utils/proto_path_utils.py @@ -0,0 +1,65 @@ +#!/usr/bin/env python3 +# Copyright 2024 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import re + +from typing import Dict + +from library_generation.model.generation_config import GenerationConfig +from library_generation.model.library_config import get_library_name + + +def remove_version_from(proto_path: str) -> str: + """ + Remove the version of a proto_path + :param proto_path: versioned proto_path + :return: the proto_path without version + """ + version_pattern = "^v[1-9]" + index = proto_path.rfind("/") + version = proto_path[index + 1 :] + if re.match(version_pattern, version): + return proto_path[:index] + return proto_path + + +def get_proto_paths(config: GenerationConfig) -> Dict[str, str]: + """ + Get versioned proto_path to library_name mapping from configuration file. + :param config: a GenerationConfig object. + :return: versioned proto_path to library_name mapping + """ + paths = {} + for library in config.libraries: + for gapic_config in library.gapic_configs: + paths[gapic_config.proto_path] = get_library_name(library) + return paths + + +def find_versioned_proto_path(file_path: str) -> str: + """ + Returns a versioned proto_path from a given file_path; or file_path itself + if it doesn't contain a versioned proto_path. + :param file_path: a proto file path + :return: the versioned proto_path + """ + version_regex = re.compile(r"^v[1-9].*") + directories = file_path.split("/") + for directory in directories: + result = version_regex.search(directory) + if result: + version = result[0] + idx = file_path.find(version) + return file_path[:idx] + version + return file_path From f4bc0f14d6627ee96881c51229ad026a55537ff2 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 26 Mar 2024 14:03:07 +0000 Subject: [PATCH 05/39] refactor unit tests --- .../test/utilities_unit_tests.py | 44 ------------ .../test/utils/proto_path_utils_unit_tests.py | 69 +++++++++++++++++++ 2 files changed, 69 insertions(+), 44 deletions(-) diff --git a/library_generation/test/utilities_unit_tests.py b/library_generation/test/utilities_unit_tests.py index 3d114f977f..bb7fded2d3 100644 --- a/library_generation/test/utilities_unit_tests.py +++ b/library_generation/test/utilities_unit_tests.py @@ -30,8 +30,6 @@ from library_generation.model.library_config import LibraryConfig from library_generation.test.test_utils import FileComparator from library_generation.test.test_utils import cleanup -from library_generation.utilities import find_versioned_proto_path -from library_generation.utilities import get_file_paths script_dir = os.path.dirname(os.path.realpath(__file__)) resources_dir = os.path.join(script_dir, "resources") @@ -217,36 +215,6 @@ def test_from_yaml_succeeds(self): self.assertEqual("google/cloud/asset/v1p5beta1", gapics[3].proto_path) self.assertEqual("google/cloud/asset/v1p7beta1", gapics[4].proto_path) - def test_get_file_paths_from_yaml_success(self): - paths = get_file_paths(from_yaml(f"{test_config_dir}/generation_config.yaml")) - self.assertEqual( - { - "google/cloud/asset/v1": "asset", - "google/cloud/asset/v1p1beta1": "asset", - "google/cloud/asset/v1p2beta1": "asset", - "google/cloud/asset/v1p5beta1": "asset", - "google/cloud/asset/v1p7beta1": "asset", - }, - paths, - ) - - @parameterized.expand( - [ - ( - "google/cloud/aiplatform/v1/schema/predict/params/image_classification.proto", - "google/cloud/aiplatform/v1", - ), - ( - "google/cloud/asset/v1p2beta1/assets.proto", - "google/cloud/asset/v1p2beta1", - ), - ("google/type/color.proto", "google/type/color.proto"), - ] - ) - def test_find_versioned_proto_path(self, file_path, expected): - proto_path = find_versioned_proto_path(file_path) - self.assertEqual(expected, proto_path) - @parameterized.expand( [ ("BUILD_no_additional_protos.bazel", " "), @@ -379,18 +347,6 @@ def test_gapic_inputs_parse_no_service_yaml_returns_empty_string(self): ) self.assertEqual("", parsed.service_yaml) - def test_remove_version_from_returns_non_versioned_path(self): - proto_path = "google/cloud/aiplatform/v1" - self.assertEqual( - "google/cloud/aiplatform", util.remove_version_from(proto_path) - ) - - def test_remove_version_from_returns_self(self): - proto_path = "google/cloud/aiplatform" - self.assertEqual( - "google/cloud/aiplatform", util.remove_version_from(proto_path) - ) - def test_generate_prerequisite_files_non_monorepo_success(self): library_path = self.__setup_prerequisite_files( num_libraries=1, library_type="GAPIC_COMBO" diff --git a/library_generation/test/utils/proto_path_utils_unit_tests.py b/library_generation/test/utils/proto_path_utils_unit_tests.py index e69de29bb2..6ff13f8080 100644 --- a/library_generation/test/utils/proto_path_utils_unit_tests.py +++ b/library_generation/test/utils/proto_path_utils_unit_tests.py @@ -0,0 +1,69 @@ +#!/usr/bin/env python3 +# Copyright 2024 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import os +import unittest +from pathlib import Path + +from library_generation.model.generation_config import from_yaml +from library_generation.utils.proto_path_utils import ( + get_proto_paths, + find_versioned_proto_path, + remove_version_from, +) + +script_dir = os.path.dirname(os.path.realpath(__file__)) +resources_dir = os.path.join(script_dir, "..", "resources") +test_config_dir = Path(os.path.join(resources_dir, "test-config")).resolve() + + +class ProtoPathsUtilsTest(unittest.TestCase): + def test_get_proto_paths_from_yaml_success(self): + paths = get_proto_paths(from_yaml(f"{test_config_dir}/generation_config.yaml")) + self.assertEqual( + { + "google/cloud/asset/v1": "asset", + "google/cloud/asset/v1p1beta1": "asset", + "google/cloud/asset/v1p2beta1": "asset", + "google/cloud/asset/v1p5beta1": "asset", + "google/cloud/asset/v1p7beta1": "asset", + }, + paths, + ) + + def test_find_versioned_proto_path_nested_version_success(self): + file_path = "google/cloud/aiplatform/v1/schema/predict/params/image_classification.proto" + expected = "google/cloud/aiplatform/v1" + proto_path = find_versioned_proto_path(file_path) + self.assertEqual(expected, proto_path) + + def test_find_versioned_proto_path_success(self): + file_path = "google/cloud/asset/v1p2beta1/assets.proto" + expected = "google/cloud/asset/v1p2beta1" + proto_path = find_versioned_proto_path(file_path) + self.assertEqual(expected, proto_path) + + def test_find_versioned_proto_without_version_return_itself(self): + file_path = "google/type/color.proto" + expected = "google/type/color.proto" + proto_path = find_versioned_proto_path(file_path) + self.assertEqual(expected, proto_path) + + def test_remove_version_from_returns_non_versioned_path(self): + proto_path = "google/cloud/aiplatform/v1" + self.assertEqual("google/cloud/aiplatform", remove_version_from(proto_path)) + + def test_remove_version_from_returns_self(self): + proto_path = "google/cloud/aiplatform" + self.assertEqual("google/cloud/aiplatform", remove_version_from(proto_path)) From 501918be0da5d55016ab6fbd1c640debc0a6d90e Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 26 Mar 2024 14:03:36 +0000 Subject: [PATCH 06/39] refactor config comparator --- library_generation/model/config_change.py | 88 +++++++++++++------ .../utils/generation_config_comparator.py | 6 +- 2 files changed, 66 insertions(+), 28 deletions(-) diff --git a/library_generation/model/config_change.py b/library_generation/model/config_change.py index 67c52020de..10e11b0d9d 100644 --- a/library_generation/model/config_change.py +++ b/library_generation/model/config_change.py @@ -12,6 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. from enum import Enum +from typing import Optional +from git import Commit +from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig @@ -46,35 +49,66 @@ def __init__(self, changed_param: str, latest_value: str, library_name: str = "" self.library_name = library_name +class QualifiedCommit: + def __init__(self, commit: Commit, libraries: list[str]): + self.commit = commit + self.libraries = libraries + + class ConfigChange: - def __init__(self, change_to_libraries: dict[ChangeType, list[LibraryChange]]): + def __init__( + self, + change_to_libraries: dict[ChangeType, list[LibraryChange]], + baseline_config: GenerationConfig, + latest_config: GenerationConfig, + ): self.change_to_libraries = change_to_libraries - - def get_changed_libraries(self): + self.baseline_config = baseline_config + self.latest_config = latest_config + + def get_changed_libraries(self) -> Optional[list[str]]: + """ + Returns library name of changed libraries. + None if there is a repository level change. + :return: library names of change libraries. + """ + if ChangeType.REPO_LEVEL_CHANGE in self.change_to_libraries: + return None + library_names = [] + for change_type, library_changes in self.change_to_libraries.items(): + if change_type == ChangeType.GOOGLEAPIS_COMMIT: + library_names.extend( + self.__get_changed_libraries_in_googleapis_commit() + ) + else: + library_names.extend( + [library_change.library_name for library_change in library_changes] + ) + return library_names + + def get_qualified_commits(self) -> list[QualifiedCommit]: pass + def __get_changed_libraries_in_googleapis_commit(self) -> list[str]: + pass - - - - - - - - - - - - - - - - - - - - - - - - + def __create_qualified_commit( + self, + proto_paths: dict[str, str], commit: Commit + ) -> Optional[QualifiedCommit]: + libraries = [] + for file in commit.stats.files.keys(): + if file.endswith("BUILD.bazel"): + continue + versioned_proto_path = find_versioned_proto_path(file) + if versioned_proto_path in proto_paths: + # Even though a commit usually only changes one + # library, we don't want to miss generating a + # library because the commit may change multiple + # libraries. + # Therefore, we keep a list of library_name to + # avoid missing changed libraries. + libraries.append(proto_paths[versioned_proto_path]) + if len(libraries) == 0: + return None + return QualifiedCommit(commit=commit, libraries=libraries) diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index b16f527c93..79a5f3c1aa 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -57,7 +57,11 @@ def compare_config( baseline_library_configs=baseline_config.libraries, latest_library_configs=latest_config.libraries, ) - return ConfigChange(change_to_libraries=diff) + return ConfigChange( + change_to_libraries=diff, + baseline_config=baseline_config, + latest_config=latest_config, + ) def __compare_libraries( From e1fc03d40faa990592128bf77d8956643226c8ec Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 26 Mar 2024 14:05:36 +0000 Subject: [PATCH 07/39] fix unit tests --- library_generation/utils/proto_path_utils.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library_generation/utils/proto_path_utils.py b/library_generation/utils/proto_path_utils.py index e8af7b55ca..1cabbff31e 100644 --- a/library_generation/utils/proto_path_utils.py +++ b/library_generation/utils/proto_path_utils.py @@ -17,7 +17,6 @@ from typing import Dict from library_generation.model.generation_config import GenerationConfig -from library_generation.model.library_config import get_library_name def remove_version_from(proto_path: str) -> str: @@ -43,7 +42,7 @@ def get_proto_paths(config: GenerationConfig) -> Dict[str, str]: paths = {} for library in config.libraries: for gapic_config in library.gapic_configs: - paths[gapic_config.proto_path] = get_library_name(library) + paths[gapic_config.proto_path] = library.get_library_name() return paths From 9598a1fa2844451b82db17147dfc00410b08488a Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 26 Mar 2024 14:08:37 +0000 Subject: [PATCH 08/39] format code --- library_generation/model/config_change.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library_generation/model/config_change.py b/library_generation/model/config_change.py index 10e11b0d9d..4a053aadc8 100644 --- a/library_generation/model/config_change.py +++ b/library_generation/model/config_change.py @@ -93,8 +93,7 @@ def __get_changed_libraries_in_googleapis_commit(self) -> list[str]: pass def __create_qualified_commit( - self, - proto_paths: dict[str, str], commit: Commit + self, proto_paths: dict[str, str], commit: Commit ) -> Optional[QualifiedCommit]: libraries = [] for file in commit.stats.files.keys(): From 9f8a95d481273de881ac37a0e98b0e833833140b Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 26 Mar 2024 16:30:52 +0000 Subject: [PATCH 09/39] refactor utilities --- library_generation/utilities.py | 49 +-------------------------------- 1 file changed, 1 insertion(+), 48 deletions(-) diff --git a/library_generation/utilities.py b/library_generation/utilities.py index fb7e8938d2..2ec220b6ed 100755 --- a/library_generation/utilities.py +++ b/library_generation/utilities.py @@ -18,12 +18,12 @@ import shutil import re from pathlib import Path -from typing import Dict from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig from typing import List from library_generation.model.repo_config import RepoConfig from library_generation.utils.file_render import render +from library_generation.utils.proto_path_utils import remove_version_from script_dir = os.path.dirname(os.path.realpath(__file__)) @@ -93,20 +93,6 @@ def eprint(*args, **kwargs): print(*args, file=sys.stderr, **kwargs) -def remove_version_from(proto_path: str) -> str: - """ - Remove the version of a proto_path - :param proto_path: versioned proto_path - :return: the proto_path without version - """ - version_pattern = "^v[1-9]" - index = proto_path.rfind("/") - version = proto_path[index + 1 :] - if re.match(version_pattern, version): - return proto_path[:index] - return proto_path - - def prepare_repo( gen_config: GenerationConfig, library_config: List[LibraryConfig], @@ -314,36 +300,3 @@ def generate_prerequisite_files( should_include_templates=True, template_excludes=config.template_excludes, ) - - -def get_file_paths(config: GenerationConfig) -> Dict[str, str]: - """ - Get versioned proto_path to library_name mapping from configuration file. - - :param config: a GenerationConfig object. - :return: versioned proto_path to library_name mapping - """ - paths = {} - for library in config.libraries: - for gapic_config in library.gapic_configs: - paths[gapic_config.proto_path] = library.get_library_name() - return paths - - -def find_versioned_proto_path(file_path: str) -> str: - """ - Returns a versioned proto_path from a given file_path; or file_path itself - if it doesn't contain a versioned proto_path. - - :param file_path: a proto file path - :return: the versioned proto_path - """ - version_regex = re.compile(r"^v[1-9].*") - directories = file_path.split("/") - for directory in directories: - result = version_regex.search(directory) - if result: - version = result[0] - idx = file_path.find(version) - return file_path[:idx] + version - return file_path From 59e02bb6739f42e908133b971397107c2e2afbf9 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 26 Mar 2024 16:31:46 +0000 Subject: [PATCH 10/39] refactor proto_path_utils --- library_generation/utils/proto_path_utils.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/library_generation/utils/proto_path_utils.py b/library_generation/utils/proto_path_utils.py index 1cabbff31e..1ee26dc93d 100644 --- a/library_generation/utils/proto_path_utils.py +++ b/library_generation/utils/proto_path_utils.py @@ -13,9 +13,6 @@ # See the License for the specific language governing permissions and # limitations under the License. import re - -from typing import Dict - from library_generation.model.generation_config import GenerationConfig @@ -33,7 +30,7 @@ def remove_version_from(proto_path: str) -> str: return proto_path -def get_proto_paths(config: GenerationConfig) -> Dict[str, str]: +def get_proto_paths(config: GenerationConfig) -> dict[str, str]: """ Get versioned proto_path to library_name mapping from configuration file. :param config: a GenerationConfig object. From 4f58df734faaf806623aff9251bdd006ddca7c21 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 26 Mar 2024 17:16:26 +0000 Subject: [PATCH 11/39] add unit tests --- .../test/model/config_change_unit_tests.py | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 library_generation/test/model/config_change_unit_tests.py diff --git a/library_generation/test/model/config_change_unit_tests.py b/library_generation/test/model/config_change_unit_tests.py new file mode 100644 index 0000000000..407c476c22 --- /dev/null +++ b/library_generation/test/model/config_change_unit_tests.py @@ -0,0 +1,92 @@ +# Copyright 2024 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import unittest + +from library_generation.model.config_change import ChangeType +from library_generation.model.config_change import ConfigChange +from library_generation.model.config_change import LibraryChange +from library_generation.model.generation_config import GenerationConfig + + +class ConfigChangeTest(unittest.TestCase): + def test_get_changed_libraries_with_repo_level_change_returns_none(self): + config_change = ConfigChange( + change_to_libraries={ + ChangeType.REPO_LEVEL_CHANGE: [], + # add a library level change to verify this type of change has + # no impact on the result. + ChangeType.LIBRARY_LEVEL_CHANGE: [ + LibraryChange( + changed_param="test", + latest_value="test", + library_name="test-library", + ) + ], + }, + baseline_config=ConfigChangeTest.__get_a_gen_config(), + latest_config=ConfigChangeTest.__get_a_gen_config(), + ) + self.assertIsNone(config_change.get_changed_libraries()) + + def test_get_changed_libraries_with_library_level_change_returns_list(self): + config_change = ConfigChange( + change_to_libraries={ + ChangeType.LIBRARY_LEVEL_CHANGE: [ + LibraryChange( + changed_param="test", + latest_value="test", + library_name="a-library", + ), + LibraryChange( + changed_param="test", + latest_value="test", + library_name="another-library", + ), + ], + ChangeType.LIBRARIES_ADDITION: [ + LibraryChange( + changed_param="test", + latest_value="test", + library_name="new-library", + ), + ], + ChangeType.GAPIC_ADDITION: [ + LibraryChange( + changed_param="test", + latest_value="test", + library_name="library-with-new-version", + ), + ], + }, + baseline_config=ConfigChangeTest.__get_a_gen_config(), + latest_config=ConfigChangeTest.__get_a_gen_config(), + ) + self.assertEqual( + ["a-library", "another-library", "new-library", "library-with-new-version"], + config_change.get_changed_libraries(), + ) + + @staticmethod + def __get_a_gen_config() -> GenerationConfig: + return GenerationConfig( + gapic_generator_version="", + googleapis_commitish="", + owlbot_cli_image="", + synthtool_commitish="", + template_excludes=[], + path_to_yaml="", + grpc_version="", + protobuf_version="", + libraries=[], + ) From 8bed4231d00e6f02afdc601b9fdbc90ee540ad6b Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 26 Mar 2024 17:17:38 +0000 Subject: [PATCH 12/39] add get_qualified_commits --- library_generation/model/config_change.py | 64 +++++++++++++++++++---- 1 file changed, 53 insertions(+), 11 deletions(-) diff --git a/library_generation/model/config_change.py b/library_generation/model/config_change.py index 4a053aadc8..3e11f602cc 100644 --- a/library_generation/model/config_change.py +++ b/library_generation/model/config_change.py @@ -11,11 +11,15 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import os +import shutil from enum import Enum from typing import Optional -from git import Commit +from git import Commit, Repo from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig +from library_generation.utils.proto_path_utils import find_versioned_proto_path +from library_generation.utils.proto_path_utils import get_proto_paths class ChangeType(Enum): @@ -77,24 +81,64 @@ def get_changed_libraries(self) -> Optional[list[str]]: library_names = [] for change_type, library_changes in self.change_to_libraries.items(): if change_type == ChangeType.GOOGLEAPIS_COMMIT: - library_names.extend( - self.__get_changed_libraries_in_googleapis_commit() - ) + library_names.extend(self.__get_library_name_from_qualified_commits()) else: library_names.extend( [library_change.library_name for library_change in library_changes] ) return library_names - def get_qualified_commits(self) -> list[QualifiedCommit]: - pass + def get_qualified_commits( + self, + repo_url: str = "https://github.com/googleapis/googleapis.git", + ) -> list[QualifiedCommit]: + """ + Returns qualified commits from configuration change. - def __get_changed_libraries_in_googleapis_commit(self) -> list[str]: - pass + A qualified commit is a commit that changes at least one file (excluding + BUILD.bazel) within a versioned proto path in the given proto_paths. + :param repo_url: the repository contains the commit history. + :return: QualifiedCommit objects. + """ + tmp_dir = "/tmp/repo" + shutil.rmtree(tmp_dir, ignore_errors=True) + os.mkdir(tmp_dir) + # we only need commit history, thus shadow clone is enough. + repo = Repo.clone_from(url=repo_url, to_path=tmp_dir, filter=["blob:none"]) + commit = repo.commit(self.latest_config.googleapis_commitish) + proto_paths = get_proto_paths(self.latest_config) + qualified_commits = [] + while str(commit.hexsha) != self.baseline_config.googleapis_commitish: + qualified_commit = ConfigChange.__create_qualified_commit( + proto_paths=proto_paths, commit=commit + ) + if qualified_commit is not None: + qualified_commits.append(qualified_commit) + commit_parents = commit.parents + if len(commit_parents) == 0: + break + commit = commit_parents[0] + shutil.rmtree(tmp_dir, ignore_errors=True) + return qualified_commits + def __get_library_name_from_qualified_commits(self) -> list[str]: + qualified_commits = self.get_qualified_commits() + library_names = [] + for qualified_commit in qualified_commits: + library_names.extend(qualified_commit.libraries) + return library_names + + @staticmethod def __create_qualified_commit( - self, proto_paths: dict[str, str], commit: Commit + proto_paths: dict[str, str], commit: Commit ) -> Optional[QualifiedCommit]: + """ + Returns a qualified commit from the given Commit object; otherwise None. + + :param proto_paths: + :param commit: + :return: + """ libraries = [] for file in commit.stats.files.keys(): if file.endswith("BUILD.bazel"): @@ -105,8 +149,6 @@ def __create_qualified_commit( # library, we don't want to miss generating a # library because the commit may change multiple # libraries. - # Therefore, we keep a list of library_name to - # avoid missing changed libraries. libraries.append(proto_paths[versioned_proto_path]) if len(libraries) == 0: return None From 88da8d8798032e8e04a3b57a836cfdc6d29ff6b6 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 26 Mar 2024 17:21:27 +0000 Subject: [PATCH 13/39] fix generate_pr_description.py --- library_generation/generate_pr_description.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py index 8048f810a4..b5f354b0d7 100644 --- a/library_generation/generate_pr_description.py +++ b/library_generation/generate_pr_description.py @@ -19,10 +19,9 @@ import click from git import Commit, Repo from library_generation.model.generation_config import from_yaml -from library_generation.utilities import find_versioned_proto_path +from library_generation.utils.proto_path_utils import find_versioned_proto_path from library_generation.utils.commit_message_formatter import format_commit_message -from library_generation.utilities import get_file_paths -from library_generation.utils.commit_message_formatter import wrap_nested_commit +from library_generation.utils.proto_path_utils import get_proto_paths from library_generation.utils.commit_message_formatter import wrap_override_commit @@ -87,7 +86,7 @@ def generate_pr_descriptions( baseline_commit: str, ) -> str: config = from_yaml(generation_config_yaml) - paths = get_file_paths(config) + paths = get_proto_paths(config) return __get_commit_messages( repo_url=repo_url, latest_commit=config.googleapis_commitish, From 5fda1d74916e61bf5b5b8d4dc3a523eac36e2872 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 26 Mar 2024 18:34:15 +0000 Subject: [PATCH 14/39] add unit tests --- library_generation/model/config_change.py | 6 +- .../test/model/config_change_unit_tests.py | 78 ++++++++++++++++++- 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/library_generation/model/config_change.py b/library_generation/model/config_change.py index 3e11f602cc..109c646a20 100644 --- a/library_generation/model/config_change.py +++ b/library_generation/model/config_change.py @@ -54,7 +54,7 @@ def __init__(self, changed_param: str, latest_value: str, library_name: str = "" class QualifiedCommit: - def __init__(self, commit: Commit, libraries: list[str]): + def __init__(self, commit: Commit, libraries: set[str]): self.commit = commit self.libraries = libraries @@ -139,7 +139,7 @@ def __create_qualified_commit( :param commit: :return: """ - libraries = [] + libraries = set() for file in commit.stats.files.keys(): if file.endswith("BUILD.bazel"): continue @@ -149,7 +149,7 @@ def __create_qualified_commit( # library, we don't want to miss generating a # library because the commit may change multiple # libraries. - libraries.append(proto_paths[versioned_proto_path]) + libraries.add(proto_paths[versioned_proto_path]) if len(libraries) == 0: return None return QualifiedCommit(commit=commit, libraries=libraries) diff --git a/library_generation/test/model/config_change_unit_tests.py b/library_generation/test/model/config_change_unit_tests.py index 407c476c22..af815d68cb 100644 --- a/library_generation/test/model/config_change_unit_tests.py +++ b/library_generation/test/model/config_change_unit_tests.py @@ -16,7 +16,9 @@ from library_generation.model.config_change import ChangeType from library_generation.model.config_change import ConfigChange from library_generation.model.config_change import LibraryChange +from library_generation.model.gapic_config import GapicConfig from library_generation.model.generation_config import GenerationConfig +from library_generation.model.library_config import LibraryConfig class ConfigChangeTest(unittest.TestCase): @@ -77,16 +79,86 @@ def test_get_changed_libraries_with_library_level_change_returns_list(self): config_change.get_changed_libraries(), ) + def test_get_qualified_commits_success(self): + config_config = ConfigChange( + change_to_libraries={}, + baseline_config=ConfigChangeTest.__get_a_gen_config( + googleapis_commitish="277145d108819fa30fbed3a7cbbb50f91eb6155e" + ), + latest_config=ConfigChangeTest.__get_a_gen_config( + googleapis_commitish="8984ddb508dea0e673b724c58338e810b1d8aee3", + libraries=[ + ConfigChangeTest.__get_a_library_config( + library_name="gke-backup", + gapic_configs=[ + GapicConfig(proto_path="google/cloud/gkebackup/v1") + ], + ), + ConfigChangeTest.__get_a_library_config( + library_name="aiplatform", + gapic_configs=[ + GapicConfig(proto_path="google/cloud/aiplatform/v1beta1") + ], + ), + ConfigChangeTest.__get_a_library_config( + library_name="network-management", + gapic_configs=[ + GapicConfig(proto_path="google/cloud/networkmanagement/v1"), + GapicConfig( + proto_path="google/cloud/networkmanagement/v1beta1" + ), + ], + ), + ], + ), + ) + qualified_commits = config_config.get_qualified_commits() + self.assertEqual(3, len(qualified_commits)) + self.assertEqual({"gke-backup"}, qualified_commits[0].libraries) + self.assertEqual( + "b8691edb3f1d3c1583aa9cd89240eb359eebe9c7", + qualified_commits[0].commit.hexsha, + ) + self.assertEqual({"aiplatform"}, qualified_commits[1].libraries) + self.assertEqual( + "b82095baef02e525bee7bb1c48911c33b66acdf0", + qualified_commits[1].commit.hexsha, + ) + self.assertEqual({"network-management"}, qualified_commits[2].libraries) + self.assertEqual( + "efad09c9f0d46ae0786d810a88024363e06c6ca3", + qualified_commits[2].commit.hexsha, + ) + @staticmethod - def __get_a_gen_config() -> GenerationConfig: + def __get_a_gen_config( + googleapis_commitish="", libraries: list[LibraryConfig] = None + ) -> GenerationConfig: + if libraries is None: + libraries = [] return GenerationConfig( gapic_generator_version="", - googleapis_commitish="", + googleapis_commitish=googleapis_commitish, owlbot_cli_image="", synthtool_commitish="", template_excludes=[], path_to_yaml="", grpc_version="", protobuf_version="", - libraries=[], + libraries=libraries, + ) + + @staticmethod + def __get_a_library_config( + library_name: str, gapic_configs: list[GapicConfig] = None + ) -> LibraryConfig: + if gapic_configs is None: + gapic_configs = [] + return LibraryConfig( + api_shortname="existing_library", + api_description="", + name_pretty="", + product_documentation="", + gapic_configs=gapic_configs, + library_name=library_name, ) From 14d419c1636abfc3111d77d78fb25913e033edc2 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 26 Mar 2024 18:53:30 +0000 Subject: [PATCH 15/39] add tests --- .../test/model/config_change_unit_tests.py | 64 ++++++++++++++++++- 1 file changed, 62 insertions(+), 2 deletions(-) diff --git a/library_generation/test/model/config_change_unit_tests.py b/library_generation/test/model/config_change_unit_tests.py index af815d68cb..ef1c0fa693 100644 --- a/library_generation/test/model/config_change_unit_tests.py +++ b/library_generation/test/model/config_change_unit_tests.py @@ -79,8 +79,48 @@ def test_get_changed_libraries_with_library_level_change_returns_list(self): config_change.get_changed_libraries(), ) + def test_get_changed_libraries_with_mix_changes_returns_list(self): + config_change = ConfigChange( + change_to_libraries={ + ChangeType.GOOGLEAPIS_COMMIT: [], + ChangeType.LIBRARY_LEVEL_CHANGE: [ + LibraryChange( + changed_param="test", + latest_value="test", + library_name="a-library", + ) + ], + ChangeType.LIBRARIES_ADDITION: [ + LibraryChange( + changed_param="test", + latest_value="test", + library_name="new-library", + ), + ], + }, + baseline_config=ConfigChangeTest.__get_a_gen_config( + googleapis_commitish="277145d108819fa30fbed3a7cbbb50f91eb6155e" + ), + latest_config=ConfigChangeTest.__get_a_gen_config( + googleapis_commitish="8984ddb508dea0e673b724c58338e810b1d8aee3", + libraries=[ + ConfigChangeTest.__get_a_library_config( + library_name="gke-backup", + gapic_configs=[ + GapicConfig(proto_path="google/cloud/gkebackup/v1") + ], + ), + ], + ), + ) + + self.assertEqual( + ["a-library", "gke-backup", "new-library"], + sorted(config_change.get_changed_libraries()), + ) + def test_get_qualified_commits_success(self): - config_config = ConfigChange( + config_change = ConfigChange( change_to_libraries={}, baseline_config=ConfigChangeTest.__get_a_gen_config( googleapis_commitish="277145d108819fa30fbed3a7cbbb50f91eb6155e" @@ -112,7 +152,7 @@ def test_get_qualified_commits_success(self): ], ), ) - qualified_commits = config_config.get_qualified_commits() + qualified_commits = config_change.get_qualified_commits() self.assertEqual(3, len(qualified_commits)) self.assertEqual({"gke-backup"}, qualified_commits[0].libraries) self.assertEqual( @@ -130,6 +170,26 @@ def test_get_qualified_commits_success(self): qualified_commits[2].commit.hexsha, ) + def test_get_qualified_commits_build_only_commit_returns_none(self): + config_change = ConfigChange( + change_to_libraries={}, + baseline_config=ConfigChangeTest.__get_a_gen_config( + googleapis_commitish="bdda0174f68a738518ec311e05e6fd9bbe19cd78" + ), + latest_config=ConfigChangeTest.__get_a_gen_config( + googleapis_commitish="c9a5050ef225b0011603e1109cf53ab1de0a8e53", + libraries=[ + ConfigChangeTest.__get_a_library_config( + library_name="chat", + gapic_configs=[GapicConfig(proto_path="google/chat/v1")], + ) + ], + ), + ) + # one commit between c9a5050 (latest) and c9a5050 (baseline) which only + # changed BUILD.bazel. + self.assertTrue(len(config_change.get_qualified_commits()) == 0) + @staticmethod def __get_a_gen_config( googleapis_commitish="", libraries: list[LibraryConfig] = None From 69715be4bae1ccc3733b59a9ff254cc145b4fed3 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 26 Mar 2024 18:57:32 +0000 Subject: [PATCH 16/39] change test name --- library_generation/test/model/config_change_unit_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library_generation/test/model/config_change_unit_tests.py b/library_generation/test/model/config_change_unit_tests.py index ef1c0fa693..86d2a9b422 100644 --- a/library_generation/test/model/config_change_unit_tests.py +++ b/library_generation/test/model/config_change_unit_tests.py @@ -170,7 +170,7 @@ def test_get_qualified_commits_success(self): qualified_commits[2].commit.hexsha, ) - def test_get_qualified_commits_build_only_commit_returns_none(self): + def test_get_qualified_commits_build_only_commit_returns_empty_list(self): config_change = ConfigChange( change_to_libraries={}, baseline_config=ConfigChangeTest.__get_a_gen_config( From c032538d017475ee03614d4f2604df6e2fd06359 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Thu, 28 Mar 2024 15:44:33 +0000 Subject: [PATCH 17/39] use meaningful variable for commit sha --- .../test/model/config_change_unit_tests.py | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/library_generation/test/model/config_change_unit_tests.py b/library_generation/test/model/config_change_unit_tests.py index 86d2a9b422..2f42789dc9 100644 --- a/library_generation/test/model/config_change_unit_tests.py +++ b/library_generation/test/model/config_change_unit_tests.py @@ -80,6 +80,8 @@ def test_get_changed_libraries_with_library_level_change_returns_list(self): ) def test_get_changed_libraries_with_mix_changes_returns_list(self): + baseline_commit = "277145d108819fa30fbed3a7cbbb50f91eb6155e" + latest_commit = "8984ddb508dea0e673b724c58338e810b1d8aee3" config_change = ConfigChange( change_to_libraries={ ChangeType.GOOGLEAPIS_COMMIT: [], @@ -99,10 +101,10 @@ def test_get_changed_libraries_with_mix_changes_returns_list(self): ], }, baseline_config=ConfigChangeTest.__get_a_gen_config( - googleapis_commitish="277145d108819fa30fbed3a7cbbb50f91eb6155e" + googleapis_commitish=baseline_commit ), latest_config=ConfigChangeTest.__get_a_gen_config( - googleapis_commitish="8984ddb508dea0e673b724c58338e810b1d8aee3", + googleapis_commitish=latest_commit, libraries=[ ConfigChangeTest.__get_a_library_config( library_name="gke-backup", @@ -120,13 +122,18 @@ def test_get_changed_libraries_with_mix_changes_returns_list(self): ) def test_get_qualified_commits_success(self): + baseline_commit = "277145d108819fa30fbed3a7cbbb50f91eb6155e" + latest_commit = "8984ddb508dea0e673b724c58338e810b1d8aee3" + gke_backup_commit = "b8691edb3f1d3c1583aa9cd89240eb359eebe9c7" + aiplatform_commit = "b82095baef02e525bee7bb1c48911c33b66acdf0" + network_management_commit = "efad09c9f0d46ae0786d810a88024363e06c6ca3" config_change = ConfigChange( change_to_libraries={}, baseline_config=ConfigChangeTest.__get_a_gen_config( - googleapis_commitish="277145d108819fa30fbed3a7cbbb50f91eb6155e" + googleapis_commitish=baseline_commit ), latest_config=ConfigChangeTest.__get_a_gen_config( - googleapis_commitish="8984ddb508dea0e673b724c58338e810b1d8aee3", + googleapis_commitish=latest_commit, libraries=[ ConfigChangeTest.__get_a_library_config( library_name="gke-backup", @@ -156,28 +163,30 @@ def test_get_qualified_commits_success(self): self.assertEqual(3, len(qualified_commits)) self.assertEqual({"gke-backup"}, qualified_commits[0].libraries) self.assertEqual( - "b8691edb3f1d3c1583aa9cd89240eb359eebe9c7", + gke_backup_commit, qualified_commits[0].commit.hexsha, ) self.assertEqual({"aiplatform"}, qualified_commits[1].libraries) self.assertEqual( - "b82095baef02e525bee7bb1c48911c33b66acdf0", + aiplatform_commit, qualified_commits[1].commit.hexsha, ) self.assertEqual({"network-management"}, qualified_commits[2].libraries) self.assertEqual( - "efad09c9f0d46ae0786d810a88024363e06c6ca3", + network_management_commit, qualified_commits[2].commit.hexsha, ) def test_get_qualified_commits_build_only_commit_returns_empty_list(self): + baseline_commit = "bdda0174f68a738518ec311e05e6fd9bbe19cd78" + latest_commit = "c9a5050ef225b0011603e1109cf53ab1de0a8e53" config_change = ConfigChange( change_to_libraries={}, baseline_config=ConfigChangeTest.__get_a_gen_config( - googleapis_commitish="bdda0174f68a738518ec311e05e6fd9bbe19cd78" + googleapis_commitish=baseline_commit ), latest_config=ConfigChangeTest.__get_a_gen_config( - googleapis_commitish="c9a5050ef225b0011603e1109cf53ab1de0a8e53", + googleapis_commitish=latest_commit, libraries=[ ConfigChangeTest.__get_a_library_config( library_name="chat", @@ -186,7 +195,7 @@ def test_get_qualified_commits_build_only_commit_returns_empty_list(self): ], ), ) - # one commit between c9a5050 (latest) and c9a5050 (baseline) which only + # one commit between latest_commit and baseline_commit which only # changed BUILD.bazel. self.assertTrue(len(config_change.get_qualified_commits()) == 0) From fc6fa0bc84a13dd6a06b8e5e4ea64196fe047d44 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Thu, 28 Mar 2024 15:51:15 +0000 Subject: [PATCH 18/39] use variable rather than none --- library_generation/test/model/config_change_unit_tests.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/library_generation/test/model/config_change_unit_tests.py b/library_generation/test/model/config_change_unit_tests.py index 2f42789dc9..fa6b270c7f 100644 --- a/library_generation/test/model/config_change_unit_tests.py +++ b/library_generation/test/model/config_change_unit_tests.py @@ -22,6 +22,8 @@ class ConfigChangeTest(unittest.TestCase): + ALL_LIBRARIES_CHANGED = None + def test_get_changed_libraries_with_repo_level_change_returns_none(self): config_change = ConfigChange( change_to_libraries={ @@ -39,7 +41,10 @@ def test_get_changed_libraries_with_repo_level_change_returns_none(self): baseline_config=ConfigChangeTest.__get_a_gen_config(), latest_config=ConfigChangeTest.__get_a_gen_config(), ) - self.assertIsNone(config_change.get_changed_libraries()) + self.assertEquals( + ConfigChangeTest.ALL_LIBRARIES_CHANGED, + config_change.get_changed_libraries(), + ) def test_get_changed_libraries_with_library_level_change_returns_list(self): config_change = ConfigChange( From 3bed10a5d61cc7436442b7b6f697a8b58cba04ba Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Thu, 28 Mar 2024 15:52:40 +0000 Subject: [PATCH 19/39] change helper method name --- library_generation/model/config_change.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library_generation/model/config_change.py b/library_generation/model/config_change.py index 109c646a20..cf9a26679d 100644 --- a/library_generation/model/config_change.py +++ b/library_generation/model/config_change.py @@ -81,7 +81,7 @@ def get_changed_libraries(self) -> Optional[list[str]]: library_names = [] for change_type, library_changes in self.change_to_libraries.items(): if change_type == ChangeType.GOOGLEAPIS_COMMIT: - library_names.extend(self.__get_library_name_from_qualified_commits()) + library_names.extend(self.__get_library_names_from_qualified_commits()) else: library_names.extend( [library_change.library_name for library_change in library_changes] @@ -121,7 +121,7 @@ def get_qualified_commits( shutil.rmtree(tmp_dir, ignore_errors=True) return qualified_commits - def __get_library_name_from_qualified_commits(self) -> list[str]: + def __get_library_names_from_qualified_commits(self) -> list[str]: qualified_commits = self.get_qualified_commits() library_names = [] for qualified_commit in qualified_commits: From 50c1fa67a72663025bb4879c665b1f85220c8726 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Thu, 28 Mar 2024 15:57:38 +0000 Subject: [PATCH 20/39] add method comment --- library_generation/model/config_change.py | 7 +++++-- .../test/model/config_change_unit_tests.py | 10 +++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/library_generation/model/config_change.py b/library_generation/model/config_change.py index cf9a26679d..f095da688a 100644 --- a/library_generation/model/config_change.py +++ b/library_generation/model/config_change.py @@ -60,6 +60,8 @@ def __init__(self, commit: Commit, libraries: set[str]): class ConfigChange: + ALL_LIBRARIES_CHANGED = None + def __init__( self, change_to_libraries: dict[ChangeType, list[LibraryChange]], @@ -73,11 +75,12 @@ def __init__( def get_changed_libraries(self) -> Optional[list[str]]: """ Returns library name of changed libraries. - None if there is a repository level change. + None if there is a repository level change, which means all libraries + in the latest_config will be generated. :return: library names of change libraries. """ if ChangeType.REPO_LEVEL_CHANGE in self.change_to_libraries: - return None + return ConfigChange.ALL_LIBRARIES_CHANGED library_names = [] for change_type, library_changes in self.change_to_libraries.items(): if change_type == ChangeType.GOOGLEAPIS_COMMIT: diff --git a/library_generation/test/model/config_change_unit_tests.py b/library_generation/test/model/config_change_unit_tests.py index fa6b270c7f..9261dc6bf9 100644 --- a/library_generation/test/model/config_change_unit_tests.py +++ b/library_generation/test/model/config_change_unit_tests.py @@ -22,9 +22,9 @@ class ConfigChangeTest(unittest.TestCase): - ALL_LIBRARIES_CHANGED = None - - def test_get_changed_libraries_with_repo_level_change_returns_none(self): + def test_get_changed_libraries_with_repo_level_change_returns_all_libraries_changed( + self, + ): config_change = ConfigChange( change_to_libraries={ ChangeType.REPO_LEVEL_CHANGE: [], @@ -41,8 +41,8 @@ def test_get_changed_libraries_with_repo_level_change_returns_none(self): baseline_config=ConfigChangeTest.__get_a_gen_config(), latest_config=ConfigChangeTest.__get_a_gen_config(), ) - self.assertEquals( - ConfigChangeTest.ALL_LIBRARIES_CHANGED, + self.assertEqual( + ConfigChange.ALL_LIBRARIES_CHANGED, config_change.get_changed_libraries(), ) From 081926676229fd24fb9abb81d26ad36cb8cde4cb Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Thu, 28 Mar 2024 16:11:22 +0000 Subject: [PATCH 21/39] return unique and sored library names --- library_generation/model/config_change.py | 10 +++---- library_generation/owlbot/bin/entrypoint.sh | 2 +- .../test/model/config_change_unit_tests.py | 26 ++++++++++++++++++- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/library_generation/model/config_change.py b/library_generation/model/config_change.py index f095da688a..4bde2566a5 100644 --- a/library_generation/model/config_change.py +++ b/library_generation/model/config_change.py @@ -74,22 +74,22 @@ def __init__( def get_changed_libraries(self) -> Optional[list[str]]: """ - Returns library name of changed libraries. + Returns a unique, sorted list of library name of changed libraries. None if there is a repository level change, which means all libraries in the latest_config will be generated. :return: library names of change libraries. """ if ChangeType.REPO_LEVEL_CHANGE in self.change_to_libraries: return ConfigChange.ALL_LIBRARIES_CHANGED - library_names = [] + library_names = set() for change_type, library_changes in self.change_to_libraries.items(): if change_type == ChangeType.GOOGLEAPIS_COMMIT: - library_names.extend(self.__get_library_names_from_qualified_commits()) + library_names.update(self.__get_library_names_from_qualified_commits()) else: - library_names.extend( + library_names.update( [library_change.library_name for library_change in library_changes] ) - return library_names + return sorted(list(library_names)) def get_qualified_commits( self, diff --git a/library_generation/owlbot/bin/entrypoint.sh b/library_generation/owlbot/bin/entrypoint.sh index e7eb91c179..94c2caea57 100755 --- a/library_generation/owlbot/bin/entrypoint.sh +++ b/library_generation/owlbot/bin/entrypoint.sh @@ -81,5 +81,5 @@ echo "...done" # ensure formatting on all .java files in the repository echo "Reformatting source..." -mvn fmt:format -V --batch-mode --no-transfer-progress +~/apache-maven-3.9.6/bin/mvn fmt:format -V --batch-mode --no-transfer-progress echo "...done" diff --git a/library_generation/test/model/config_change_unit_tests.py b/library_generation/test/model/config_change_unit_tests.py index 9261dc6bf9..b64884149e 100644 --- a/library_generation/test/model/config_change_unit_tests.py +++ b/library_generation/test/model/config_change_unit_tests.py @@ -80,7 +80,31 @@ def test_get_changed_libraries_with_library_level_change_returns_list(self): latest_config=ConfigChangeTest.__get_a_gen_config(), ) self.assertEqual( - ["a-library", "another-library", "new-library", "library-with-new-version"], + ["a-library", "another-library", "library-with-new-version", "new-library"], + config_change.get_changed_libraries(), + ) + + def test_get_changed_libraries_with_duplicated_library_name_returns_unique_name(self): + config_change = ConfigChange( + change_to_libraries={ + ChangeType.LIBRARY_LEVEL_CHANGE: [ + LibraryChange( + changed_param="a-param", + latest_value="new_test", + library_name="a-library", + ), + LibraryChange( + changed_param="another-param", + latest_value="new_value", + library_name="a-library", + ), + ], + }, + baseline_config=ConfigChangeTest.__get_a_gen_config(), + latest_config=ConfigChangeTest.__get_a_gen_config(), + ) + self.assertEqual( + ["a-library"], config_change.get_changed_libraries(), ) From 4848d959e3d79f224529eded0ea179cdbd74f547 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Thu, 28 Mar 2024 16:14:46 +0000 Subject: [PATCH 22/39] add comment --- library_generation/model/config_change.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library_generation/model/config_change.py b/library_generation/model/config_change.py index 4bde2566a5..5770443dfd 100644 --- a/library_generation/model/config_change.py +++ b/library_generation/model/config_change.py @@ -138,9 +138,9 @@ def __create_qualified_commit( """ Returns a qualified commit from the given Commit object; otherwise None. - :param proto_paths: - :param commit: - :return: + :param proto_paths: a mapping from versioned proto_path to library_name + :param commit: a GitHub commit object. + :return: qualified commits. """ libraries = set() for file in commit.stats.files.keys(): From 1094041b0defe6ee86746f51fe77c2e003fe4a8b Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Thu, 28 Mar 2024 16:15:37 +0000 Subject: [PATCH 23/39] restore owlbot --- library_generation/owlbot/bin/entrypoint.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library_generation/owlbot/bin/entrypoint.sh b/library_generation/owlbot/bin/entrypoint.sh index 94c2caea57..e7eb91c179 100755 --- a/library_generation/owlbot/bin/entrypoint.sh +++ b/library_generation/owlbot/bin/entrypoint.sh @@ -81,5 +81,5 @@ echo "...done" # ensure formatting on all .java files in the repository echo "Reformatting source..." -~/apache-maven-3.9.6/bin/mvn fmt:format -V --batch-mode --no-transfer-progress +mvn fmt:format -V --batch-mode --no-transfer-progress echo "...done" From d37a2b4d3c4198d493fd5fa855ca15ce6cd47f7c Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Thu, 28 Mar 2024 16:17:16 +0000 Subject: [PATCH 24/39] format code --- library_generation/test/model/config_change_unit_tests.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library_generation/test/model/config_change_unit_tests.py b/library_generation/test/model/config_change_unit_tests.py index b64884149e..d6309c98c1 100644 --- a/library_generation/test/model/config_change_unit_tests.py +++ b/library_generation/test/model/config_change_unit_tests.py @@ -84,7 +84,9 @@ def test_get_changed_libraries_with_library_level_change_returns_list(self): config_change.get_changed_libraries(), ) - def test_get_changed_libraries_with_duplicated_library_name_returns_unique_name(self): + def test_get_changed_libraries_with_duplicated_library_name_returns_unique_name( + self, + ): config_change = ConfigChange( change_to_libraries={ ChangeType.LIBRARY_LEVEL_CHANGE: [ From 93d5c1055e4e9c7f445d03b3c563d9a2d2209d36 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Thu, 28 Mar 2024 16:59:43 +0000 Subject: [PATCH 25/39] change param name --- .../test/utils/proto_path_utils_unit_tests.py | 15 ++++++--------- library_generation/utils/proto_path_utils.py | 14 +++++++------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/library_generation/test/utils/proto_path_utils_unit_tests.py b/library_generation/test/utils/proto_path_utils_unit_tests.py index 6ff13f8080..2bfecd4621 100644 --- a/library_generation/test/utils/proto_path_utils_unit_tests.py +++ b/library_generation/test/utils/proto_path_utils_unit_tests.py @@ -43,22 +43,19 @@ def test_get_proto_paths_from_yaml_success(self): ) def test_find_versioned_proto_path_nested_version_success(self): - file_path = "google/cloud/aiplatform/v1/schema/predict/params/image_classification.proto" + proto_path = "google/cloud/aiplatform/v1/schema/predict/params/image_classification.proto" expected = "google/cloud/aiplatform/v1" - proto_path = find_versioned_proto_path(file_path) - self.assertEqual(expected, proto_path) + self.assertEqual(expected, find_versioned_proto_path(proto_path)) def test_find_versioned_proto_path_success(self): - file_path = "google/cloud/asset/v1p2beta1/assets.proto" + proto_path = "google/cloud/asset/v1p2beta1/assets.proto" expected = "google/cloud/asset/v1p2beta1" - proto_path = find_versioned_proto_path(file_path) - self.assertEqual(expected, proto_path) + self.assertEqual(expected, find_versioned_proto_path(proto_path)) def test_find_versioned_proto_without_version_return_itself(self): - file_path = "google/type/color.proto" + proto_path = "google/type/color.proto" expected = "google/type/color.proto" - proto_path = find_versioned_proto_path(file_path) - self.assertEqual(expected, proto_path) + self.assertEqual(expected, find_versioned_proto_path(proto_path)) def test_remove_version_from_returns_non_versioned_path(self): proto_path = "google/cloud/aiplatform/v1" diff --git a/library_generation/utils/proto_path_utils.py b/library_generation/utils/proto_path_utils.py index 1ee26dc93d..ce387d15b0 100644 --- a/library_generation/utils/proto_path_utils.py +++ b/library_generation/utils/proto_path_utils.py @@ -43,19 +43,19 @@ def get_proto_paths(config: GenerationConfig) -> dict[str, str]: return paths -def find_versioned_proto_path(file_path: str) -> str: +def find_versioned_proto_path(proto_path: str) -> str: """ - Returns a versioned proto_path from a given file_path; or file_path itself + Returns a versioned proto_path from a given proto_path; or proto_path itself if it doesn't contain a versioned proto_path. - :param file_path: a proto file path + :param proto_path: a proto file path :return: the versioned proto_path """ version_regex = re.compile(r"^v[1-9].*") - directories = file_path.split("/") + directories = proto_path.split("/") for directory in directories: result = version_regex.search(directory) if result: version = result[0] - idx = file_path.find(version) - return file_path[:idx] + version - return file_path + idx = proto_path.find(version) + return proto_path[:idx] + version + return proto_path From df5f310a6c59e0e69f0a10d1d493dbe0da1d28c6 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Thu, 28 Mar 2024 17:04:34 +0000 Subject: [PATCH 26/39] use util to get output dir --- library_generation/model/config_change.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library_generation/model/config_change.py b/library_generation/model/config_change.py index 5770443dfd..837693b053 100644 --- a/library_generation/model/config_change.py +++ b/library_generation/model/config_change.py @@ -18,6 +18,7 @@ from git import Commit, Repo from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig +from library_generation.utilities import sh_util from library_generation.utils.proto_path_utils import find_versioned_proto_path from library_generation.utils.proto_path_utils import get_proto_paths @@ -103,7 +104,7 @@ def get_qualified_commits( :param repo_url: the repository contains the commit history. :return: QualifiedCommit objects. """ - tmp_dir = "/tmp/repo" + tmp_dir = sh_util("get_output_folder") shutil.rmtree(tmp_dir, ignore_errors=True) os.mkdir(tmp_dir) # we only need commit history, thus shadow clone is enough. From f140c2bc80d2655d685d9faf9d62cb4c97bbc72e Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Mon, 1 Apr 2024 21:28:19 +0000 Subject: [PATCH 27/39] refactor get_proto_path_to_library_name to generation_config --- library_generation/model/config_change.py | 3 +- library_generation/model/generation_config.py | 12 ++++++ .../test/model/generation_config_unit_test.py | 39 +++++++++++++++++++ .../test/utils/proto_path_utils_unit_tests.py | 16 -------- library_generation/utils/proto_path_utils.py | 14 ------- 5 files changed, 52 insertions(+), 32 deletions(-) create mode 100644 library_generation/test/model/generation_config_unit_test.py diff --git a/library_generation/model/config_change.py b/library_generation/model/config_change.py index 837693b053..b5f240f55b 100644 --- a/library_generation/model/config_change.py +++ b/library_generation/model/config_change.py @@ -20,7 +20,6 @@ from library_generation.model.library_config import LibraryConfig from library_generation.utilities import sh_util from library_generation.utils.proto_path_utils import find_versioned_proto_path -from library_generation.utils.proto_path_utils import get_proto_paths class ChangeType(Enum): @@ -110,7 +109,7 @@ def get_qualified_commits( # we only need commit history, thus shadow clone is enough. repo = Repo.clone_from(url=repo_url, to_path=tmp_dir, filter=["blob:none"]) commit = repo.commit(self.latest_config.googleapis_commitish) - proto_paths = get_proto_paths(self.latest_config) + proto_paths = self.latest_config.get_proto_path_to_library_name() qualified_commits = [] while str(commit.hexsha) != self.baseline_config.googleapis_commitish: qualified_commit = ConfigChange.__create_qualified_commit( diff --git a/library_generation/model/generation_config.py b/library_generation/model/generation_config.py index 22823b903d..b37b24b5cb 100644 --- a/library_generation/model/generation_config.py +++ b/library_generation/model/generation_config.py @@ -49,6 +49,18 @@ def __init__( # monorepos have more than one library defined in the config yaml self.is_monorepo = len(libraries) > 1 + def get_proto_path_to_library_name(self) -> dict[str, str]: + """ + Get versioned proto_path to library_name mapping from configuration. + + :return: versioned proto_path to library_name mapping + """ + paths = {} + for library in self.libraries: + for gapic_config in library.gapic_configs: + paths[gapic_config.proto_path] = library.get_library_name() + return paths + def from_yaml(path_to_yaml: str) -> GenerationConfig: """ diff --git a/library_generation/test/model/generation_config_unit_test.py b/library_generation/test/model/generation_config_unit_test.py new file mode 100644 index 0000000000..b056c70979 --- /dev/null +++ b/library_generation/test/model/generation_config_unit_test.py @@ -0,0 +1,39 @@ +# Copyright 2024 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import os +import unittest +from pathlib import Path + +from library_generation.model.generation_config import from_yaml + +script_dir = os.path.dirname(os.path.realpath(__file__)) +resources_dir = os.path.join(script_dir, "..", "resources") +test_config_dir = Path(os.path.join(resources_dir, "test-config")).resolve() + + +class GenerationConfigTest(unittest.TestCase): + def test_get_proto_path_to_library_name_success(self): + paths = from_yaml( + f"{test_config_dir}/generation_config.yaml" + ).get_proto_path_to_library_name() + self.assertEqual( + { + "google/cloud/asset/v1": "asset", + "google/cloud/asset/v1p1beta1": "asset", + "google/cloud/asset/v1p2beta1": "asset", + "google/cloud/asset/v1p5beta1": "asset", + "google/cloud/asset/v1p7beta1": "asset", + }, + paths, + ) diff --git a/library_generation/test/utils/proto_path_utils_unit_tests.py b/library_generation/test/utils/proto_path_utils_unit_tests.py index 2bfecd4621..2e23c2b403 100644 --- a/library_generation/test/utils/proto_path_utils_unit_tests.py +++ b/library_generation/test/utils/proto_path_utils_unit_tests.py @@ -15,10 +15,7 @@ import os import unittest from pathlib import Path - -from library_generation.model.generation_config import from_yaml from library_generation.utils.proto_path_utils import ( - get_proto_paths, find_versioned_proto_path, remove_version_from, ) @@ -29,19 +26,6 @@ class ProtoPathsUtilsTest(unittest.TestCase): - def test_get_proto_paths_from_yaml_success(self): - paths = get_proto_paths(from_yaml(f"{test_config_dir}/generation_config.yaml")) - self.assertEqual( - { - "google/cloud/asset/v1": "asset", - "google/cloud/asset/v1p1beta1": "asset", - "google/cloud/asset/v1p2beta1": "asset", - "google/cloud/asset/v1p5beta1": "asset", - "google/cloud/asset/v1p7beta1": "asset", - }, - paths, - ) - def test_find_versioned_proto_path_nested_version_success(self): proto_path = "google/cloud/aiplatform/v1/schema/predict/params/image_classification.proto" expected = "google/cloud/aiplatform/v1" diff --git a/library_generation/utils/proto_path_utils.py b/library_generation/utils/proto_path_utils.py index ce387d15b0..d2ae25f602 100644 --- a/library_generation/utils/proto_path_utils.py +++ b/library_generation/utils/proto_path_utils.py @@ -13,7 +13,6 @@ # See the License for the specific language governing permissions and # limitations under the License. import re -from library_generation.model.generation_config import GenerationConfig def remove_version_from(proto_path: str) -> str: @@ -30,19 +29,6 @@ def remove_version_from(proto_path: str) -> str: return proto_path -def get_proto_paths(config: GenerationConfig) -> dict[str, str]: - """ - Get versioned proto_path to library_name mapping from configuration file. - :param config: a GenerationConfig object. - :return: versioned proto_path to library_name mapping - """ - paths = {} - for library in config.libraries: - for gapic_config in library.gapic_configs: - paths[gapic_config.proto_path] = library.get_library_name() - return paths - - def find_versioned_proto_path(proto_path: str) -> str: """ Returns a versioned proto_path from a given proto_path; or proto_path itself From ac8f0a9c9d8560b8d1fcb58eb1d1a34f5f384414 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Mon, 1 Apr 2024 21:33:49 +0000 Subject: [PATCH 28/39] move utilities.py to utils/utilities.py --- library_generation/generate_composed_library.py | 2 +- library_generation/generate_repo.py | 2 +- library_generation/model/config_change.py | 2 +- library_generation/test/compare_poms.py | 2 +- library_generation/test/integration_tests.py | 2 +- library_generation/test/utilities_unit_tests.py | 2 +- library_generation/utils/__init__.py | 0 library_generation/{ => utils}/utilities.py | 2 +- 8 files changed, 7 insertions(+), 7 deletions(-) create mode 100644 library_generation/utils/__init__.py rename library_generation/{ => utils}/utilities.py (99%) diff --git a/library_generation/generate_composed_library.py b/library_generation/generate_composed_library.py index 4caf0c45ff..877757e719 100755 --- a/library_generation/generate_composed_library.py +++ b/library_generation/generate_composed_library.py @@ -30,7 +30,7 @@ import os from pathlib import Path from typing import List -import library_generation.utilities as util +import library_generation.utils.utilities as util from library_generation.model.generation_config import GenerationConfig from library_generation.model.gapic_config import GapicConfig from library_generation.model.gapic_inputs import GapicInputs diff --git a/library_generation/generate_repo.py b/library_generation/generate_repo.py index a4a4a826f2..9af3a08c56 100755 --- a/library_generation/generate_repo.py +++ b/library_generation/generate_repo.py @@ -13,7 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -import library_generation.utilities as util +import library_generation.utils.utilities as util import click import os from library_generation.generate_composed_library import generate_composed_library diff --git a/library_generation/model/config_change.py b/library_generation/model/config_change.py index b5f240f55b..5b1a97d22a 100644 --- a/library_generation/model/config_change.py +++ b/library_generation/model/config_change.py @@ -18,7 +18,7 @@ from git import Commit, Repo from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig -from library_generation.utilities import sh_util +from library_generation.utils.utilities import sh_util from library_generation.utils.proto_path_utils import find_versioned_proto_path diff --git a/library_generation/test/compare_poms.py b/library_generation/test/compare_poms.py index f58953505d..a1ef443799 100644 --- a/library_generation/test/compare_poms.py +++ b/library_generation/test/compare_poms.py @@ -4,7 +4,7 @@ The only comparison points are: element path (e.g. project/dependencies) and element text There is a special case for `dependency`, where the maven coordinates are prepared as well """ -from library_generation.utilities import eprint +from library_generation.utils.utilities import eprint import xml.etree.ElementTree as et from collections import Counter import sys diff --git a/library_generation/test/integration_tests.py b/library_generation/test/integration_tests.py index 35cbf565ac..9c1d84a33c 100755 --- a/library_generation/test/integration_tests.py +++ b/library_generation/test/integration_tests.py @@ -28,7 +28,7 @@ from library_generation.generate_repo import generate_from_yaml from library_generation.model.generation_config import from_yaml, GenerationConfig from library_generation.test.compare_poms import compare_xml -from library_generation.utilities import ( +from library_generation.utils.utilities import ( sh_util as shell_call, run_process_and_print_output, ) diff --git a/library_generation/test/utilities_unit_tests.py b/library_generation/test/utilities_unit_tests.py index bb7fded2d3..706570ccc1 100644 --- a/library_generation/test/utilities_unit_tests.py +++ b/library_generation/test/utilities_unit_tests.py @@ -22,7 +22,7 @@ import contextlib from pathlib import Path from parameterized import parameterized -from library_generation import utilities as util +from library_generation.utils 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 diff --git a/library_generation/utils/__init__.py b/library_generation/utils/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/library_generation/utilities.py b/library_generation/utils/utilities.py similarity index 99% rename from library_generation/utilities.py rename to library_generation/utils/utilities.py index 2ec220b6ed..c16fa1884d 100755 --- a/library_generation/utilities.py +++ b/library_generation/utils/utilities.py @@ -65,7 +65,7 @@ def sh_util(statement: str, **kwargs) -> str: kwargs["stderr"] = subprocess.PIPE output = "" with subprocess.Popen( - ["bash", "-exc", f"source {script_dir}/utilities.sh && {statement}"], + ["bash", "-exc", f"source {script_dir}/../utilities.sh && {statement}"], **kwargs, ) as proc: print("command stderr:") From 9dd21a53deefc5e2b9f7add479a4b70dad7a0d9b Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 2 Apr 2024 14:06:24 +0000 Subject: [PATCH 29/39] refactor --- library_generation/generate_pr_description.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py index b5f354b0d7..2af636d2fe 100644 --- a/library_generation/generate_pr_description.py +++ b/library_generation/generate_pr_description.py @@ -21,7 +21,6 @@ from library_generation.model.generation_config import from_yaml from library_generation.utils.proto_path_utils import find_versioned_proto_path from library_generation.utils.commit_message_formatter import format_commit_message -from library_generation.utils.proto_path_utils import get_proto_paths from library_generation.utils.commit_message_formatter import wrap_override_commit @@ -86,7 +85,7 @@ def generate_pr_descriptions( baseline_commit: str, ) -> str: config = from_yaml(generation_config_yaml) - paths = get_proto_paths(config) + paths = config.get_proto_path_to_library_name() return __get_commit_messages( repo_url=repo_url, latest_commit=config.googleapis_commitish, From 8e61a2b44c74ae36e977881e5530d9a0f522e4b2 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 2 Apr 2024 17:08:18 +0000 Subject: [PATCH 30/39] move utilities.sh to utils/utilities.sh --- library_generation/test/generate_library_unit_tests.sh | 2 +- library_generation/utils/utilities.py | 2 +- library_generation/{ => utils}/utilities.sh | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename library_generation/{ => utils}/utilities.sh (100%) diff --git a/library_generation/test/generate_library_unit_tests.sh b/library_generation/test/generate_library_unit_tests.sh index e9f4954298..ac499435c8 100755 --- a/library_generation/test/generate_library_unit_tests.sh +++ b/library_generation/test/generate_library_unit_tests.sh @@ -5,7 +5,7 @@ set -xeo pipefail # Unit tests against ../utilities.sh script_dir=$(dirname "$(readlink -f "$0")") source "${script_dir}"/test_utilities.sh -source "${script_dir}"/../utilities.sh +source "${script_dir}"/../utils/utilities.sh # Unit tests extract_folder_name_test() { diff --git a/library_generation/utils/utilities.py b/library_generation/utils/utilities.py index c16fa1884d..2ec220b6ed 100755 --- a/library_generation/utils/utilities.py +++ b/library_generation/utils/utilities.py @@ -65,7 +65,7 @@ def sh_util(statement: str, **kwargs) -> str: kwargs["stderr"] = subprocess.PIPE output = "" with subprocess.Popen( - ["bash", "-exc", f"source {script_dir}/../utilities.sh && {statement}"], + ["bash", "-exc", f"source {script_dir}/utilities.sh && {statement}"], **kwargs, ) as proc: print("command stderr:") diff --git a/library_generation/utilities.sh b/library_generation/utils/utilities.sh similarity index 100% rename from library_generation/utilities.sh rename to library_generation/utils/utilities.sh From 067d06d2bd620729834c03eb8bc33a5f8e0645f8 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 2 Apr 2024 17:12:38 +0000 Subject: [PATCH 31/39] refactor --- library_generation/generate_library.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library_generation/generate_library.sh b/library_generation/generate_library.sh index a23cf14653..a5d73ebec4 100755 --- a/library_generation/generate_library.sh +++ b/library_generation/generate_library.sh @@ -74,7 +74,7 @@ done script_dir=$(dirname "$(readlink -f "$0")") # source utility functions -source "${script_dir}"/utilities.sh +source "${script_dir}"/utils/utilities.sh output_folder="$(get_output_folder)" if [ -z "${gapic_generator_version}" ]; then From 7d7ac09944400d426340c5214515be544293978f Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 2 Apr 2024 17:17:07 +0000 Subject: [PATCH 32/39] refactor --- library_generation/postprocess_library.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library_generation/postprocess_library.sh b/library_generation/postprocess_library.sh index c6a5b4020d..480b7e5170 100755 --- a/library_generation/postprocess_library.sh +++ b/library_generation/postprocess_library.sh @@ -34,7 +34,7 @@ synthtool_commitish=$6 is_monorepo=$7 configuration_yaml_path=$8 -source "${scripts_root}"/utilities.sh +source "${scripts_root}"/utils/utilities.sh declare -a required_inputs=("postprocessing_target" "versions_file" "owlbot_cli_image_sha" "synthtool_commitish" "is_monorepo") for required_input in "${required_inputs[@]}"; do From 37364d6c4028694f83664c25c71b20fb1b576b61 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 2 Apr 2024 17:26:04 +0000 Subject: [PATCH 33/39] refactor --- showcase/scripts/generate_showcase.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/showcase/scripts/generate_showcase.sh b/showcase/scripts/generate_showcase.sh index 1c1b1f58de..d524aaa77f 100755 --- a/showcase/scripts/generate_showcase.sh +++ b/showcase/scripts/generate_showcase.sh @@ -8,7 +8,7 @@ set -ex readonly SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) lib_gen_scripts_dir="${SCRIPT_DIR}/../../library_generation/" source "${lib_gen_scripts_dir}/test/test_utilities.sh" -source "${lib_gen_scripts_dir}/utilities.sh" +source "${lib_gen_scripts_dir}/utils/utilities.sh" readonly perform_cleanup=$1 cd "${SCRIPT_DIR}" From 3e7bed8775c82933da2cbc407f88bbbeef12fda6 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 2 Apr 2024 17:43:26 +0000 Subject: [PATCH 34/39] refactor --- library_generation/utils/utilities.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library_generation/utils/utilities.py b/library_generation/utils/utilities.py index 2ec220b6ed..8d0ca48dba 100755 --- a/library_generation/utils/utilities.py +++ b/library_generation/utils/utilities.py @@ -57,7 +57,7 @@ def run_process_and_print_output(arguments: List[str], job_name: str = "Job"): def sh_util(statement: str, **kwargs) -> str: """ - Calls a function defined in library_generation/utilities.sh + Calls a function defined in library_generation/utils/utilities.sh """ if "stdout" not in kwargs: kwargs["stdout"] = subprocess.PIPE From c5d5540f505a65695d35e1bd02c7ad1b1ba7891b Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 2 Apr 2024 18:09:51 +0000 Subject: [PATCH 35/39] use file path --- library_generation/utils/utilities.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library_generation/utils/utilities.py b/library_generation/utils/utilities.py index 8d0ca48dba..785600d4c3 100755 --- a/library_generation/utils/utilities.py +++ b/library_generation/utils/utilities.py @@ -65,7 +65,7 @@ def sh_util(statement: str, **kwargs) -> str: kwargs["stderr"] = subprocess.PIPE output = "" with subprocess.Popen( - ["bash", "-exc", f"source {script_dir}/utilities.sh && {statement}"], + ["bash", "-exc", f"source library_generation/utils/utilities.sh && {statement}"], **kwargs, ) as proc: print("command stderr:") From f9bfed69ab34b148e819a60f7878dd6fcd189901 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 2 Apr 2024 18:12:54 +0000 Subject: [PATCH 36/39] format --- library_generation/utils/utilities.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/library_generation/utils/utilities.py b/library_generation/utils/utilities.py index 785600d4c3..61c8a1c1c0 100755 --- a/library_generation/utils/utilities.py +++ b/library_generation/utils/utilities.py @@ -65,7 +65,11 @@ def sh_util(statement: str, **kwargs) -> str: kwargs["stderr"] = subprocess.PIPE output = "" with subprocess.Popen( - ["bash", "-exc", f"source library_generation/utils/utilities.sh && {statement}"], + [ + "bash", + "-exc", + f"source library_generation/utils/utilities.sh && {statement}", + ], **kwargs, ) as proc: print("command stderr:") From 4e18fff2a7cf4e7d210f8468c92a065000aa4ef5 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 2 Apr 2024 18:16:49 +0000 Subject: [PATCH 37/39] restore --- library_generation/utils/utilities.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library_generation/utils/utilities.py b/library_generation/utils/utilities.py index 61c8a1c1c0..8ab1a11b7c 100755 --- a/library_generation/utils/utilities.py +++ b/library_generation/utils/utilities.py @@ -68,7 +68,7 @@ def sh_util(statement: str, **kwargs) -> str: [ "bash", "-exc", - f"source library_generation/utils/utilities.sh && {statement}", + f"source {script_dir}/utilities.sh && {statement}", ], **kwargs, ) as proc: From a784d06b069cd7d89a3bcd8963c0e45fd2a361b6 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 2 Apr 2024 18:19:24 +0000 Subject: [PATCH 38/39] change package --- library_generation/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library_generation/setup.py b/library_generation/setup.py index 477346e711..ebe62af801 100755 --- a/library_generation/setup.py +++ b/library_generation/setup.py @@ -12,7 +12,7 @@ }, package_data={ "library_generation": [ - "*.sh", + "**/*.sh", "templates/*.j2", "gapic-generator-java-wrapper", "requirements.*", From cf589883895da3cb129a71e52781ab7ad9df8338 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 2 Apr 2024 18:30:54 +0000 Subject: [PATCH 39/39] explicitly declare shell scripts --- library_generation/setup.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library_generation/setup.py b/library_generation/setup.py index ebe62af801..bd17cc5fcf 100755 --- a/library_generation/setup.py +++ b/library_generation/setup.py @@ -12,7 +12,9 @@ }, package_data={ "library_generation": [ - "**/*.sh", + "generate_library.sh", + "postprocess_library.sh", + "utils/utilities.sh", "templates/*.j2", "gapic-generator-java-wrapper", "requirements.*",