Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add entry point #2616

Merged
merged 34 commits into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
95b66a8
feat: add entry_point.py
JoeWang1127 Apr 2, 2024
964d81a
remove cli in generate_repo.py
JoeWang1127 Apr 2, 2024
5b303cb
remove cli in generate_pr_description.py
JoeWang1127 Apr 2, 2024
4f6cedd
add comment
JoeWang1127 Apr 2, 2024
531244d
fix integration test
JoeWang1127 Apr 2, 2024
a6811d5
rename config
JoeWang1127 Apr 2, 2024
14cdfb9
change pr description file path
JoeWang1127 Apr 2, 2024
24f2070
add comments
JoeWang1127 Apr 3, 2024
853b70c
restore CLI
JoeWang1127 Apr 3, 2024
923f149
format code
JoeWang1127 Apr 3, 2024
1cd3859
change input to current_generation_config
JoeWang1127 Apr 3, 2024
ffc16ee
change to optional inputs
JoeWang1127 Apr 4, 2024
87c7d1c
raise exception if current commit is older than baseline commit
JoeWang1127 Apr 4, 2024
9d723b0
add unit test
JoeWang1127 Apr 4, 2024
fd7897a
add error log
JoeWang1127 Apr 4, 2024
ca902ec
convert time to int
JoeWang1127 Apr 4, 2024
4895971
add comment
JoeWang1127 Apr 4, 2024
87c0bdd
extract helper method
JoeWang1127 Apr 4, 2024
69fd09b
refactor tests
JoeWang1127 Apr 4, 2024
d2b3963
add integration test
JoeWang1127 Apr 4, 2024
b4b907b
change if
JoeWang1127 Apr 4, 2024
ce25efa
refactor
JoeWang1127 Apr 4, 2024
8d1d3f6
restore integration test
JoeWang1127 Apr 4, 2024
e155ea0
add unit tests
JoeWang1127 Apr 4, 2024
8de0c3d
add unit tests
JoeWang1127 Apr 4, 2024
e518d3f
test setup
JoeWang1127 Apr 4, 2024
22eb127
restore unit tests
JoeWang1127 Apr 4, 2024
c25e2bd
refactor
JoeWang1127 Apr 4, 2024
cc137b0
change error message
JoeWang1127 Apr 5, 2024
5dbcd2c
fail if current config is null but baseline is not
JoeWang1127 Apr 5, 2024
3d3522b
two commits should be different
JoeWang1127 Apr 5, 2024
280cee5
change doc
JoeWang1127 Apr 5, 2024
881b557
change comment
JoeWang1127 Apr 5, 2024
4de89de
change readme
JoeWang1127 Apr 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 94 additions & 0 deletions library_generation/cli/entry_point.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
# 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 click as click
from library_generation.generate_pr_description import generate_pr_descriptions
from library_generation.generate_repo import generate_from_yaml
from library_generation.model.generation_config import from_yaml
from library_generation.utils.generation_config_comparator import compare_config


@click.group(invoke_without_command=False)
@click.pass_context
@click.version_option(message="%(version)s")
def main(ctx):
pass


@main.command()
@click.option(
"--baseline-generation-config",
required=True,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make the interface as simple as possible? For example, both of the configs can be optional. We can use the existence of baseline config to decide if we want to to generate PR descriptions, and assume the current config to be exist in the current folder?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If both are optional, then we have:

  1. both are not specified, we assume there's a generation_config.yaml in the current dir and do not generate pr description.
  2. one is specified, we use it to generate repo and do not generate pr description.
  3. both are specified, we use current-generation-config to generate repo and generate pr description.

type=str,
help="""
Path to generation_config.yaml that contains the metadata about
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolute path?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relative path is fine because all path will be converted to absolute path later.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So either relative path or absolute path should work? If that's the case, let's document it here as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

library generation.
The googleapis commit in the configuration is the baseline commit,
exclusively, from which the commit message is considered.
""",
)
@click.option(
"--latest-generation-config",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we all it something more generic?Because theoretically we can compare any two configs, not always compare with the latest.

In addition, what if the latest-generation-config has a googleapis commitish that is prior to it in basedline-generation-config? I don't think we have to support this case, but we should at least handle this case so our script does not blow up, maybe by adding validations that the googleapis commitish in one of them should be higher than the other.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we all it something more generic?Because theoretically we can compare any two configs, not always compare with the latest.

I think these two names are intuitive, do you have other candidates?

what if the latest-generation-config has a googleapis commitish that is prior to it in basedline-generation-config?

The problem is we don't the relationship between two commitish without traversing from one to another. The worst scenario is the description has too many characters (due to lots of commits) and gh command returns error.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We decided to use current_generation_config and baseline_generation_config as parameter names.

The googleapis commitish in current_generation_config should be newer than baseline_generation_config and we'll verify this by commit time.

required=True,
type=str,
help="""
Path to generation_config.yaml that contains the metadata about
library generation.
The googleapis commit in the configuration is the latest commit,
inclusively, to which the commit message is considered.
""",
)
@click.option(
"--repository-path",
type=str,
default=".",
show_default=True,
help="""
The repository path to which the generated files
will be sent.
If not specified, the repository will be generated to the current working
directory.
""",
)
def generate(
baseline_generation_config: str,
latest_generation_config: str,
repository_path: str,
):
# convert paths to absolute paths, so they can be correctly referenced in
# downstream scripts
baseline_generation_config = os.path.abspath(baseline_generation_config)
latest_generation_config = os.path.abspath(latest_generation_config)
repository_path = os.path.abspath(repository_path)
config_change = compare_config(
baseline_config=from_yaml(baseline_generation_config),
latest_config=from_yaml(latest_generation_config),
)
# generate libraries
generate_from_yaml(
config=config_change.latest_config,
repository_path=repository_path,
target_library_names=config_change.get_changed_libraries(),
)
# generate pull request description
generate_pr_descriptions(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want to rename generate_pr_description to something like generate_commit_history, because we just happen to use PR description to capture the commit history, and we could use other ways. It could be a separate PR.

config=config_change.latest_config,
baseline_commit=config_change.baseline_config.googleapis_commitish,
description_path=repository_path,
)


