-
Notifications
You must be signed in to change notification settings - Fork 53
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: move synthtool templates to library_generation/owlbot
#2443
Conversation
Here is the summary of possible violations 😱 There is a possible violation for not having product prefix.
There is a format violation for a region tag.
The end of the violation section. All the stuff below is FYI purposes only. Here is the summary of changes. You are about to add 1 region tag.
This comment is generated by snippet-bot.
|
) | ||
|
||
|
||
def parse_template_excludes(owlbot_py_contents: str) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think parsing generated owlbot.py
is not intuitive.
Can we change the implementation of java.common_templates
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
java.common_templates
lives in synthtool
If we want to change the implementation of that function we would need to transfer synthtool to sdk-platform-java (which is an actual option).
Also from the PR description, we could modify all owlbot.py files (and its template) to call our_own_owlbot.common_templates
instead of java.common_templates
cc: @suztomo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed with @suztomo, I'll use the configuration yaml to have a list of exclusions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I modified the implementation to use the configuration yaml
pyyaml | ||
absl-py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need these two packages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
Co-authored-by: Joe Wang <[email protected]>
…ove-synthtool-templates
https://github.com/googleapis/sdk-platform-java/pull/2443/checks?check_run_id=21380060834 |
https://github.com/googleapis/sdk-platform-java/pull/2443/checks?check_run_id=21378993952
Adding a step to install via |
@@ -4,3 +4,4 @@ ignoreFiles: | |||
- src/test/** | |||
- test/** | |||
- showcase/** | |||
- library_generation/owlbot/templates/java_library/samples/install-without-bom/pom.xml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#2443 (comment) has the context. Basically the region tag check thinks that template has a region tag (wrong). I'm yet to get a response from the automation team
Since you format Python code in |
Quality Gate passed for 'gapic-generator-java-root'Issues Measures |
Quality Gate passed for 'java_showcase_integration_tests'Issues Measures |
🤖 I have created a release *beep* *boop* --- <details><summary>2.35.0</summary> ## [2.35.0](v2.34.0...v2.35.0) (2024-02-13) ### Features * add `generate_repo.py` ([#2431](#2431)) ([47b632a](47b632a)) * move synthtool templates to `library_generation/owlbot` ([#2443](#2443)) ([5c95472](5c95472)) ### Bug Fixes * Apiary Host returns user set host if set ([#2455](#2455)) ([5f17e62](5f17e62)) * Cancel the Timeout Task for HttpJson ([#2360](#2360)) ([b177d9e](b177d9e)) ### Dependencies * update dependency commons-codec:commons-codec to v1.16.1 ([#2473](#2473)) ([8c6e91d](8c6e91d)) * update google api dependencies ([#2469](#2469)) ([ad4d4e6](ad4d4e6)) * update google auth library dependencies to v1.23.0 ([#2466](#2466)) ([349a5d3](349a5d3)) * update google auth library dependencies to v1.23.0 ([#2476](#2476)) ([6c9127c](6c9127c)) * update google http client dependencies to v1.44.1 ([#2467](#2467)) ([87d1435](87d1435)) * update googleapis/java-cloud-bom digest to ac9893c ([#2472](#2472)) ([7fff34a](7fff34a)) * update grpc dependencies to v1.61.1 ([#2463](#2463)) ([9ec575b](9ec575b)) </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 transfers the java-specific templates from synthtool to `library_geneation/owlbot` There is a new branch in synthtool (googleapis/synthtool#1923) that has the removed templates. The intention is to keep that synthtool branch as parallel until we fully roll out the hermetic build workflows to both HW libraries and the monorepo. ### Approach We add a list of template exclusions to the configuration yaml and call `java.common_templates` with a custom template path (pointing to our OwlBot Java Postprocessor implementation here in `library_generation` plus the template exclusions found in the yaml. The list of exclusions were obtained from owlbot.py files. Since google-cloud-java has the same exclusions for all libraries, we use a repo-level configuration entry. An example of template excludes is: https://github.com/googleapis/sdk-platform-java/blob/b0a57b70d589e2bdc6e5fcb8cf64d08c3496bc57/java-iam/owlbot.py#L26-L37 ### Different approach possible? We could modify all owlbot.py files to use something other than `java.common_templates`. For example ``` from custom_owlbot import common_templates ... common_templates(excludes=[/*preserved excludes*/]) ``` With such approach, we would not have to parse owlbot.py files and take advantage of the fact it's an executable script. ## Follow up? We probably don't want to call `common_templates` twice, so it may be better to modify owlbot.py files to reference our own implementation instead of synthtool. (This is similar to "Different approach possible?" but it is more of a follow up once the scripts are live). --------- Co-authored-by: Joe Wang <[email protected]>
🤖 I have created a release *beep* *boop* --- <details><summary>2.35.0</summary> ## [2.35.0](v2.34.0...v2.35.0) (2024-02-13) ### Features * add `generate_repo.py` ([#2431](#2431)) ([47b632a](47b632a)) * move synthtool templates to `library_generation/owlbot` ([#2443](#2443)) ([5c95472](5c95472)) ### Bug Fixes * Apiary Host returns user set host if set ([#2455](#2455)) ([5f17e62](5f17e62)) * Cancel the Timeout Task for HttpJson ([#2360](#2360)) ([b177d9e](b177d9e)) ### Dependencies * update dependency commons-codec:commons-codec to v1.16.1 ([#2473](#2473)) ([8c6e91d](8c6e91d)) * update google api dependencies ([#2469](#2469)) ([ad4d4e6](ad4d4e6)) * update google auth library dependencies to v1.23.0 ([#2466](#2466)) ([349a5d3](349a5d3)) * update google auth library dependencies to v1.23.0 ([#2476](#2476)) ([6c9127c](6c9127c)) * update google http client dependencies to v1.44.1 ([#2467](#2467)) ([87d1435](87d1435)) * update googleapis/java-cloud-bom digest to ac9893c ([#2472](#2472)) ([7fff34a](7fff34a)) * update grpc dependencies to v1.61.1 ([#2463](#2463)) ([9ec575b](9ec575b)) </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>
🤖 I have created a release *beep* *boop* --- <details><summary>2.35.0</summary> ## [2.35.0](v2.34.0...v2.35.0) (2024-02-13) ### Features * add `generate_repo.py` ([#2431](#2431)) ([47b632a](47b632a)) * move synthtool templates to `library_generation/owlbot` ([#2443](#2443)) ([5c95472](5c95472)) ### Bug Fixes * Apiary Host returns user set host if set ([#2455](#2455)) ([5f17e62](5f17e62)) * Cancel the Timeout Task for HttpJson ([#2360](#2360)) ([b177d9e](b177d9e)) ### Dependencies * update dependency commons-codec:commons-codec to v1.16.1 ([#2473](#2473)) ([8c6e91d](8c6e91d)) * update google api dependencies ([#2469](#2469)) ([ad4d4e6](ad4d4e6)) * update google auth library dependencies to v1.23.0 ([#2466](#2466)) ([349a5d3](349a5d3)) * update google auth library dependencies to v1.23.0 ([#2476](#2476)) ([6c9127c](6c9127c)) * update google http client dependencies to v1.44.1 ([#2467](#2467)) ([87d1435](87d1435)) * update googleapis/java-cloud-bom digest to ac9893c ([#2472](#2472)) ([7fff34a](7fff34a)) * update grpc dependencies to v1.61.1 ([#2463](#2463)) ([9ec575b](9ec575b)) </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 transfers the java-specific templates from synthtool to
library_geneation/owlbot
There is a new branch in synthtool (googleapis/synthtool#1923) that has the removed templates.
The intention is to keep that synthtool branch as parallel until we fully roll out the hermetic build workflows to both HW libraries and the monorepo.
Approach
We add a list of template exclusions to the configuration yaml and call
java.common_templates
with a custom template path (pointing to our OwlBot Java Postprocessor implementation here inlibrary_generation
plus the template exclusions found in the yaml.The list of exclusions were obtained from owlbot.py files. Since google-cloud-java has the same exclusions for all libraries, we use a repo-level configuration entry. An example of template excludes is:
sdk-platform-java/java-iam/owlbot.py
Lines 26 to 37 in b0a57b7
Different approach possible?
We could modify all owlbot.py files to use something other than
java.common_templates
. For exampleWith such approach, we would not have to parse owlbot.py files and take advantage of the fact it's an executable script.
Follow up?
We probably don't want to call
common_templates
twice, so it may be better to modify owlbot.py files to reference our own implementation instead of synthtool. (This is similar to "Different approach possible?" but it is more of a follow up once the scripts are live).