-
Notifications
You must be signed in to change notification settings - Fork 53
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
feat: add entry point #2616
Changes from 28 commits
95b66a8
964d81a
5b303cb
4f6cedd
531244d
a6811d5
14cdfb9
24f2070
853b70c
923f149
1cd3859
ffc16ee
87c7d1c
9d723b0
fd7897a
ca902ec
4895971
87c0bdd
69fd09b
d2b3963
b4b907b
ce25efa
8d1d3f6
e155ea0
8de0c3d
e518d3f
22eb127
c25e2bd
cc137b0
5dbcd2c
3d3522b
280cee5
881b557
4de89de
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
library generation. | ||
The googleapis commit in the configuration is the oldest commit, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we include some high level description for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added high level description of
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We probably want to rename |
||
config=config_change.current_config, | ||
baseline_commit=config_change.baseline_config.googleapis_commitish, | ||
description_path=repository_path, | ||
) | ||
|
||
|
||
if __name__ == "__main__": | ||
main() |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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 | ||||||
|
@@ -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, | ||||||
|
@@ -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 | ||||||
|
@@ -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: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like this validation! I think we should add
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||||||
|
@@ -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, | ||||||
|
@@ -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: | ||||||
|
@@ -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() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolute path?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.