From 3b1a91551ab6bbaf6a46950e1677c15cdd70d2e9 Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Mon, 22 Jul 2024 16:05:48 -0400 Subject: [PATCH] feat: add `transport` option to `generation_config.yaml` (#3052) This PR adds a new optional entry "`transport`" to `generation_config.yaml`. The `transport` entry will only have effect in the postprocessing output (i.e. generated libraries will still have its transport inferred via BUILD.bazel). The main output file affected by this is `.repo-metadata.json` and its derived files (e.g. README.md). This addresses the [need to allow a custom transport for java-storage](https://github.com/googleapis/java-storage/pull/2619#discussion_r1662963790). --- library_generation/README.md | 48 ++++++++++--------- .../generate_composed_library.py | 7 ++- library_generation/model/generation_config.py | 1 + library_generation/model/library_config.py | 21 ++++++++ library_generation/model/repo_config.py | 6 +++ ...repo-metadata-custom-transport-golden.json | 16 +++++++ .../test/utilities_unit_tests.py | 46 +++++++++++++++++- 7 files changed, 119 insertions(+), 26 deletions(-) create mode 100644 library_generation/test/resources/goldens/.repo-metadata-custom-transport-golden.json diff --git a/library_generation/README.md b/library_generation/README.md index 7ffe6a4a57..8bc47bad73 100644 --- a/library_generation/README.md +++ b/library_generation/README.md @@ -105,29 +105,31 @@ The library level parameters define how to generate a (multi-versions) GAPIC library. They are shared by all GAPICs of a library. -| Name | Required | Notes | -|:----------------------|:--------:|:-----------------------------------------------------------------------------------| -| api_shortname | Yes | | -| api_description | Yes | | -| name_pretty | Yes | | -| product_docs | Yes | | -| library_type | No | `GAPIC_AUTO` if not specified | -| release_level | No | `preview` if not specified | -| api_id | No | `{api_shortname}.googleapis.com` if not specified | -| api_reference | No | | -| codeowner_team | No | | -| client_documentation | No | | -| distribution_name | No | `{group_id}:google-{cloud_prefix}{library_name}` if not specified | -| excluded_poms | No | | -| excluded_dependencies | No | | -| googleapis_commitish | No | use repository level `googleapis_commitish` if not specified. | -| group_id | No | `com.google.cloud` if not specified | -| issue_tracker | No | | -| library_name | No | `api_shortname` is not specified. This value should be unique among all libraries. | -| rest_documentation | No | | -| rpc_documentation | No | | -| cloud_api | No | `true` if not specified | -| requires-billing | No | `true` if not specified | +| Name | Required | Notes | +|:----------------------|:--------:|:------------------------------------------------------------------------------------------------------------------------------------------| +| api_shortname | Yes | | +| api_description | Yes | | +| name_pretty | Yes | | +| product_docs | Yes | | +| library_type | No | `GAPIC_AUTO` if not specified | +| release_level | No | `preview` if not specified | +| api_id | No | `{api_shortname}.googleapis.com` if not specified | +| api_reference | No | | +| codeowner_team | No | | +| client_documentation | No | | +| distribution_name | No | `{group_id}:google-{cloud_prefix}{library_name}` if not specified | +| excluded_poms | No | | +| excluded_dependencies | No | | +| googleapis_commitish | No | use repository level `googleapis_commitish` if not specified. | +| group_id | No | `com.google.cloud` if not specified | +| issue_tracker | No | | +| library_name | No | `api_shortname` is not specified. This value should be unique among all libraries. | +| rest_documentation | No | | +| rpc_documentation | No | | +| cloud_api | No | `true` if not specified | +| requires-billing | No | `true` if not specified | +| transport | No | must be one of `grpc`, `rest` or `both`. This value would only be used for generating .repo-metadata.json and relevant sections in README | + Note that `cloud_prefix` is `cloud-` if `cloud_api` is `true`; empty otherwise. diff --git a/library_generation/generate_composed_library.py b/library_generation/generate_composed_library.py index dc094c0b11..5b503450a9 100755 --- a/library_generation/generate_composed_library.py +++ b/library_generation/generate_composed_library.py @@ -72,13 +72,16 @@ def generate_composed_library( gapic_inputs = parse_build_file(build_file_folder, gapic.proto_path) # generate postprocessing prerequisite files (.repo-metadata.json, .OwlBot-hermetic.yaml, # owlbot.py) here because transport is parsed from BUILD.bazel, - # which lives in a versioned proto_path. + # which lives in a versioned proto_path. The value of transport will be + # overriden by the config object if specified. Note that this override + # does not affect library generation but instead used only for + # generating postprocessing files such as README. util.generate_postprocessing_prerequisite_files( config=config, library=library, proto_path=util.remove_version_from(gapic.proto_path), - transport=gapic_inputs.transport, library_path=library_path, + transport=library.get_transport(gapic_inputs), ) temp_destination_path = f"java-{gapic.proto_path.replace('/','-')}" effective_arguments = __construct_effective_arg( diff --git a/library_generation/model/generation_config.py b/library_generation/model/generation_config.py index 8a2b0829c4..87242fd1a6 100644 --- a/library_generation/model/generation_config.py +++ b/library_generation/model/generation_config.py @@ -143,6 +143,7 @@ def from_yaml(path_to_yaml: str) -> GenerationConfig: ), recommended_package=__optional(library, "recommended_package", None), min_java_version=__optional(library, "min_java_version", None), + transport=__optional(library, "transport", None), ) parsed_libraries.append(new_library) diff --git a/library_generation/model/library_config.py b/library_generation/model/library_config.py index 9855e4e834..f7992f47a2 100644 --- a/library_generation/model/library_config.py +++ b/library_generation/model/library_config.py @@ -15,6 +15,7 @@ from hashlib import sha256 from typing import Optional from library_generation.model.gapic_config import GapicConfig +from library_generation.model.gapic_inputs import GapicInputs MAVEN_COORDINATE_SEPARATOR = ":" @@ -52,6 +53,7 @@ def __init__( extra_versioned_modules: Optional[str] = None, recommended_package: Optional[str] = None, min_java_version: Optional[int] = None, + transport: Optional[str] = None, ): self.api_shortname = api_shortname self.api_description = api_description @@ -78,6 +80,7 @@ def __init__( self.recommended_package = recommended_package self.min_java_version = min_java_version self.distribution_name = self.__get_distribution_name(distribution_name) + self.transport = self.__validate_transport(transport) def get_library_name(self) -> str: """ @@ -101,6 +104,15 @@ def get_artifact_id(self) -> str: """ return self.get_maven_coordinate().split(MAVEN_COORDINATE_SEPARATOR)[-1] + def get_transport(self, gapic_inputs: GapicInputs) -> str: + """ + Returns the transport of the library. If directly set in library config, return it. + Otherwise, return the transport inferred from gapic_inputs. This value is only + used for postprocessing - the generation still infers the transport from BUILD + files. + """ + return self.transport if self.transport is not None else gapic_inputs.transport + def __get_distribution_name(self, distribution_name: Optional[str]) -> str: LibraryConfig.__check_distribution_name(distribution_name) if distribution_name: @@ -109,6 +121,13 @@ def __get_distribution_name(self, distribution_name: Optional[str]) -> str: library_name = self.get_library_name() return f"{self.group_id}:google-{cloud_prefix}{library_name}" + def __validate_transport(self, transport: str): + if transport not in [None, "grpc", "rest", "both"]: + raise ValueError( + "allowed values for library.transport: grpc, rest, both and None" + ) + return transport + @staticmethod def __check_distribution_name(distribution_name: str) -> None: if not distribution_name: @@ -144,6 +163,7 @@ def __eq__(self, other): and self.extra_versioned_modules == other.extra_versioned_modules and self.recommended_package == other.recommended_package and self.min_java_version == other.min_java_version + and self.transport == other.transport ) def __hash__(self): @@ -175,6 +195,7 @@ def __hash__(self): self.extra_versioned_modules, self.recommended_package, self.min_java_version, + self.transport, ] + [config.proto_path for config in self.gapic_configs] ).encode("utf-8") diff --git a/library_generation/model/repo_config.py b/library_generation/model/repo_config.py index 520c505823..58ed3fa3bf 100644 --- a/library_generation/model/repo_config.py +++ b/library_generation/model/repo_config.py @@ -57,6 +57,12 @@ def get_library_version(self, artifact_id: str) -> str: return self.library_versions.get(artifact_id, NEW_CLIENT_VERSION) def __parse_versions(self) -> dict[str, str]: + """ + For a given versions.txt file (defined in self.versions_file) + creates a map of artifact-id to its version + + :return: a map "artifact-id -> version" + """ library_versions = dict() with open(self.versions_file) as f: for line in f.readlines(): diff --git a/library_generation/test/resources/goldens/.repo-metadata-custom-transport-golden.json b/library_generation/test/resources/goldens/.repo-metadata-custom-transport-golden.json new file mode 100644 index 0000000000..2cccd4b889 --- /dev/null +++ b/library_generation/test/resources/goldens/.repo-metadata-custom-transport-golden.json @@ -0,0 +1,16 @@ +{ + "api_shortname": "secretmanager", + "name_pretty": "Secret Management", + "product_documentation": "https://cloud.google.com/solutions/secrets-management/", + "api_description": "allows you to encrypt, store, manage, and audit infrastructure and application-level secrets.", + "client_documentation": "https://cloud.google.com/java/docs/reference/google-cloud-secretmanager/latest/overview", + "release_level": "preview", + "transport": "http", + "language": "java", + "repo": "googleapis/google-cloud-java", + "repo_short": "java-secretmanager", + "distribution_name": "com.google.cloud:google-cloud-secretmanager", + "api_id": "secretmanager.googleapis.com", + "library_type": "GAPIC_AUTO", + "requires_billing": true +} \ No newline at end of file diff --git a/library_generation/test/utilities_unit_tests.py b/library_generation/test/utilities_unit_tests.py index c38016fcba..7d9f09ec2c 100644 --- a/library_generation/test/utilities_unit_tests.py +++ b/library_generation/test/utilities_unit_tests.py @@ -23,6 +23,7 @@ from pathlib import Path from library_generation.utils import utilities as util from library_generation.model.gapic_config import GapicConfig +from library_generation.model.gapic_inputs import GapicInputs from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig from library_generation.test.test_utils import FileComparator @@ -57,6 +58,14 @@ api_description="example description", gapic_configs=list(), ) +test_library_with_custom_transport = LibraryConfig( + api_shortname="secretmanager", + name_pretty="Secret Management", + product_documentation="https://cloud.google.com/solutions/secrets-management/", + api_description="allows you to encrypt, store, manage, and audit infrastructure and application-level secrets.", + gapic_configs=list(), + transport="rest", +) class UtilitiesTest(unittest.TestCase): @@ -225,6 +234,40 @@ def test_generate_postprocessing_prerequisite_files_proto_only_repo_success(self ) self.__remove_postprocessing_prerequisite_files(path=library_path) + def test_generate_postprocessing_prerequisite_files__custom_transport_set_in_config__success( + self, + ): + """ + This test generates files for `test_library_with_custom_transport`, which + has an explicit value for transport declared (http). This is expected to + override the value obtained in BUILD.bazel via gapic_inputs.parse(). For + testing purposes, we test with a default GapicInputs object, whose transport + is set to "grpc". + """ + library_path = self.__setup_postprocessing_prerequisite_files( + combination=2, library=test_library_with_custom_transport + ) + + file_comparator.compare_files( + f"{library_path}/.repo-metadata.json", + f"{library_path}/.repo-metadata-custom-transport-golden.json", + ) + self.__remove_postprocessing_prerequisite_files(path=library_path) + + def test_create__library_invalid_transport__fails( + self, + ): + + with self.assertRaises(ValueError): + test_library_with_invalid_transport = LibraryConfig( + api_shortname="secretmanager", + name_pretty="Secret Management", + product_documentation="https://cloud.google.com/solutions/secrets-management/", + api_description="allows you to encrypt, store, manage, and audit infrastructure and application-level secrets.", + gapic_configs=list(), + transport="http", + ) + def test_prepare_repo_monorepo_success(self): gen_config = self.__get_a_gen_config(2) repo_config = util.prepare_repo( @@ -276,7 +319,8 @@ def __setup_postprocessing_prerequisite_files( library.library_type = library_type config = self.__get_a_gen_config(combination, library_type=library_type) proto_path = "google/cloud/baremetalsolution/v2" - transport = "grpc" + gapic_inputs = GapicInputs() # defaults to transport=grpc + transport = library.get_transport(gapic_inputs) util.generate_postprocessing_prerequisite_files( config=config, library=library,