feat: search gapic additional protos in BUILD.bazel#2004
Conversation
|
The following list is used in testing: |
library_generation/README.md
Outdated
| ### contains_iam_policy (optional) | ||
| Whether to include `google/iam/v1/iam_policy.proto` into the library. | ||
| The value is either `true` or `false`. | ||
| The default value is `false`. | ||
|
|
||
| Use `--contains_iam_policy` to specify the value. | ||
|
|
||
| ### contains_locations (optional) | ||
| Whether to include `google/cloud/location/locations.proto` into the library. | ||
| The value is either `true` or `false`. | ||
| The default value is `false`. | ||
|
|
||
| Use `--contains_locations` to specify the value. | ||
|
|
There was a problem hiding this comment.
I can see this list growing over time. Would it make sense to just have additional_protos parameter?
There was a problem hiding this comment.
Do you mean additional_protos as a string of protos?
For example:
additional_protos="google/iam/v1/iam_policy.proto google/cloud/location/locations.proto" or additional_protos="google/cloud/location/locations.proto"
There was a problem hiding this comment.
+1 to make it more generic. It would solve the problem for common shopping protos as well. I'm also curious why google/longrunning/operations.proto is not affected by this issue.
There was a problem hiding this comment.
@blakeli0 @meltsufin I plan to name additional protos that pass to the generator gapic_additional_protos, WDYT?
So far I found google/cloud/location/locations.proto, google/iam/v1/iam_policy.proto and google/cloud/common_resources.proto may go to this list.
|
[gapic-generator-java-root] Kudos, SonarCloud Quality Gate passed!
|
|
[java_showcase_integration_tests] Kudos, SonarCloud Quality Gate passed!
|
|
[java_showcase_unit_tests] Kudos, SonarCloud Quality Gate passed!
|
BUILD.bazelBUILD.bazel
BUILD.bazelBUILD.bazel
| echo "${result}" | ||
| } | ||
|
|
||
| __get_iam_policy_from_BUILD() { |
There was a problem hiding this comment.
Do we still need to search for these protos from BUILD now that they are being passed in as parameters?
In addition, do we need to read anything else from BUILD file other than searching for common protos?
There was a problem hiding this comment.
Do we still need to search for these protos from BUILD now that they are being passed in as parameters?
All functions that read BUILD are used by generate_library_integration_test.sh for testing against googleapis-gen. They are not in test_utilities.sh because they are part of generate_sdk_libraries.sh later.
In addition, do we need to read anything else from BUILD file other than searching for common protos?
transport, rest_numeric_enums and include_samples are also read from BUILD for testing.
There was a problem hiding this comment.
All functions that read BUILD are used by generate_library_integration_test.sh for testing against googleapis-gen. They are not in test_utilities.sh because they are part of generate_sdk_libraries.sh later.
I'm not sure we need it in generate_sdk_libraries.sh either. We would still need to keep the bazel interface, so for a single client library, the flow would be bazel build -> java_gapic_assembly_gradle_pkg -> generate_library.sh, where java_gapic_assembly_gradle_pkg would pass all the info from bazel file to our shell script. So generate_sdk_libraries.sh would just be calling bazel build for each service in this case.
There was a problem hiding this comment.
If that's the case then we probably don't need these utility functions.
Right now it's only used in testing and can be removed without impacting generate_library.sh.
There was a problem hiding this comment.
Right now it's only used in testing and can be removed without impacting generate_library.sh.
We can still keep them and move them to the test utils.
There was a problem hiding this comment.
We can still keep them and move them to the test utils.
How about I create another PR to refactor test utilities into test_utilities.sh. I think there are other functions can move into test_utilities.sh as well.
Fixes #1999 ☕️