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: generate proto-only repository #2720

Merged
merged 61 commits into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
53e0a2b
chore: add generation config
JoeWang1127 May 2, 2024
7541b67
Merge branch 'main' into chore/add-generation-config
JoeWang1127 May 2, 2024
4f5118c
add gapics
JoeWang1127 May 2, 2024
6629f2f
Merge branch 'main' into chore/add-generation-config
JoeWang1127 May 2, 2024
b8be0b2
add java-iam
JoeWang1127 May 2, 2024
7f48e08
fix proto_path
JoeWang1127 May 2, 2024
0026eed
rename OwlBot.yaml
JoeWang1127 May 2, 2024
78dd8d7
change command
JoeWang1127 May 2, 2024
788e30c
Merge branch 'main' into chore/add-generation-config
JoeWang1127 May 6, 2024
971867e
restore change
JoeWang1127 May 6, 2024
f3a6fbd
change source dir
JoeWang1127 May 6, 2024
ddce346
change source
JoeWang1127 May 6, 2024
8c21004
change to snapshot
JoeWang1127 May 7, 2024
7da4e21
change readme template
JoeWang1127 May 7, 2024
abcc81b
change copyright year
JoeWang1127 May 7, 2024
9a65467
do not apply repo level postprocessing if it has proto-only libraries
JoeWang1127 May 7, 2024
e19c0ff
generate proto-only repo
JoeWang1127 May 7, 2024
edc09e2
do not generate api-id
JoeWang1127 May 7, 2024
4321f11
Merge branch 'main' into chore/add-generation-config
JoeWang1127 May 7, 2024
a4f945f
refactor unit tests
JoeWang1127 May 7, 2024
8b3563b
add unit test to verify proto-only BUILD
JoeWang1127 May 7, 2024
396ea8e
change readme template
JoeWang1127 May 8, 2024
905ce01
fix unit test
JoeWang1127 May 8, 2024
ed8e811
Merge branch 'main' into chore/add-generation-config
JoeWang1127 May 9, 2024
afa2f2f
merge main
JoeWang1127 May 9, 2024
a4a6f4a
restore template
JoeWang1127 May 10, 2024
a7c3768
remove unused parameters
JoeWang1127 May 10, 2024
49f4b11
Merge branch 'main' into chore/add-generation-config
JoeWang1127 May 10, 2024
fb2d417
exclude readme
JoeWang1127 May 10, 2024
2830340
exclude readme from configuration
JoeWang1127 May 10, 2024
f5a2d5b
merge main
JoeWang1127 May 13, 2024
b3e5955
change to is_gapic_monorepo
JoeWang1127 May 13, 2024
6bba76b
change logic in is_gapic_monorepo
JoeWang1127 May 13, 2024
1497d1b
change to gapic repo
JoeWang1127 May 13, 2024
1a34c4f
fix unit tests
JoeWang1127 May 13, 2024
b0982b5
remove comment
JoeWang1127 May 13, 2024
c808c62
make libraries bom version optional
JoeWang1127 May 13, 2024
ea50d89
Revert "change copyright year"
JoeWang1127 May 14, 2024
71690cd
change to function
JoeWang1127 May 14, 2024
e7b0232
restore readme template
JoeWang1127 May 14, 2024
5a4e495
restore synthtool
JoeWang1127 May 14, 2024
4b1e5c8
separate two functions
JoeWang1127 May 14, 2024
08ddac8
Merge branch 'main' into chore/add-generation-config
JoeWang1127 May 14, 2024
599a45b
Revert "change source"
JoeWang1127 May 14, 2024
6c7fa80
Revert "change source dir"
JoeWang1127 May 14, 2024
ff451ff
Revert "rename OwlBot.yaml"
JoeWang1127 May 14, 2024
59b5657
lint
JoeWang1127 May 14, 2024
d4d9082
remove generation config
JoeWang1127 May 14, 2024
bd7859f
Merge branch 'main' into chore/add-generation-config
JoeWang1127 May 14, 2024
c18090f
Merge branch 'main' into chore/add-generation-config
JoeWang1127 May 14, 2024
b46723d
Merge branch 'main' into chore/add-generation-config
JoeWang1127 May 14, 2024
685905a
Merge branch 'main' into chore/add-generation-config
JoeWang1127 May 14, 2024
7748828
Merge branch 'main' into chore/add-generation-config
JoeWang1127 May 15, 2024
a25e604
remove variable
JoeWang1127 May 15, 2024
958ef5a
Merge branch 'main' into chore/add-generation-config
JoeWang1127 May 15, 2024
efa3efa
remove setter
JoeWang1127 May 15, 2024
c76524d
add break
JoeWang1127 May 15, 2024
c4731d1
Merge branch 'main' into chore/add-generation-config
JoeWang1127 May 16, 2024
7385de4
remove a parameter
JoeWang1127 May 16, 2024
534ae9c
refactor
JoeWang1127 May 16, 2024
1751738
Merge branch 'main' into chore/add-generation-config
JoeWang1127 May 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 0 additions & 4 deletions java-common-protos/codecov.yaml

