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 28 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
118 changes: 118 additions & 0 deletions library_generation/cli/entry_point.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
# 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=False,
default=None,
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 oldest commit,
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 include some high level description for baseline-generation-config? For example, something like "We compare baseline-generation-config with the current generation config, and generate commit messages based on the diff of the googleapis-commitish between the two generation configs."
Same comment for current-generation-config below.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added high level description of generate method in doc string.
User can read the docs through

python library_generation/cli/entry_point.py generate --help

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, looks good! I think we still need more info to distinguish between baseline-generation-config from current-generation-config. For example, both descriptions include Absolute or relative path to generation_config.yaml that contains the metadata about library generation., in fact, only the info in current-generation-config is used for generation, baseline-generation-config is only used for generating commit history. Also the additional info regarding inclusive/exclusive of googleapis commitish might be a little too detailed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed the comment.

exclusively, from which the commit message is considered.
""",
)
@click.option(
"--current-generation-config",
required=False,
default=None,
type=str,
help="""
Path to generation_config.yaml that contains the metadata about
library generation.
The googleapis commit in the configuration is the newest 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,
current_generation_config: str,
repository_path: str,
):
default_generation_config = f"{os.getcwd()}/generation_config.yaml"
if baseline_generation_config is None and current_generation_config is None:
if not os.path.isfile(default_generation_config):
raise FileNotFoundError(
f"{default_generation_config} does not exist when "
f"both baseline_generation_config and current_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.

This additional info may be more complete but may also confuse people, took me sometime to understand as well. Can we simply saying
f"{default_generation_config} does not exist.
or
f"{default_generation_config} does not exist. A valid generation config has to be passed in as "current_generation_config" or exist in current working directory.

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.

f" are not specified."
)
current_generation_config = default_generation_config
elif current_generation_config is None:
blakeli0 marked this conversation as resolved.
Show resolved Hide resolved
# make sure current_generation_config is not None if only one config
# is specified.
current_generation_config = baseline_generation_config
baseline_generation_config = None
current_generation_config = os.path.abspath(current_generation_config)
repository_path = os.path.abspath(repository_path)
if not baseline_generation_config:
blakeli0 marked this conversation as resolved.
Show resolved Hide resolved
# Execute full generation based on current_generation_config if
# baseline_generation_config is not specified.
# Do not generate pull request description.
generate_from_yaml(
config=from_yaml(current_generation_config),
repository_path=repository_path,
)
return

# Compare two generation configs and only generate changed libraries.
# Generate pull request description.
baseline_generation_config = os.path.abspath(baseline_generation_config)
config_change = compare_config(
baseline_config=from_yaml(baseline_generation_config),
current_config=from_yaml(current_generation_config),
)
generate_from_yaml(
config=config_change.current_config,
repository_path=repository_path,
target_library_names=config_change.get_changed_libraries(),
)
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.current_config,
baseline_commit=config_change.baseline_config.googleapis_commitish,
description_path=repository_path,
)


if __name__ == "__main__":
main()
99 changes: 64 additions & 35 deletions library_generation/generate_pr_description.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,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 calendar
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,51 +53,61 @@ 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,
current_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(
def get_commit_messages(
repo_url: str,
latest_commit: str,
current_commit: str,
baseline_commit: str,
paths: Dict[str, str],
is_monorepo: bool,
Expand All @@ -109,7 +119,7 @@ def __get_commit_messages(
Note that baseline_commit should be an ancestor of latest_commit.

:param repo_url: the url of the repository.
:param latest_commit: the newest commit to be considered in
:param current_commit: the newest commit to be considered in
selecting commit message.
:param baseline_commit: the oldest commit to be considered in
selecting commit message. This commit should be an ancestor of
Expand All @@ -121,7 +131,15 @@ def __get_commit_messages(
shutil.rmtree(tmp_dir, ignore_errors=True)
os.mkdir(tmp_dir)
repo = Repo.clone_from(repo_url, tmp_dir)
commit = repo.commit(latest_commit)
commit = repo.commit(current_commit)
current_commit_time = __get_commit_timestamp(commit)
baseline_commit_time = __get_commit_timestamp(repo.commit(baseline_commit))
if current_commit_time < baseline_commit_time:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Like this validation! I think we should add = though.

Suggested change
if current_commit_time < baseline_commit_time:
if current_commit_time <= baseline_commit_time:

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.

Added unit tests to verify the behavior.

raise ValueError(
f"current_commit ({current_commit[:7]}, committed on "
f"{current_commit_time}) should be newer than baseline_commit "
f"({baseline_commit[:7]}, committed on {baseline_commit_time})."
)
qualified_commits = {}
while str(commit.hexsha) != baseline_commit:
commit_and_name = __filter_qualified_commit(paths=paths, commit=commit)
Expand All @@ -133,7 +151,7 @@ def __get_commit_messages(
commit = commit_parents[0]
shutil.rmtree(tmp_dir, ignore_errors=True)
return __combine_commit_messages(
latest_commit=latest_commit,
current_commit=current_commit,
baseline_commit=baseline_commit,
commits=qualified_commits,
is_monorepo=is_monorepo,
Expand All @@ -159,13 +177,13 @@ def __filter_qualified_commit(paths: Dict[str, str], commit: Commit) -> (Commit,


def __combine_commit_messages(
latest_commit: str,
current_commit: str,
baseline_commit: str,
commits: Dict[Commit, str],
is_monorepo: bool,
) -> str:
messages = [
f"This pull request is generated with proto changes between googleapis commit {baseline_commit} (exclusive) and {latest_commit} (inclusive).",
f"This pull request is generated with proto changes between googleapis commit {baseline_commit} (exclusive) and {current_commit} (inclusive).",
"Qualified commits are:",
]
for commit in commits:
Expand All @@ -183,5 +201,16 @@ def __combine_commit_messages(
return "\n".join(messages)


def __get_commit_timestamp(commit: Commit) -> int:
"""
# Convert datetime to UTC timestamp. For more info:
# https://stackoverflow.com/questions/5067218/get-utc-timestamp-in-python-with-datetime

:param commit: a Commit object
:return: the timestamp of the commit
"""
return calendar.timegm(commit.committed_datetime.utctimetuple())


if __name__ == "__main__":
main()
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
Loading
Loading