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: enable generation with postprocessing of multiple service versions #2342

Merged
merged 219 commits into from
Jan 29, 2024

Conversation

diegomarquezp
Copy link
Contributor

@diegomarquezp diegomarquezp commented Jan 5, 2024

context: enabling postprocessing for HW libraries

Changes in this PR

This PR provides a generate_composed_library.py that will call generate_library.sh for a series of GAPICs as defined in a configuration yaml. All of these calls are without postprocessing. The script will then organize a pre-processed-sources folder that will be used for a single final postprocessing run. The resulting post-processed combined library will then be transferred to the location specified in --repository_path (if --repository_path is not specified, then the repo will be downloaded into the output folder).

What does this change allow?

This achieves generation for libraries that are composed of more than one service or service version (e.g. bigtable which is composed of two services google/bigtable/v2 and google/bigtable/admin/v2, dialogflow which is composed of two service versions google/cloud/dialogflow/v2 and google/cloud/dialogflow/v2beta1), with its root problem being owlbot.py files that assume several service versions existing in the owl-bot-staging folder.

Generating a set of libraries with their respective GAPICs from a single configuration yaml

This PR enhances the list of libraries in the integration test generation_config.yaml

- api_shortname: speech
library_type: GAPIC_AUTO
GAPICs:
- proto_path: google/cloud/speech/v1
- proto_path: google/cloud/speech/v1p1beta1
- proto_path: google/cloud/speech/v2
- api_shortname: apigee-connect
library_type: GAPIC_AUTO
GAPICs:
- proto_path: google/cloud/apigeeconnect/v1
- api_shortname: dialogflow

They take advantage of common configuration entries at the root of the configuration yaml

Trying it locally

For example, we will clone google-cloud-java in /tmp...
image

...and remove a few source files from java-asset
image

Then we use the test configuration yaml (for convenience I only defined the asset service to speed up) to generate a few libraries, including all versions of asset
image

The resulting generation only modified the README.md file (which is ignored in our integration tests)
image

@diegomarquezp
Copy link
Contributor Author

I disabled the compute/v1 test

https://github.com/googleapis/sdk-platform-java/actions/runs/7674168459/job/20918240804

--- a/java-compute/google-cloud-compute/src/main/java/com/google/cloud/compute/v1/stub/ZonesStubSettings.java
+++ b/java-compute/google-cloud-compute/src/main/java/com/google/cloud/compute/v1/stub/ZonesStubSettings.java
@@ -51,7 +51,6 @@ import com.google.common.collect.Lists;
 import java.io.IOException;
 import java.util.List;
 import javax.annotation.Generated;
-import org.threeten.bp.Duration;
 
 // AUTO-GENERATED DOCUMENTATION AND CLASS.
 /**
@@ -266,11 +265,7 @@ public class ZonesStubSettings extends StubSettings<ZonesStubSettings> {
     static {
       ImmutableMap.Builder<String, ImmutableSet<StatusCode.Code>> definitions =
           ImmutableMap.builder();
-      definitions.put(
-          "retry_policy_0_codes",
-          ImmutableSet.copyOf(
-              Lists.<StatusCode.Code>newArrayList(
-                  StatusCode.Code.DEADLINE_EXCEEDED, StatusCode.Code.UNAVAILABLE)));
+      definitions.put("no_retry_codes", ImmutableSet.copyOf(Lists.<StatusCode.Code>newArrayList()));
       RETRYABLE_CODE_DEFINITIONS = definitions.build();
     }
 

There is some retry code problem, and I'm clueless. @JoeWang1127 could this be some missing additional proto problem?

@JoeWang1127
Copy link
Collaborator

There is some retry code problem, and I'm clueless. @JoeWang1127 could this be some missing additional proto problem?

I think the issue is that the grpc_service_config in BUILD has a leading ":".

This case is considered when parsing gapic option using shell script (link).

However, I didn't add the similar logic in the corresponding python script.

Copy link
Member

@suztomo suztomo left a comment

Choose a reason for hiding this comment

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

Thank you for confirmation with the java-asset.

@diegomarquezp
Copy link
Contributor Author

There is some retry code problem, and I'm clueless. @JoeWang1127 could this be some missing additional proto problem?

I think the issue is that the grpc_service_config in BUILD has a leading ":".

This case is considered when parsing gapic option using shell script (link).

However, I didn't add the similar logic in the corresponding python script.

@JoeWang1127 it was indeed a problem with a colon in the service config. It's now fixed, thanks!

#Optional. The root folder name of generated client libraries. If empty, modules will be created under current folder, useful for single module
destination-path: google-cloud-java
synthtool_commitish: fac8444edd5f5526e804c306b766a271772a3e2f
#Required. The root folder name of generated client libraries. The value "google-cloud-java" implies special treatment in the lower level scripts
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does "google-cloud-java" still implies special treatment in the lower level scripts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's more precise to say "more than 1 library". I modified the comments of that yaml to explain this.

# inspects a $build_file for a certain $rule (e.g. java_gapic_library). If the
# first 15 lines after the declaration of the rule contain $pattern, then
# it will return $if_match if $pattern is found, otherwise $default
__get_config_from_BUILD() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, I think all the parsing utils in this class can be removed now?

Copy link

Quality Gate Passed Quality Gate passed for 'gapic-generator-java-root'

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link

Quality Gate Passed Quality Gate passed for 'java_showcase_integration_tests'

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@diegomarquezp diegomarquezp merged commit 363e35e into main Jan 29, 2024
46 checks passed
@diegomarquezp diegomarquezp deleted the postprocess-hw-libs branch January 29, 2024 18:09
diegomarquezp added a commit that referenced this pull request Mar 4, 2024
re-enables the changes from
#2342 as part of
[milestone
3.2](https://docs.google.com/document/d/1v-sJBmdNEVBRryL8n90XK8OIiqVphaJemXVhANw14Jk/edit?pli=1&resourcekey=0-QGUb2do-JBlDWKvptWBp_g&tab=t.0#bookmark=id.kl5fzqdav6u7)
of Hermetic Build

### Changes:
 - add support for HW libs in `integration_tests.py`
 - adapt configuration of docker volumes
 - adapt usage of `fix-poms.py` to use monorepo and split_repo modes
- fix temp_destination_path in `generate_composed_library.py` in order
to avoid collisions
- For example `google/bigtable/v2` and `google/bigtable/admin/v2` would
both be sent to a temporal `java-bigtable-v2` folder, spoiling the files
being transferred via `copy-code`.
- The fix is to have a `proto_path`-based folder name to achieve
uniqueness

### After this PR:
- [ ] Reconfigure the nightly CI in google-cloud-java to use the updated
volume mapping
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: xl Pull request size is extra large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants