Skip to content

Commit

Permalink
feat: add a CLI tool to validate generation configuration (#2691)
Browse files Browse the repository at this point in the history
In this PR:
- Add a CLI tool to validate generation config.

Downstream libraries, e.g., google-cloud-java, can write a workflow job
like:
```
docker run ...
 python /src/cli/entry_point.py \
 validate-generation-config \
 --generation-config-path=path/to/generation_config.yaml
```
  • Loading branch information
JoeWang1127 authored and lqiu96 committed May 22, 2024
1 parent 243b538 commit 7ae6a40
Show file tree
Hide file tree
Showing 10 changed files with 315 additions and 83 deletions.
24 changes: 24 additions & 0 deletions library_generation/cli/entry_point.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import os
import sys

import click as click
from library_generation.generate_pr_description import generate_pr_descriptions
Expand Down Expand Up @@ -142,5 +143,28 @@ def generate(
)


@main.command()
@click.option(
"--generation-config-path",
required=False,
type=str,
help="""
Absolute or relative path to a generation_config.yaml.
Default to generation_config.yaml in the current working directory.
""",
)
def validate_generation_config(generation_config_path: str) -> None:
"""
Validate the given generation configuration.
"""
if generation_config_path is None:
generation_config_path = "generation_config.yaml"
try:
from_yaml(os.path.abspath(generation_config_path))
except ValueError as err:
print(err)
sys.exit(1)


if __name__ == "__main__":
main()
65 changes: 49 additions & 16 deletions library_generation/model/generation_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,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 yaml
from typing import List, Optional, Dict
from typing import Optional
from library_generation.model.library_config import LibraryConfig
from library_generation.model.gapic_config import GapicConfig

REPO_LEVEL_PARAMETER = "Repo level parameter"
LIBRARY_LEVEL_PARAMETER = "Library level parameter"
GAPIC_LEVEL_PARAMETER = "GAPIC level parameter"


class GenerationConfig:
"""
Expand All @@ -32,8 +34,8 @@ def __init__(
libraries_bom_version: str,
owlbot_cli_image: str,
synthtool_commitish: str,
template_excludes: List[str],
libraries: List[LibraryConfig],
template_excludes: list[str],
libraries: list[LibraryConfig],
grpc_version: Optional[str] = None,
protoc_version: Optional[str] = None,
):
Expand All @@ -46,6 +48,7 @@ def __init__(
self.libraries = libraries
self.grpc_version = grpc_version
self.protoc_version = protoc_version
self.__validate()

def get_proto_path_to_library_name(self) -> dict[str, str]:
"""
Expand All @@ -62,6 +65,19 @@ def get_proto_path_to_library_name(self) -> dict[str, str]:
def is_monorepo(self) -> bool:
return len(self.libraries) > 1

def __validate(self) -> None:
seen_library_names = dict()
for library in self.libraries:
library_name = library.get_library_name()
if library_name in seen_library_names:
raise ValueError(
f"Both {library.name_pretty} and "
f"{seen_library_names.get(library_name)} have the same "
f"library name: {library_name}, please update one of the "
f"library to have a different library name."
)
seen_library_names[library_name] = library.name_pretty


def from_yaml(path_to_yaml: str) -> GenerationConfig:
"""
Expand All @@ -72,15 +88,19 @@ def from_yaml(path_to_yaml: str) -> GenerationConfig:
with open(path_to_yaml, "r") as file_stream:
config = yaml.safe_load(file_stream)

libraries = __required(config, "libraries")
libraries = __required(config, "libraries", REPO_LEVEL_PARAMETER)
if not libraries:
raise ValueError(f"Library is None in {path_to_yaml}.")

parsed_libraries = list()
for library in libraries:
gapics = __required(library, "GAPICs")

parsed_gapics = list()
if not gapics:
raise ValueError(f"GAPICs is None in {library}.")
for gapic in gapics:
proto_path = __required(gapic, "proto_path")
proto_path = __required(gapic, "proto_path", GAPIC_LEVEL_PARAMETER)
new_gapic = GapicConfig(proto_path)
parsed_gapics.append(new_gapic)

Expand Down Expand Up @@ -114,27 +134,40 @@ def from_yaml(path_to_yaml: str) -> GenerationConfig:
parsed_libraries.append(new_library)

parsed_config = GenerationConfig(
gapic_generator_version=__required(config, "gapic_generator_version"),
gapic_generator_version=__required(
config, "gapic_generator_version", REPO_LEVEL_PARAMETER
),
grpc_version=__optional(config, "grpc_version", None),
protoc_version=__optional(config, "protoc_version", None),
googleapis_commitish=__required(config, "googleapis_commitish"),
libraries_bom_version=__required(config, "libraries_bom_version"),
owlbot_cli_image=__required(config, "owlbot_cli_image"),
synthtool_commitish=__required(config, "synthtool_commitish"),
template_excludes=__required(config, "template_excludes"),
googleapis_commitish=__required(
config, "googleapis_commitish", REPO_LEVEL_PARAMETER
),
libraries_bom_version=__required(
config, "libraries_bom_version", REPO_LEVEL_PARAMETER
),
owlbot_cli_image=__required(config, "owlbot_cli_image", REPO_LEVEL_PARAMETER),
synthtool_commitish=__required(
config, "synthtool_commitish", REPO_LEVEL_PARAMETER
),
template_excludes=__required(config, "template_excludes", REPO_LEVEL_PARAMETER),
libraries=parsed_libraries,
)

return parsed_config


def __required(config: Dict, key: str):
def __required(config: dict, key: str, level: str = LIBRARY_LEVEL_PARAMETER):
if key not in config:
raise ValueError(f"required key {key} not found in yaml")
message = (
f"{level}, {key}, is not found in {config} in yaml."
if level != REPO_LEVEL_PARAMETER
else f"{level}, {key}, is not found in yaml."
)
raise ValueError(message)
return config[key]


def __optional(config: Dict, key: str, default: any):
def __optional(config: dict, key: str, default: any):
if key not in config:
return default
return config[key]
35 changes: 34 additions & 1 deletion library_generation/test/cli/entry_point_unit_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,13 @@
# 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 click.testing import CliRunner
from library_generation.cli.entry_point import generate
from library_generation.cli.entry_point import generate, validate_generation_config

script_dir = os.path.dirname(os.path.realpath(__file__))
test_resource_dir = os.path.join(script_dir, "..", "resources", "test-config")


class EntryPointTest(unittest.TestCase):
Expand Down Expand Up @@ -44,3 +48,32 @@ def test_entry_point_with_baseline_without_current_raise_file_exception(self):
"current_generation_config is not specified when "
"baseline_generation_config is specified.",
)

def test_validate_generation_config_succeeds(
self,
):
runner = CliRunner()
# noinspection PyTypeChecker
result = runner.invoke(
validate_generation_config,
[f"--generation-config-path={test_resource_dir}/generation_config.yaml"],
)
self.assertEqual(0, result.exit_code)

def test_validate_generation_config_with_duplicate_library_name_raise_file_exception(
self,
):
runner = CliRunner()
# noinspection PyTypeChecker
result = runner.invoke(
validate_generation_config,
[
f"--generation-config-path={test_resource_dir}/generation_config_with_duplicate_library_name.yaml"
],
)
self.assertEqual(1, result.exit_code)
self.assertEqual(SystemExit, result.exc_info[0])
self.assertRegex(
result.output,
"have the same library name",
)
150 changes: 150 additions & 0 deletions library_generation/test/model/generation_config_unit_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,153 @@ def test_is_monorepo_with_two_libraries_returns_true(self):
libraries=[library_1, library_2],
)
self.assertTrue(config.is_monorepo())

def test_validate_with_duplicate_library_name_raise_exception(self):
self.assertRaisesRegex(
ValueError,
"the same library name",
GenerationConfig,
gapic_generator_version="",
googleapis_commitish="",
libraries_bom_version="",
owlbot_cli_image="",
synthtool_commitish="",
template_excludes=[],
libraries=[
LibraryConfig(
api_shortname="secretmanager",
name_pretty="Secret API",
product_documentation="",
api_description="",
gapic_configs=list(),
),
LibraryConfig(
api_shortname="another-secret",
name_pretty="Another Secret API",
product_documentation="",
api_description="",
gapic_configs=list(),
library_name="secretmanager",
),
],
)

def test_from_yaml_without_gapic_generator_version_raise_exception(self):
self.assertRaisesRegex(
ValueError,
"Repo level parameter, gapic_generator_version",
from_yaml,
f"{test_config_dir}/config_without_generator.yaml",
)

def test_from_yaml_without_googleapis_commitish_raise_exception(self):
self.assertRaisesRegex(
ValueError,
"Repo level parameter, googleapis_commitish",
from_yaml,
f"{test_config_dir}/config_without_googleapis.yaml",
)

def test_from_yaml_without_libraries_bom_version_raise_exception(self):
self.assertRaisesRegex(
ValueError,
"Repo level parameter, libraries_bom_version",
from_yaml,
f"{test_config_dir}/config_without_libraries_bom_version.yaml",
)

def test_from_yaml_without_owlbot_cli_image_raise_exception(self):
self.assertRaisesRegex(
ValueError,
"Repo level parameter, owlbot_cli_image",
from_yaml,
f"{test_config_dir}/config_without_owlbot.yaml",
)

def test_from_yaml_without_synthtool_commitish_raise_exception(self):
self.assertRaisesRegex(
ValueError,
"Repo level parameter, synthtool_commitish",
from_yaml,
f"{test_config_dir}/config_without_synthtool.yaml",
)

def test_from_yaml_without_template_excludes_raise_exception(self):
self.assertRaisesRegex(
ValueError,
"Repo level parameter, template_excludes",
from_yaml,
f"{test_config_dir}/config_without_temp_excludes.yaml",
)

def test_from_yaml_without_libraries_raise_exception(self):
self.assertRaisesRegex(
ValueError,
"Repo level parameter, libraries",
from_yaml,
f"{test_config_dir}/config_without_libraries.yaml",
)

def test_from_yaml_without_api_shortname_raise_exception(self):
self.assertRaisesRegex(
ValueError,
"Library level parameter, api_shortname",
from_yaml,
f"{test_config_dir}/config_without_api_shortname.yaml",
)

def test_from_yaml_without_api_description_raise_exception(self):
self.assertRaisesRegex(
ValueError,
r"Library level parameter, api_description.*'api_shortname': 'apigeeconnect'.*",
from_yaml,
f"{test_config_dir}/config_without_api_description.yaml",
)

def test_from_yaml_without_name_pretty_raise_exception(self):
self.assertRaisesRegex(
ValueError,
r"Library level parameter, name_pretty.*'api_shortname': 'apigeeconnect'.*",
from_yaml,
f"{test_config_dir}/config_without_name_pretty.yaml",
)

def test_from_yaml_without_product_documentation_raise_exception(self):
self.assertRaisesRegex(
ValueError,
r"Library level parameter, product_documentation.*'api_shortname': 'apigeeconnect'.*",
from_yaml,
f"{test_config_dir}/config_without_product_docs.yaml",
)

def test_from_yaml_without_gapics_raise_exception(self):
self.assertRaisesRegex(
ValueError,
"Library level parameter, GAPICs.*'api_shortname': 'apigeeconnect'.*",
from_yaml,
f"{test_config_dir}/config_without_gapics_key.yaml",
)

def test_from_yaml_without_proto_path_raise_exception(self):
self.assertRaisesRegex(
ValueError,
"GAPIC level parameter, proto_path",
from_yaml,
f"{test_config_dir}/config_without_proto_path.yaml",
)

def test_from_yaml_with_zero_library_raise_exception(self):
self.assertRaisesRegex(
ValueError,
"Library is None",
from_yaml,
f"{test_config_dir}/config_without_library_value.yaml",
)

def test_from_yaml_with_zero_proto_path_raise_exception(self):
self.assertRaisesRegex(
ValueError,
r"GAPICs is None in.*'api_shortname': 'apigeeconnect'.*",
from_yaml,
f"{test_config_dir}/config_without_gapics_value.yaml",
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
gapic_generator_version: 2.34.0
libraries:
- api_shortname: apigeeconnect
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
gapic_generator_version: 2.34.0
libraries:
- api_shortname: apigeeconnect
GAPICs:
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
gapic_generator_version: 2.34.0
libraries:
random_key:
Loading

0 comments on commit 7ae6a40

Please sign in to comment.