This file was deleted.

1 change: 1 addition & 0 deletions java-common-protos/owlbot.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
"SECURITY.md",
"java.header",
"license-checks.xml",
"README.md",
"renovate.json",
".gitignore"
])
4 changes: 0 additions & 4 deletions java-iam/codecov.yaml

This file was deleted.

5 changes: 5 additions & 0 deletions library_generation/generate_composed_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,12 @@ def generate_composed_library(
library: LibraryConfig,
output_folder: str,
versions_file: str,
gapic_repo: bool,
) -> None:
"""
Generate libraries composed of more than one service or service version

:param has_proto_only_libraries:
:param config_path: Path to generation configuration.
:param config: a GenerationConfig object representing a parsed configuration
yaml
Expand All @@ -59,6 +61,8 @@ def generate_composed_library(
for convenience and to prevent all libraries to be processed
:param output_folder: the folder to where tools go
:param versions_file: the file containing version of libraries
:param gapic_repo: whether the library is generated into a gapic
repository or not.
:return None
"""
util.pull_api_definition(
Expand All @@ -81,6 +85,7 @@ def generate_composed_library(
proto_path=util.remove_version_from(gapic.proto_path),
transport=gapic_inputs.transport,
library_path=library_path,
gapic_repo=gapic_repo,
)
temp_destination_path = f"java-{gapic.proto_path.replace('/','-')}"
effective_arguments = __construct_effective_arg(
Expand Down
7 changes: 4 additions & 3 deletions library_generation/generate_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
from library_generation.model.library_config import LibraryConfig
from library_generation.utils.monorepo_postprocessor import monorepo_postprocessing

PROTO_ONLY_LIBRARIES = ["common-protos"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this one should also be changed to "common proto libraries"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This variable is actually not used, I removed it.



def generate_from_yaml(
config_path: str,
Expand Down Expand Up @@ -48,18 +50,17 @@ def generate_from_yaml(

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

generate_composed_library(
config_path=config_path,
config=config,
library_path=library_path,
library=library,
output_folder=repo_config.output_folder,
versions_file=repo_config.versions_file,
gapic_repo=not config.contains_common_protos(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we call this gapic_repo? I see that config is already being passed around, can we just use config.contains_common_protos()? I feel it is easier to understand.

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 removed this parameter as we already pass the config.

)

# we skip monorepo_postprocessing if not in a monorepo
if not config.is_monorepo():
if not config.is_monorepo() or config.contains_common_protos():
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Only perform monorepo post processing if this is a gapic monorepo.

return

monorepo_postprocessing(
Expand Down
2 changes: 1 addition & 1 deletion library_generation/model/gapic_inputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def __init__(
self,
proto_only="true",
additional_protos="google/cloud/common_resources.proto",
transport="",
transport="grpc",
rest_numeric_enum="",
gapic_yaml="",
service_config="",
Expand Down
21 changes: 15 additions & 6 deletions library_generation/model/generation_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
REPO_LEVEL_PARAMETER = "Repo level parameter"
LIBRARY_LEVEL_PARAMETER = "Library level parameter"
GAPIC_LEVEL_PARAMETER = "GAPIC level parameter"
COMMON_PROTOS = "common-protos"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
COMMON_PROTOS = "common-protos"
COMMON_PROTOS_LIBRARY_NAME = "common-protos"

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.



class GenerationConfig:
Expand All @@ -31,9 +32,9 @@ def __init__(
self,
gapic_generator_version: str,
googleapis_commitish: str,
libraries_bom_version: str,
template_excludes: list[str],
libraries: list[LibraryConfig],
libraries_bom_version: Optional[str] = None,
grpc_version: Optional[str] = None,
protoc_version: Optional[str] = None,
):
Expand All @@ -44,6 +45,7 @@ def __init__(
self.libraries = libraries
self.grpc_version = grpc_version
self.protoc_version = protoc_version
self.__contains_common_protos = self.__set_contains_common_protos()
self.__validate()

def get_proto_path_to_library_name(self) -> dict[str, str]:
Expand All @@ -61,6 +63,15 @@ def get_proto_path_to_library_name(self) -> dict[str, str]:
def is_monorepo(self) -> bool:
return len(self.libraries) > 1

def contains_common_protos(self) -> bool:
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 this method to determine whether there are common protos in the configuration.

We can't just have one is_gapic_monorepo method since we have three cases:

  • sdk-platform-java, contains_common_protos returns True
  • google-cloud-java, contains_common_protos returns False and is_monorepo returns True
  • handwritten libraries, contains_common_protos returns False and is_monorepo returns False

Copy link
Contributor

Choose a reason for hiding this comment

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

We have one method to set the value and one to compute, plus we store a variable in the config object. Maybe we can put the logic of the setter in the getter method and compute the value once in the field we create in the constructor. That would leave us with one less method.

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 removed the setter method.

return self.__contains_common_protos

def __set_contains_common_protos(self) -> bool:
for library in self.libraries:
if library.get_library_name() == COMMON_PROTOS:
return True
return False

def __validate(self) -> None:
seen_library_names = dict()
for library in self.libraries:
Expand Down Expand Up @@ -133,15 +144,13 @@ def from_yaml(path_to_yaml: str) -> GenerationConfig:
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", REPO_LEVEL_PARAMETER
),
libraries_bom_version=__required(
config, "libraries_bom_version", REPO_LEVEL_PARAMETER
),
template_excludes=__required(config, "template_excludes", REPO_LEVEL_PARAMETER),
grpc_version=__optional(config, "grpc_version", None),
protoc_version=__optional(config, "protoc_version", None),
libraries_bom_version=__optional(config, "libraries_bom_version", None),
libraries=parsed_libraries,
)

Expand Down
1 change: 1 addition & 0 deletions library_generation/test/cli/entry_point_unit_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

class EntryPointTest(unittest.TestCase):
def test_entry_point_without_config_raise_file_exception(self):
os.chdir(script_dir)
runner = CliRunner()
# noinspection PyTypeChecker
result = runner.invoke(generate, ["--repository-path=."])
Expand Down
132 changes: 132 additions & 0 deletions library_generation/test/model/gapic_inputs_unit_tests.py
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for backfilling this test!

Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
import os
import unittest
from pathlib import Path

from parameterized import parameterized
from library_generation.model.gapic_inputs import parse

script_dir = os.path.dirname(os.path.realpath(__file__))
resources_dir = os.path.join(script_dir, "..", "resources")
build_file = Path(os.path.join(resources_dir, "misc")).resolve()


class UtilitiesTest(unittest.TestCase):
@parameterized.expand(
[
("BUILD_no_additional_protos.bazel", " "),
("BUILD_common_resources.bazel", " google/cloud/common_resources.proto"),
("BUILD_comment_common_resources.bazel", " "),
("BUILD_locations.bazel", " google/cloud/location/locations.proto"),
("BUILD_comment_locations.bazel", " "),
("BUILD_iam_policy.bazel", " google/iam/v1/iam_policy.proto"),
("BUILD_comment_iam_policy.bazel", " "),
(
"BUILD_iam_locations.bazel",
" google/cloud/location/locations.proto google/iam/v1/iam_policy.proto",
),
]
)
def test_gapic_inputs_parse_additional_protos(self, build_name, expected):
parsed = parse(build_file, "", build_name)
self.assertEqual(
expected,
parsed.additional_protos,
)

def test_gapic_inputs_parse_grpc_only_succeeds(self):
parsed = parse(build_file, "", "BUILD_grpc.bazel")
self.assertEqual("grpc", parsed.transport)

def test_gapic_inputs_parse_grpc_rest_succeeds(self):
parsed = parse(build_file, "", "BUILD_grpc_rest.bazel")
self.assertEqual("grpc+rest", parsed.transport)

def test_gapic_inputs_parse_rest_succeeds(self):
parsed = parse(build_file, "", "BUILD_rest.bazel")
self.assertEqual("rest", parsed.transport)

def test_gapic_inputs_parse_empty_include_samples_succeeds(self):
parsed = parse(build_file, "", "BUILD_include_samples_empty.bazel")
self.assertEqual("false", parsed.include_samples)

def test_gapic_inputs_parse_include_samples_false_succeeds(self):
parsed = parse(build_file, "", "BUILD_include_samples_false.bazel")
self.assertEqual("false", parsed.include_samples)

def test_gapic_inputs_parse_include_samples_true_succeeds(self):
parsed = parse(build_file, "", "BUILD_include_samples_true.bazel")
self.assertEqual("true", parsed.include_samples)

def test_gapic_inputs_parse_empty_rest_numeric_enums_succeeds(self):
parsed = parse(build_file, "", "BUILD_rest_numeric_enums_empty.bazel")
self.assertEqual("false", parsed.rest_numeric_enum)

def test_gapic_inputs_parse_rest_numeric_enums_false_succeeds(self):
parsed = parse(build_file, "", "BUILD_rest_numeric_enums_false.bazel")
self.assertEqual("false", parsed.rest_numeric_enum)

def test_gapic_inputs_parse_rest_numeric_enums_true_succeeds(self):
parsed = parse(build_file, "", "BUILD_rest_numeric_enums_true.bazel")
self.assertEqual("true", parsed.rest_numeric_enum)

def test_gapic_inputs_parse_no_gapic_library_returns_proto_only_true(self):
# include_samples_empty only has a gradle assembly rule
parsed = parse(build_file, "", "BUILD_include_samples_empty.bazel")
self.assertEqual("true", parsed.proto_only)

def test_gapic_inputs_parse_with_gapic_library_returns_proto_only_false(self):
# rest.bazel has a java_gapic_library rule
parsed = parse(build_file, "", "BUILD_rest.bazel")
self.assertEqual("false", parsed.proto_only)

def test_gapic_inputs_parse_gapic_yaml_succeeds(self):
parsed = parse(build_file, "test/versioned/path", "BUILD_gapic_yaml.bazel")
self.assertEqual("test/versioned/path/test_gapic_yaml.yaml", parsed.gapic_yaml)

def test_gapic_inputs_parse_no_gapic_yaml_returns_empty_string(self):
parsed = parse(build_file, "test/versioned/path", "BUILD_no_gapic_yaml.bazel")
self.assertEqual("", parsed.gapic_yaml)

def test_gapic_inputs_parse_service_config_succeeds(self):
parsed = parse(build_file, "test/versioned/path", "BUILD_service_config.bazel")
self.assertEqual(
"test/versioned/path/test_service_config.json", parsed.service_config
)

def test_gapic_inputs_parse_service_yaml_relative_target(self):
parsed = parse(
build_file,
"google/cloud/compute/v1",
"BUILD_service_config_relative_target.bazel",
)
self.assertEqual(
"google/cloud/compute/v1/compute_grpc_service_config.json",
parsed.service_config,
)

def test_gapic_inputs_parse_no_service_config_returns_empty_string(self):
parsed = parse(
build_file, "test/versioned/path", "BUILD_no_service_config.bazel"
)
self.assertEqual("", parsed.service_config)

def test_gapic_inputs_parse_service_yaml_succeeds(self):
parsed = parse(build_file, "test/versioned/path", "BUILD_service_yaml.bazel")
self.assertEqual(
"test/versioned/path/test_service_yaml.yaml", parsed.service_yaml
)

def test_gapic_inputs_parse_service_yaml_absolute_target(self):
parsed = parse(build_file, "", "BUILD_service_yaml_absolute_target.bazel")
self.assertEqual(
"google/cloud/videointelligence/videointelligence_v1p3beta1.yaml",
parsed.service_yaml,
)

def test_gapic_inputs_parse_no_service_yaml_returns_empty_string(self):
parsed = parse(build_file, "test/versioned/path", "BUILD_no_service_yaml.bazel")
self.assertEqual("", parsed.service_yaml)

def test_gapic_inputs_parse_proto_only_returns_grpc(self):
parsed = parse(build_file, "test/versioned/path", "BUILD_proto_only.bazel")
self.assertEqual("grpc", parsed.transport)
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@
product_documentation="",
gapic_configs=[],
)
common_protos_library = LibraryConfig(
api_shortname="common-protos",
api_description="",
name_pretty="",
product_documentation="",
gapic_configs=[],
)


class GenerationConfigTest(unittest.TestCase):
Expand Down Expand Up @@ -123,6 +130,26 @@ def test_is_monorepo_with_two_libraries_returns_true(self):
)
self.assertTrue(config.is_monorepo())

def test_contains_common_protos_with_common_protos_returns_true(self):
config = GenerationConfig(
gapic_generator_version="",
googleapis_commitish="",
libraries_bom_version="",
template_excludes=[],
libraries=[library_1, library_2, common_protos_library],
)
self.assertTrue(config.contains_common_protos())

def test_contains_common_protos_without_common_protos_returns_false(self):
config = GenerationConfig(
gapic_generator_version="",
googleapis_commitish="",
libraries_bom_version="",
template_excludes=[],
libraries=[library_1, library_2],
)
self.assertFalse(config.contains_common_protos())

def test_validate_with_duplicate_library_name_raise_exception(self):
self.assertRaisesRegex(
ValueError,
Expand All @@ -131,8 +158,6 @@ def test_validate_with_duplicate_library_name_raise_exception(self):
gapic_generator_version="",
googleapis_commitish="",
libraries_bom_version="",
owlbot_cli_image="",
synthtool_commitish="",
template_excludes=[],
libraries=[
LibraryConfig(
Expand Down Expand Up @@ -169,30 +194,6 @@ def test_from_yaml_without_googleapis_commitish_raise_exception(self):
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,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"api_shortname": "baremetalsolution",
"name_pretty": "Bare Metal Solution",
"product_documentation": "https://cloud.google.com/bare-metal/docs",
"api_description": "Bring your Oracle workloads to Google Cloud with Bare Metal Solution and jumpstart your cloud journey with minimal risk.",
"client_documentation": "https://cloud.google.com/java/docs/reference/google-cloud-bare-metal-solution/latest/overview",
"release_level": "preview",
"transport": "grpc",
"language": "java",
"repo": "googleapis/sdk-platform-java",
"repo_short": "java-bare-metal-solution",
"distribution_name": "com.google.cloud:google-cloud-bare-metal-solution",
"library_type": "OTHER",
"requires_billing": true,
"rest_documentation": "https://cloud.google.com/bare-metal/docs/reference/rest",
"rpc_documentation": "https://cloud.google.com/bare-metal/docs/reference/rpc"
}