feat: add transport option to generation_config.yaml#3052
feat: add transport option to generation_config.yaml#3052diegomarquezp merged 9 commits intomainfrom
transport option to generation_config.yaml#3052Conversation
library_generation/README.md
Outdated
| | 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`, `http` (alias) or `both`. This will override the value obtained from BUILD.bazel | |
There was a problem hiding this comment.
I think we use transport from BUILD.bazel to determine if we should generate a grpc module or not, can you please confirm that? If yes, we should update the description to something like This value would only be used for generating .repo-metadata.json and relevant sections in README.
| :param language: programming language of the library | ||
| :return: None | ||
| """ | ||
| transport = ( |
There was a problem hiding this comment.
Can we move this logic to generate_composed_library.py, right before calling the utilities.py, for simplicity?
There was a problem hiding this comment.
I initially did it that way, but then I found that it would be better for testing if we leave the transport inference logic to generate_postprocessing_prerequisite_files.
This test helper passes a hard-coded value:
sdk-platform-java/library_generation/test/utilities_unit_tests.py
Lines 279 to 286 in e46648f
And we don't have unit tests for generate_composed_library, so the only way to have a realistic execution path (other than the integration test) is by moving this logic to this method.
Do you think it's enough to make an exception?
There was a problem hiding this comment.
Resolution from discussion with @blakeli0: let's move the logic to the model class
| if transport == "grpc": | ||
| converted_transport = "grpc" | ||
| elif transport == "rest": | ||
| elif transport in [ |
There was a problem hiding this comment.
I guess this is because we use rest in BUILD but http in repo-metadata.json? This is convenient but I feel we should just stick with rest for consistency and simplicity.
| proto_path=util.remove_version_from(gapic.proto_path), | ||
| transport=gapic_inputs.transport, | ||
| library_path=library_path, | ||
| gapic_inputs=gapic_inputs, |
There was a problem hiding this comment.
from @blakeli0: let's encapsulate this logic in a function of model.LibraryConfig so it's easier to test and we avoid passing gapic_inputs down the call
| """ | ||
| return self.get_maven_coordinate().split(MAVEN_COORDINATE_SEPARATOR)[-1] | ||
|
|
||
| def get_transport(self, gapic_inputs: GapicInputs) -> str: |
There was a problem hiding this comment.
Nit: Add a comment mentioning that the value returned from this method is only used for generating post-processing prerequisite files.
|
|
🤖 I have created a release *beep* *boop* --- <details><summary>2.43.0</summary> ## [2.43.0](v2.42.0...v2.43.0) (2024-07-25) ### Features * add `transport` option to `generation_config.yaml` ([#3052](#3052)) ([3b1a915](3b1a915)) * get released version from versions.txt to render `README.md` ([#3007](#3007)) ([99bb2b3](99bb2b3)) * Introduce java.time to Gax-Java ([#1872](#1872)) ([308aeaf](308aeaf)) * Mark `getDefaultEndpoint()` with @ObsoleteApi ([#2347](#2347)) ([e46648f](e46648f)) * parse `BUILD.bzel` to determine whether a commit that only changed `BUILD.bazel` is a qualified commit ([#2937](#2937)) ([502f801](502f801)) ### Bug Fixes * Fix: ([d996c2d](d996c2d)) * `BaseApiTracer` to noop on attemptFailed via overloaded method call ([#3016](#3016)) ([2fc938a](2fc938a)) * Generator to skip generation for empty services. ([#3051](#3051)) ([ff2c485](ff2c485)) * restore hermetic build image publication ([#2952](#2952)) ([97a6d67](97a6d67)) ### Dependencies * update dependency com.fasterxml.jackson:jackson-bom to v2.17.2 ([#3028](#3028)) ([d16f9d1](d16f9d1)) * update dependency com.google.cloud.opentelemetry:detector-resources-support to v0.30.0 ([#2975](#2975)) ([b3ec93f](b3ec93f)) * update dependency com.google.cloud.opentelemetry:detector-resources-support to v0.31.0 ([#3044](#3044)) ([6bd07dc](6bd07dc)) * update dependency com.google.errorprone:error_prone_annotations to v2.29.2 ([#3058](#3058)) ([8ea0868](8ea0868)) * update dependency com.google.errorprone:error_prone_annotations to v2.29.2 ([#3059](#3059)) ([81b23dc](81b23dc)) * update dependency com.google.guava:guava to v33.2.1-jre ([#3027](#3027)) ([12ee456](12ee456)) * update dependency commons-codec:commons-codec to v1.17.1 ([#3049](#3049)) ([58d94b7](58d94b7)) * update dependency dev.cel:cel to v0.6.0 ([#3050](#3050)) ([bc332d9](bc332d9)) * update dependency net.bytebuddy:byte-buddy to v1.14.18 ([#3029](#3029)) ([8799cf6](8799cf6)) * update dependency org.apache.commons:commons-lang3 to v3.15.0 ([#3060](#3060)) ([2538334](2538334)) * update dependency org.checkerframework:checker-qual to v3.45.0 ([#2988](#2988)) ([4edd216](4edd216)) * update google api dependencies ([#2951](#2951)) ([c16f6c9](c16f6c9)) * update google auth library dependencies to v1.24.0 ([#3039](#3039)) ([98b5bd7](98b5bd7)) * update googleapis/java-cloud-bom digest to 47c5dbc ([#2974](#2974)) ([57623f0](57623f0)) * update grpc dependencies to v1.65.1 ([#3061](#3061)) ([27497e2](27497e2)) * update junit5 monorepo to v5.10.3 ([#2963](#2963)) ([bc55fe1](bc55fe1)) * update netty dependencies to v4.1.112.final ([#3057](#3057)) ([5af127b](5af127b)) * update opentelemetry-java monorepo to v1.40.0 ([#3035](#3035)) ([5c31c42](5c31c42)) * Use Gapic-Showcase v0.35.1 ([#3018](#3018)) ([43773f0](43773f0)) ### Documentation * add support option to 'new issue' choices ([#3055](#3055)) ([6a2a17d](6a2a17d)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>



This PR adds a new optional entry "
transport" togeneration_config.yaml.The
transportentry 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.jsonand its derived files (e.g. README.md).This addresses the need to allow a custom transport for java-storage.