Skip to content

Commit

Permalink
chore: remove is_monorepo and path_to_yaml from GenerationConfig (
Browse files Browse the repository at this point in the history
#2634)

In this PR:
- Ronvert `is_monorepo` to a method in `GenerationConfig`.
- Remove `path_to_yaml` from `GenerationConfig`.

These two parameters are not used for library generation. Therefore,
they should not be a parameter in `GenerationConfig`.
  • Loading branch information
JoeWang1127 authored Apr 12, 2024
1 parent cff2f87 commit ad641e5
Show file tree
Hide file tree
Showing 12 changed files with 64 additions and 22 deletions.
2 changes: 2 additions & 0 deletions library_generation/cli/entry_point.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ def generate(
# baseline_generation_config is not specified.
# Do not generate pull request description.
generate_from_yaml(
config_path=current_generation_config,
config=from_yaml(current_generation_config),
repository_path=repository_path,
)
Expand All @@ -126,6 +127,7 @@ def generate(
current_config=from_yaml(current_generation_config),
)
generate_from_yaml(
config_path=current_generation_config,
config=config_change.current_config,
repository_path=repository_path,
target_library_names=config_change.get_changed_libraries(),
Expand Down
7 changes: 5 additions & 2 deletions library_generation/generate_composed_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@


def generate_composed_library(
config_path: str,
config: GenerationConfig,
library_path: str,
library: LibraryConfig,
Expand All @@ -49,6 +50,8 @@ def generate_composed_library(
) -> None:
"""
Generate libraries composed of more than one service or service version
:param config_path: Path to generation configuration.
:param config: a GenerationConfig object representing a parsed configuration
yaml
:param library_path: the path to which the generated file goes
Expand Down Expand Up @@ -111,8 +114,8 @@ def generate_composed_library(
owlbot_cli_source_folder,
config.owlbot_cli_image,
config.synthtool_commitish,
str(config.is_monorepo).lower(),
config.path_to_yaml,
str(config.is_monorepo()).lower(),
config_path,
],
"Library postprocessing",
)
Expand Down
2 changes: 1 addition & 1 deletion library_generation/generate_pr_description.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def generate_pr_descriptions(
current_commit=config.googleapis_commitish,
baseline_commit=baseline_commit,
paths=paths,
is_monorepo=config.is_monorepo,
is_monorepo=config.is_monorepo(),
)

description_file = f"{description_path}/pr_description.txt"
Expand Down
6 changes: 5 additions & 1 deletion library_generation/generate_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ def generate(
):
config = from_yaml(generation_config_yaml)
generate_from_yaml(
config_path=generation_config_yaml,
config=config,
repository_path=repository_path,
target_library_names=target_library_names.split(",")
Expand All @@ -83,6 +84,7 @@ def generate(


def generate_from_yaml(
config_path: str,
config: GenerationConfig,
repository_path: str,
target_library_names: list[str] = None,
Expand All @@ -91,6 +93,7 @@ def generate_from_yaml(
Based on the generation config, generates libraries via
generate_composed_library.py
:param config_path: Path to generation configuration.
:param config: a GenerationConfig object.
:param repository_path: The repository path to which the generated files
will be sent.
Expand All @@ -111,6 +114,7 @@ def generate_from_yaml(
print(f"generating library {library.get_library_name()}")

generate_composed_library(
config_path=config_path,
config=config,
library_path=library_path,
library=library,
Expand All @@ -119,7 +123,7 @@ def generate_from_yaml(
)

# we skip monorepo_postprocessing if not in a monorepo
if not config.is_monorepo:
if not config.is_monorepo():
return

monorepo_postprocessing(
Expand Down
8 changes: 3 additions & 5 deletions library_generation/model/generation_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ def __init__(
owlbot_cli_image: str,
synthtool_commitish: str,
template_excludes: List[str],
path_to_yaml: str,
libraries: List[LibraryConfig],
grpc_version: Optional[str] = None,
protobuf_version: Optional[str] = None,
Expand All @@ -42,12 +41,9 @@ def __init__(
self.owlbot_cli_image = owlbot_cli_image
self.synthtool_commitish = synthtool_commitish
self.template_excludes = template_excludes
self.path_to_yaml = path_to_yaml
self.libraries = libraries
self.grpc_version = grpc_version
self.protobuf_version = protobuf_version
# 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]:
"""
Expand All @@ -61,6 +57,9 @@ def get_proto_path_to_library_name(self) -> dict[str, str]:
paths[gapic_config.proto_path] = library.get_library_name()
return paths

def is_monorepo(self) -> bool:
return len(self.libraries) > 1


def from_yaml(path_to_yaml: str) -> GenerationConfig:
"""
Expand Down Expand Up @@ -120,7 +119,6 @@ def from_yaml(path_to_yaml: str) -> GenerationConfig:
owlbot_cli_image=__required(config, "owlbot_cli_image"),
synthtool_commitish=__required(config, "synthtool_commitish"),
template_excludes=__required(config, "template_excludes"),
path_to_yaml=path_to_yaml,
libraries=parsed_libraries,
)

Expand Down
1 change: 0 additions & 1 deletion library_generation/test/generate_repo_unit_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ def __get_an_empty_generation_config() -> GenerationConfig:
synthtool_commitish="",
owlbot_cli_image="",
template_excludes=[],
path_to_yaml="",
libraries=[],
)

Expand Down
8 changes: 4 additions & 4 deletions library_generation/test/integration_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,14 @@ def test_entry_point_running_in_container(self):
)
for library_name in library_names:
actual_library = (
f"{repo_dest}/{library_name}" if config.is_monorepo else repo_dest
f"{repo_dest}/{library_name}" if config.is_monorepo() else repo_dest
)
print("*" * 50)
print(f"Checking for differences in '{library_name}'.")
print(f" The expected library is in {golden_dir}/{library_name}.")
print(f" The actual library is in {actual_library}. ")
target_repo_dest = (
f"{repo_dest}/{library_name}" if config.is_monorepo else repo_dest
f"{repo_dest}/{library_name}" if config.is_monorepo() else repo_dest
)
compare_result = dircmp(
f"{golden_dir}/{library_name}",
Expand Down Expand Up @@ -138,7 +138,7 @@ def test_entry_point_running_in_container(self):
)
print(" .repo-metadata.json comparison succeed.")

if not config.is_monorepo:
if not config.is_monorepo():
continue

# compare gapic-libraries-bom/pom.xml and pom.xml
Expand Down Expand Up @@ -198,7 +198,7 @@ def __prepare_golden_files(
cls, config: GenerationConfig, library_names: list[str], repo_dest: str
):
for library_name in library_names:
if config.is_monorepo:
if config.is_monorepo():
copy_tree(f"{repo_dest}/{library_name}", f"{golden_dir}/{library_name}")
copy_tree(
f"{repo_dest}/gapic-libraries-bom",
Expand Down
1 change: 0 additions & 1 deletion library_generation/test/model/config_change_unit_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ def __get_a_gen_config(
owlbot_cli_image="",
synthtool_commitish="",
template_excludes=[],
path_to_yaml="",
grpc_version="",
protobuf_version="",
libraries=libraries,
Expand Down
40 changes: 39 additions & 1 deletion library_generation/test/model/generation_config_unit_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,28 @@
import unittest
from pathlib import Path

from library_generation.model.generation_config import from_yaml
from library_generation.model.generation_config import from_yaml, GenerationConfig
from library_generation.model.library_config import LibraryConfig

script_dir = os.path.dirname(os.path.realpath(__file__))
resources_dir = os.path.join(script_dir, "..", "resources")
test_config_dir = Path(os.path.join(resources_dir, "test-config")).resolve()

library_1 = LibraryConfig(
api_shortname="a_library",
api_description="",
name_pretty="",
product_documentation="",
gapic_configs=[],
)
library_2 = LibraryConfig(
api_shortname="another_library",
api_description="",
name_pretty="",
product_documentation="",
gapic_configs=[],
)


class GenerationConfigTest(unittest.TestCase):
def test_get_proto_path_to_library_name_success(self):
Expand All @@ -37,3 +53,25 @@ def test_get_proto_path_to_library_name_success(self):
},
paths,
)

def test_is_monorepo_with_one_library_returns_false(self):
config = GenerationConfig(
gapic_generator_version="",
googleapis_commitish="",
owlbot_cli_image="",
synthtool_commitish="",
template_excludes=[],
libraries=[library_1],
)
self.assertFalse(config.is_monorepo())

def test_is_monorepo_with_two_libraries_returns_true(self):
config = GenerationConfig(
gapic_generator_version="",
googleapis_commitish="",
owlbot_cli_image="",
synthtool_commitish="",
template_excludes=[],
libraries=[library_1, library_2],
)
self.assertTrue(config.is_monorepo())
1 change: 0 additions & 1 deletion library_generation/test/utilities_unit_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,6 @@ def __get_a_gen_config(
"renovate.json",
".gitignore",
],
path_to_yaml=".",
libraries=libraries,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ def setUp(self) -> None:
owlbot_cli_image="",
synthtool_commitish="",
template_excludes=[],
path_to_yaml="",
grpc_version="",
protobuf_version="",
libraries=[self.baseline_library],
Expand All @@ -53,7 +52,6 @@ def setUp(self) -> None:
owlbot_cli_image="",
synthtool_commitish="",
template_excludes=[],
path_to_yaml="",
grpc_version="",
protobuf_version="",
libraries=[self.latest_library],
Expand Down
8 changes: 5 additions & 3 deletions library_generation/utils/utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,9 @@ def prepare_repo(
for library in library_config:
library_name = f"{language}-{library.get_library_name()}"
library_path = (
f"{repo_path}/{library_name}" if gen_config.is_monorepo else f"{repo_path}"
f"{repo_path}/{library_name}"
if gen_config.is_monorepo()
else f"{repo_path}"
)
# use absolute path because docker requires absolute path
# in volume name.
Expand Down Expand Up @@ -215,7 +217,7 @@ def generate_prerequisite_files(
distribution_name_short = re.split(r"[:/]", distribution_name)[-1]
repo = (
"googleapis/google-cloud-java"
if config.is_monorepo
if config.is_monorepo()
else f"googleapis/{language}-{library_name}"
)
api_id = (
Expand Down Expand Up @@ -282,7 +284,7 @@ def generate_prerequisite_files(
owlbot_yaml_file = ".OwlBot.yaml"
path_to_owlbot_yaml_file = (
f"{library_path}/{owlbot_yaml_file}"
if config.is_monorepo
if config.is_monorepo()
else f"{library_path}/.github/{owlbot_yaml_file}"
)
if not os.path.exists(path_to_owlbot_yaml_file):
Expand Down

0 comments on commit ad641e5

Please sign in to comment.