if __name__ == "__main__":
main()
63 changes: 36 additions & 27 deletions library_generation/generate_pr_description.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,10 @@
# limitations under the License.
import os
import shutil
import click as click
from typing import Dict

import click
from git import Commit, Repo
from library_generation.model.generation_config import from_yaml
from library_generation.model.generation_config import GenerationConfig, 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.commit_message_formatter import wrap_override_commit
Expand Down Expand Up @@ -53,47 +52,57 @@ def main(ctx):
This commit should be an ancestor of googleapis commit in configuration.
""",
)
@click.option(
"--repo-url",
type=str,
default="https://github.com/googleapis/googleapis.git",
show_default=True,
help="""
GitHub repository URL.
""",
)
def generate(
generation_config_yaml: str,
repo_url: str,
baseline_commit: str,
) -> str:
description = generate_pr_descriptions(
generation_config_yaml=generation_config_yaml,
repo_url=repo_url,
baseline_commit=baseline_commit,
)
) -> None:
idx = generation_config_yaml.rfind("/")
config_path = generation_config_yaml[:idx]
with open(f"{config_path}/pr_description.txt", "w+") as f:
f.write(description)
return description
generate_pr_descriptions(
config=from_yaml(generation_config_yaml),
baseline_commit=baseline_commit,
description_path=config_path,
)


def generate_pr_descriptions(
generation_config_yaml: str,
repo_url: str,
config: GenerationConfig,
baseline_commit: str,
) -> str:
config = from_yaml(generation_config_yaml)
description_path: str,
repo_url: str = "https://github.com/googleapis/googleapis.git",
) -> None:
"""
Generate pull request description from baseline_commit (exclusive) to the
googleapis commit (inclusive) in the given generation config.

The pull request description will be generated into
description_path/pr_description.txt.

:param config: a GenerationConfig object. The googleapis commit in this
configuration is the latest commit, inclusively, from which the commit
message is considered.
:param baseline_commit: The baseline (oldest) commit, exclusively, from
which the commit message is considered. This commit should be an ancestor
of googleapis commit in configuration.
:param description_path: the path to which the pull request description
file goes.
:param repo_url: the GitHub repository from which retrieves the commit
history.
"""
paths = config.get_proto_path_to_library_name()
return __get_commit_messages(
description = __get_commit_messages(
repo_url=repo_url,
latest_commit=config.googleapis_commitish,
baseline_commit=baseline_commit,
paths=paths,
is_monorepo=config.is_monorepo,
)

description_file = f"{description_path}/pr_description.txt"
print(f"Writing pull request description to {description_file}")
with open(description_file, "w+") as f:
f.write(description)


def __get_commit_messages(
repo_url: str,
Expand Down
35 changes: 13 additions & 22 deletions library_generation/generate_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,10 @@
# 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 click as click
import library_generation.utils.utilities as util
import click
import os
from library_generation.generate_composed_library import generate_composed_library
from library_generation.model.generation_config import GenerationConfig
from library_generation.model.generation_config import from_yaml
from library_generation.model.generation_config import GenerationConfig, from_yaml
from library_generation.model.library_config import LibraryConfig
from library_generation.utils.monorepo_postprocessor import monorepo_postprocessing

Expand Down Expand Up @@ -75,8 +72,9 @@ def generate(
target_library_names: str,
repository_path: str,
):
config = from_yaml(generation_config_yaml)
generate_from_yaml(
generation_config_yaml=generation_config_yaml,
config=config,
repository_path=repository_path,
target_library_names=target_library_names.split(",")
if target_library_names is not None
Expand All @@ -85,30 +83,23 @@ def generate(


def generate_from_yaml(
generation_config_yaml: str,
config: GenerationConfig,
repository_path: str,
target_library_names: list[str] = None,
) -> None:
"""
Parses a config yaml and generates libraries via
Based on the generation config, generates libraries via
generate_composed_library.py
:param generation_config_yaml: Path to generation_config.yaml that contains
the metadata about library generation
:param repository_path: If specified, the generated files will be sent to
this location. If not specified, the repository will be generated to the
current working directory.

