Skip to content

feat: move Java Owlbot into this repository for postprocessing #2282

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.

Sign up for GitHub

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

Jump to bottom
Merged
diegomarquezp merged 74 commits into from
Dec 18, 2023
Merged

feat: move Java Owlbot into this repository for postprocessing #2282

diegomarquezp merged 74 commits into from
Dec 18, 2023

Conversation

@diegomarquezp
Copy link
Contributor

@diegomarquezp diegomarquezp commented Dec 1, 2023 •
edited
Loading

This is stage 1 of the Owlbot replacement project.
This PR moves the implementation of Synthtool's Java Owlbot to this repository.

Changes

  • Synthtool will be used as an external tool by cloning it and running pip install
  • The Java Owlbot implementation has been transferred to sdk-platform-java
  • The non-java-specific Owlbot CLI (copy-code) remains referenced as a docker image
  • The interface of postprocess_library.sh will be modified (see next section). It will still, however, be intended to be called by generate_library.sh. It will still be expected to be run on pre-existing fully functional GAPIC libraries.

New interface of postprocess_library.sh

We will define the interface of postprocess_library.sh as follows:

Input parameters

Argument Required Description
postprocessing_target yes This is the main parameter to the script. Java Owlbot runs its scripts in this target directory and modifies it (hence this being also an output). Some files are expected to be here
  • owlbot.py used by java-owlbot
  • .OwlBot.yaml (optional) used to transfer code from preprocessed_sources_path
  • .repo-metadata.json - essential metadata file used by java owlbot

    In the context of generate_library.sh, destination_path should be pointing to a folder with such files, since it is destination_path that will be called as postprocessing_target
versions_file yes Versions to be applied to README.md and pom files. This is done by java owlbot
preprocessed_sources_path yes This folder is the input preprocessed libraries, which should look like a googleapis-gen folder (example). The script will infer the necessary `proto_path` variable from the proto library. .

In the perspective of generate_library.sh, the script will generate a temporary folder with such structure and will pass it to postprocess_library.sh

Outputs

Artifact Description
postprocessing_target Also passed as input to the script. The post-processor will modify the files contained here. It will also use copy-code to transfer preprocessed java sources into this folder if preprocessed_sources_path is provided..

Remarks

  • Since synthtool and java owlbot run on python, a python version will be stored in library_generation/configuration. This will be managed via pyenv
  • Since synthtool is designed to run on Linux only, the integration test with post-processing for macos were disabled. We will not follow up in a later stage by creating a synthtool-only image (POC)
  • google-java-format was originally added in the image definition. The scripts will now download it at runtime. The formatter requires java 11, so the github integration test was updated to run on java 11.
  • Since destination_path was previously being used as a pre-existing, fully functional GAPIC library when postprocessing is enabled, it will now clash with the java owlbot postprocessor by having unexpected folders in the GAPIC folder (i.e. preprocessed libraries produced by generate_library.sh will be sent to, for example, google-cloud-java/java-.../ which will produce a library with pre-processed folders). This is dealt with by having a temp_destination_path in generate_library.sh that we will either (1) use it as preprocessed_sources_path when postprocessing is enabled or (2) simply transfer its contents to destination_path when it's not enabled.
  • Due to Hermetic Build integration tests fail when googleapis have proto changes not yet reflected in google-cloud-java or googleapis-gen #2303, I temporarily disabled the tests for google/cloud/asset/v1p2beta1 and google/cloud/compute

@diegomarquezp
use local synthtool and owlbot
1394f1c
@diegomarquezp
remove unused files
bf80792
@diegomarquezp
remove more unused files
ce3e9a2
@diegomarquezp
remove cache files in owlbot
76d4892
@diegomarquezp
use java 11 for it
bd00a72
@diegomarquezp
remove kokoro files
988132e
@diegomarquezp
use glob in owlbot entrypoint
5b1b70e
@diegomarquezp
remove unused files
5340c39
@diegomarquezp
do not do post-process IT on mac
c62a373
@diegomarquezp
concise entrypoint logic
3f28553
@diegomarquezp
cleanup i
46a0c89
@diegomarquezp
cleanup ii
ddf202e
@diegomarquezp
cleanup iii
7cfbd6f
@diegomarquezp
cleanup iv
a2b843d
@diegomarquezp
remove templates
35b7916
@diegomarquezp
remove protos folder
81d4d60
@diegomarquezp
remove synthtool
1e9a788
@diegomarquezp
connect image to owlbot entrypoint
d99e0ae
@diegomarquezp
simplify synthtool docker run command
b1baed7
@diegomarquezp
install synthtool locally
860c45b
@diegomarquezp
install synthtool only once
6755cd1
@diegomarquezp
Merge remote-tracking branch 'origin/main' into local-owlbot-java-owl…
6ecef4e 
…bot-only
@diegomarquezp
use virtualenvs to run python scripts
558118d
@diegomarquezp
install pyenv in action
f8183f8
@diegomarquezp
remove jar from history
bdb6cd0
@diegomarquezp
download google-java-format
e42fc0f
@product-auto-label product-auto-label bot added the size: xl Pull request size is extra large. label Dec 1, 2023
@diegomarquezp diegomarquezp changed the title feat: move Java Owlbot into this respository for postprocessing feat: move Java Owlbot into this repository for postprocessing Dec 1, 2023
JoeWang1127
JoeWang1127 reviewed Dec 13, 2023
View reviewed changes
@diegomarquezp diegomarquezp requested review from meltsufin and suztomo and removed request for meltsufin December 14, 2023 18:01
suztomo
suztomo approved these changes Dec 14, 2023
View reviewed changes
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.

Memo: As per the design doc, we won't use this until the end of Q1 2024.

blakeli0
blakeli0 reviewed Dec 15, 2023
View reviewed changes
suztomo
suztomo reviewed Dec 15, 2023
View reviewed changes
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.

Until we start using this postprocessor (around Feb/March 2024), people, including me, continue make changes to the Owlbot Java postprocessor via the synthtool repository. Would you write some README section what steps are needed to copy these changes in future (when?)?

@blakeli0
Copy link
Contributor

blakeli0 commented Dec 16, 2023

Until we start using this postprocessor (around Feb/March 2024), people, including me, continue make changes to the Owlbot Java postprocessor via the synthtool repository. Would you write some README section what steps are needed to copy these changes in future (when?)?

That's valid concern. How often do we usually update owlbot post-processor? We thought it does not change often so expected it to not change in another two months. But if it does change, I think we can either

  1. Update the post-processing logic here as well when we make changes to synthtool.
  2. Don't do copy the changes over until the final cut over, we can copy all the changes in the past few months at that time in one shot.

blakeli0
blakeli0 approved these changes Dec 16, 2023
View reviewed changes
Copy link
Contributor

@blakeli0 blakeli0 left a comment

Choose a reason for hiding this comment

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

LGTM. As we discussed offline, please look into the possibility of moving the Java specific Github templates into this repo, but it can be a separate PR.

meltsufin
meltsufin approved these changes Dec 18, 2023
View reviewed changes
Copy link
Member

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

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

Nice work @diegomarquezp!

@diegomarquezp
Copy link
Contributor Author

diegomarquezp commented Dec 18, 2023

Until we start using this postprocessor (around Feb/March 2024), people, including me, continue make changes to the Owlbot Java postprocessor via the synthtool repository. Would you write some README section what steps are needed to copy these changes in future (when?)?

@suztomo Done

@diegomarquezp
Copy link
Contributor Author

diegomarquezp commented Dec 18, 2023 •
edited
Loading

Until we start using this postprocessor (around Feb/March 2024), people, including me, continue make changes to the Owlbot Java postprocessor via the synthtool repository. Would you write some README section what steps are needed to copy these changes in future (when?)?

That's valid concern. How often do we usually update owlbot post-processor? We thought it does not change often so expected it to not change in another two months. But if it does change, I think we can either

  1. Update the post-processing logic here as well when we make changes to synthtool.
  2. Don't do copy the changes over until the final cut over, we can copy all the changes in the past few months at that time in one shot.

@blakeli0 I missed point 2 when modifying the README. I mentioned in the README that there were slight tweaks here and there in the transferred owlbot implementation. Whether we modify them within or after the two coming months, the instructions apply to both cases equally.

edit: I would lean for avoiding modifications until end up using this implementation, then we can transfer all the changes since this PR and then.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 18, 2023

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

@diegomarquezp
Copy link
Contributor Author

diegomarquezp commented Dec 18, 2023

IT for google/cloud/asset/v1 failing as recent changes in googleapis have not been reflected in google-cloud-java (pending PR: googleapis/google-cloud-java#10138)

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 18, 2023

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 f8969d2 into main Dec 18, 2023
@diegomarquezp diegomarquezp deleted the move-java-owlbot branch December 18, 2023 15:54
@release-please release-please bot mentioned this pull request Dec 18, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@suztomo suztomo suztomo approved these changes

@blakeli0 blakeli0 blakeli0 approved these changes

@meltsufin meltsufin meltsufin approved these changes

@JoeWang1127 JoeWang1127 Awaiting requested review from JoeWang1127

Assignees

No one assigned

Labels

hermetic-build size: xl Pull request size is extra large.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@diegomarquezp @blakeli0 @suztomo @meltsufin @JoeWang1127