:param config: a GenerationConfig object.
:param repository_path: The repository path to which the generated files
will be sent.
:param target_library_names: a list of libraries to be generated.
If specified, only the library whose library_name is in
target-library-names will be generated.
If specified, only the library whose library_name is in target_library_names
will be generated.
If specified with an empty list, then no library will be generated.
If not specified, all libraries in the configuration yaml will be generated.
"""
# convert paths to absolute paths, so they can be correctly referenced in
# downstream scripts
generation_config_yaml = os.path.abspath(generation_config_yaml)
repository_path = os.path.abspath(repository_path)

config = from_yaml(generation_config_yaml)
target_libraries = get_target_libraries(
config=config, target_library_names=target_library_names
)
Expand All @@ -117,7 +108,7 @@ def generate_from_yaml(
)

for library_path, library in repo_config.libraries.items():
print(f"generating library {library.api_shortname}")
print(f"generating library {library.get_library_name()}")

generate_composed_library(
config=config,
Expand Down
52 changes: 10 additions & 42 deletions library_generation/test/integration_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,8 @@
"java-bigtable": "chore/test-hermetic-build",
}
config_dir = f"{script_dir}/resources/integration"
config_name = "generation_config.yaml"
monorepo_baseline_commit = "a17d4caf184b050d50cacf2b0d579ce72c31ce74"
split_repo_baseline_commit = "679060c64136e85b52838f53cfe612ce51e60d1d"
baseline_config_name = "baseline_generation_config.yaml"
latest_config_name = "latest_generation_config.yaml"


class IntegrationTest(unittest.TestCase):
Expand Down Expand Up @@ -74,15 +73,11 @@ def test_entry_point_running_in_container(self):
)
repo_volumes = f"-v repo-{repo}:/workspace/{repo} -v config-{repo}:/workspace/config-{repo}"
# 4. run entry_point.py in docker container
baseline_commit = (
monorepo_baseline_commit
if repo == "google-cloud-java"
else split_repo_baseline_commit
)
self.__run_entry_point_in_docker_container(
repo=repo,
repo_volumes=repo_volumes,
baseline_commit=baseline_commit,
baseline_config=baseline_config_name,
latest_config=latest_config_name,
)
# 5. compare generation result with golden files
print(
Expand Down Expand Up @@ -162,7 +157,7 @@ def test_entry_point_running_in_container(self):
)
print(" pom.xml comparison succeed.")
# compare PR description
description_file = f"{config_dir}/{repo}/pr_description.txt"
description_file = f"{output_dir}/{repo}/pr_description.txt"
self.assertTrue(
cmp(
f"{config_dir}/{repo}/pr-description-golden.txt",
Expand Down Expand Up @@ -243,7 +238,7 @@ def __bind_device_to_volumes(cls, volume_name: str, device_dir: str):

@classmethod
def __run_entry_point_in_docker_container(
cls, repo: str, repo_volumes: str, baseline_commit: str
cls, repo: str, repo_volumes: str, baseline_config: str, latest_config: str
):
subprocess.check_call(
[
Expand All @@ -266,41 +261,14 @@ def __run_entry_point_in_docker_container(
"/src",
image_tag,
"python",
"/src/generate_repo.py",
"/src/cli/entry_point.py",
"generate",
f"--generation-config-yaml=/workspace/config-{repo}/{config_name}",
f"--baseline-generation-config=/workspace/config-{repo}/{baseline_config}",
f"--latest-generation-config=/workspace/config-{repo}/{latest_config}",
f"--repository-path=/workspace/{repo}",
]
)

subprocess.check_call(
[
"docker",
"run",
"--rm",
"-v",
f"repo-{repo}:/workspace/{repo}",
"-v",
f"config-{repo}:/workspace/config-{repo}",
"-v",
"/tmp:/tmp",
"-v",
"/var/run/docker.sock:/var/run/docker.sock",
"-e",
"RUNNING_IN_DOCKER=true",
"-e",
f"REPO_BINDING_VOLUMES={repo_volumes}",
"-w",
"/src",
image_tag,
"python",
"/src/generate_pr_description.py",
"generate",
f"--generation-config-yaml=/workspace/config-{repo}/{config_name}",
f"--baseline-commit={baseline_commit}",
]
)

@classmethod
def __get_config_files(cls, path: str) -> list[tuple[str, str]]:
config_files = []
Expand All @@ -310,7 +278,7 @@ def __get_config_files(cls, path: str) -> list[tuple[str, str]]:
repo = sub_dir.name
if repo in ["golden", "java-bigtable"]:
continue
config = f"{sub_dir}/{config_name}"
config = f"{sub_dir}/{latest_config_name}"
config_files.append((repo, config))
return config_files

Expand Down
Loading
Loading