feat: create pr after library generation #10503

JoeWang1127 · 2024-03-08T00:42:36Z

In this PR:

Update workflow to create a PR of hermetic code generation.
- search baseline commit (B) in generation_config.yaml.
- search latest commit (L) in googleapis repository.
- update googleapis commit to L in generation_config.yaml.
- search for open pull request associated with branch generate-libraries.
  - create a new branch if no pull request is not found; otherwise checkout the pull request.
- generate GAPIC libraries from generation_config.yaml.
- generate pull request description from B to L.
- create a pull request or push a commit.
Update generation_config.yaml to include new libraries and new versions.

Context: this is the launch plan of hermetic build project.

suztomo · 2024-03-11T17:04:55Z

.github/workflows/verify-generation-config.yaml

+      run: |
+        title="chore: generate libraries at $(date)"
+        git add java-* pom.xml gapic-libraries-bom/pom.xml versions.txt generation_config.yaml
+        git commit -m "chore: ${title}"


"chore" appears twice.

Updated the commit message.

suztomo · 2024-03-11T17:07:06Z

.github/workflows/verify-generation-config.yaml

+    - name: create or update the pull request
+      shell: bash
+      run: |
+        title="chore: generate libraries at $(date)"


My instinct says "chore:" does not goes to Release Please pull request and the pull request description is not read by Release Please. Let's see.

(If this is really the case, we can fix it by "BEGIN_COMMIT_OVERRIDE".)

I feel the same way. Can we make sure we understand the behavior before merging? e.g. If it is a chore, is release please going to include the NESTE_COMMIT in the description? If it is a feat, is the title going to be included? The behavior of release-please will affect how we generate the PR descriptions.

According to @chingor13, release-please won't skip a commit if it's a chore, it just ignores chore commit messages.

blakeli0 · 2024-03-11T17:10:26Z

Is this PR an example of the generated PR?

JoeWang1127 · 2024-03-11T17:12:12Z

Is this PR an example of the generated PR?

Yes.

JoeWang1127 · 2024-03-11T17:42:30Z

generation diff/root-pom and generation diff/gapic-bom failed because the sorted entries in pom.xml are in different order when using shell sort and python sorted.

The content is the same, thus no impact on build.

.github/workflows/verify-generation-config.yaml

blakeli0 · 2024-03-11T17:42:56Z

.github/workflows/verify-generation-config.yaml

+    - name: create or update the pull request
+      shell: bash
+      run: |
+        title="chore: generate libraries at $(date)"


I feel the same way. Can we make sure we understand the behavior before merging? e.g. If it is a chore, is release please going to include the NESTE_COMMIT in the description? If it is a feat, is the title going to be included? The behavior of release-please will affect how we generate the PR descriptions.

blakeli0 · 2024-03-11T17:46:11Z

.github/workflows/verify-generation-config.yaml

+          git fetch -q --unshallow monorepo
+          git push -f monorepo "${branch_name}"
+          set -x
+          gh pr create --title "${title}" --label "owlbot:run" --head "${branch_name}" --body "$(cat pr_description.txt)"


Why do we still need owlbot:run here?

The check is not removed yet.

I think we should remove the label here and disable the check for this branch, otherwise owlbot postprocessing would still be run against this PR right?

+1 to removing the required check for owlbot postprocessor and the label

blakeli0 · 2024-03-11T17:53:12Z

.github/workflows/verify-generation-config.yaml

@@ -30,3 +72,40 @@ jobs:
          -e "REPO_BINDING_VOLUMES=${repo_volumes}" \
          gcr.io/cloud-devrel-public-resources/java-library-generation:latest \


Ideally we should not use latest, but rather a versioned docker image, otherwise it is not hermetic. But since this is the first time we use the scripts and there might be bug fixes in this cycle, let's keep it for now. Once the scripts are stable, we should read the gapic-generator-java version from the build config file and use that version for the docker image.
For now, can we extract latest as a parameter and share it between this step and the step below?

Just FYI, we have a release job that releases gcr.io/cloud-devrel-public-resources/java-library-generation:${GGJ_VERSION}, so if we are to remove it, we can use that tag (and configure renovate bot)

Once the scripts are stable, we should read the gapic-generator-java version from the build config file and use that version for the docker image.

I think we should group the docker image tag with generator update so that renovate bot can update them together.

For now, can we extract latest as a parameter and share it between this step and the step below?

Sounds good.

blakeli0 · 2024-03-11T20:01:37Z

.github/workflows/generate-from-configuration.yaml

+  generate-from-configuration:
+    runs-on: ubuntu-22.04
+    env:
+      branch_name: generate-libraries


This could be a follow up PR. Can we make this a parameter so that we can test the Github action in a separate branch? Otherwise it would always override the current PR if it is not merged.

Otherwise it would always override the current PR if it is not merged

This is an intended behavior because we don't want a lot of open PRs in the repo.

This is the description of the workflow.

In addition, can we change the branch name to include the branch name, something like generate-libraries-main? As we would need to support LTS and multiple Java versions in the future.

Otherwise it would always override the current PR if it is not merged

This is an intended behavior because we don't want a lot of open PRs in the repo.

This is the description of the workflow.

I agree with this behavior. What I'm suggesting is to make it more flexible so that we can test the Github action without overriding the PR.

blakeli0

LGTM. I think this PR should be a chore as it does not really change any code, otherwise it would appear in the release notes.
A few follow up enhancements that I can think of:

Make this Github action more generic so that it can be reused for other use cases. e.g. Extract the target branch, googleapis commitish to parameters, do not always update googleapis commitish etc.
Encapsulate the PR creation logic in the docker image. So that it can be tested independently and more hermetic.
Create a separate Github action or reuse this Github action to automatically update renovate bot PRs for gapic-generator-java version update.

blakeli0 · 2024-03-11T20:11:12Z

.github/workflows/generate-from-configuration.yaml

+  generate-from-configuration:
+    runs-on: ubuntu-22.04
+    env:
+      branch_name: generate-libraries


In addition, can we change the branch name to include the branch name, something like generate-libraries-main? As we would need to support LTS and multiple Java versions in the future.

blakeli0 · 2024-03-11T20:12:03Z

.github/workflows/generate-from-configuration.yaml

+          git fetch -q --unshallow monorepo
+          git push -f monorepo "${branch_name}"
+          set -x
+          gh pr create --title "${title}" --head "${branch_name}" --body "$(cat pr_description.txt)"


I guess this creates a PR against main branch by default? Can we make the target branch a parameter as well?

blakeli0 · 2024-03-11T20:25:16Z

.github/workflows/generate-from-configuration.yaml

+    - name: create or update the pull request
+      shell: bash
+      run: |
+        title="chore: generate libraries at $(date)"


The title generation could be moved to sdk-platform-java and/or inside the docker images.

blakeli0 · 2024-03-11T20:26:56Z

.github/workflows/generate-from-configuration.yaml

+    env:
+      branch_name: generate-libraries
+      library_generation_image_tag: latest
+      repo_volumes: "-v repo-google-cloud-java:/workspace/google-cloud-java"


Anything that references google-cloud-java should be extracted to a parameter, so that we can reuse this Github action in handwritten library repos.

I'll leave this one as-is and we'll refactor it once we prove this is working on google-cloud-java.

diegomarquezp · 2024-03-12T03:05:14Z

generation_config.yaml

@@ -167,6 +167,14 @@ libraries:
  GAPICs:
  - proto_path: google/appengine/v1

+- api_shortname: apphub


Which script do you use to populate this yaml?

I manually added these libraries, the parameters are copied from new library generation PR.

JoeWang1127 added 30 commits March 7, 2024 19:42

feat: create pr after library generation

a18841c

add comment

949c779

setup branch

fbdf767

create or update pr

5c3b52f

enable workflow

f1fb22f

setup github token

87d9c3a

select one library to verify the workflow

03af469

change format

86f3b9b

remove head

e859513

add quote

03193a3

shorten config for testsing

f5e36ac

add a library

84b5543

add head

668f6af

set remote repo

087982f

commit change in configuration

8f2f6cf

update googleapis commit

2702a5c

test body

adbeb2a

set x

9f9a7b4

set x for docker

b65be6f

set env

eabbc07

change env

ebffdbd

restore generation

8f955d7

remove gke v1alpha2

41cc413

add apphub

d0d0cdb

add chat

8acbc80

add cloudcontrolspartner (commented out)

0862a24

remove compute small

746676f

update committish

7f7c38d

restore cloudcontrolspartner

8b12617

update policytroubleshooter

8d698ab

JoeWang1127 added 2 commits March 11, 2024 15:52

use partial clone

0211cfe

use job level env

d336503

diegomarquezp approved these changes Mar 11, 2024

View reviewed changes

suztomo reviewed Mar 11, 2024

View reviewed changes

suztomo approved these changes Mar 11, 2024

View reviewed changes

suztomo reviewed Mar 11, 2024

View reviewed changes

change commit message

23190c4

Merge branch 'main' into feat/hermetic-pr-creation

6815a40

blakeli0 reviewed Mar 11, 2024

View reviewed changes

JoeWang1127 added 5 commits March 11, 2024 18:03

change workflow name

400025a

allow empty commit

fd27bab

extract image tag

8698923

remove owlbot:run tag

554938f

change workflow name

c964025

blakeli0 reviewed Mar 11, 2024

View reviewed changes

blakeli0 approved these changes Mar 11, 2024

View reviewed changes

blakeli0 reviewed Mar 11, 2024

View reviewed changes

JoeWang1127 added 2 commits March 11, 2024 23:00

define bash branch

a1dc0ce

remove branch dispatcher

b103130

JoeWang1127 enabled auto-merge (squash) March 12, 2024 00:34

JoeWang1127 merged commit cd29fa9 into main Mar 12, 2024
29 of 31 checks passed

JoeWang1127 deleted the feat/hermetic-pr-creation branch March 12, 2024 01:51

release-please bot mentioned this pull request Mar 12, 2024

chore(main): release 1.32.0 #10504

Closed

diegomarquezp reviewed Mar 12, 2024

View reviewed changes

This was referenced Mar 13, 2024

chore(main): release 1.32.0 #10533

Closed

chore(main): release 1.32.0 #10534

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: create pr after library generation #10503

feat: create pr after library generation #10503

JoeWang1127 commented Mar 8, 2024 •

edited

suztomo Mar 11, 2024

JoeWang1127 Mar 11, 2024

suztomo Mar 11, 2024 •

edited

blakeli0 Mar 11, 2024

JoeWang1127 Mar 11, 2024

blakeli0 commented Mar 11, 2024

JoeWang1127 commented Mar 11, 2024

JoeWang1127 commented Mar 11, 2024 •

edited

blakeli0 Mar 11, 2024

blakeli0 Mar 11, 2024

JoeWang1127 Mar 11, 2024

blakeli0 Mar 11, 2024

diegomarquezp Mar 11, 2024

JoeWang1127 Mar 11, 2024

blakeli0 Mar 11, 2024

diegomarquezp Mar 11, 2024 •

edited

JoeWang1127 Mar 11, 2024

blakeli0 Mar 11, 2024

JoeWang1127 Mar 11, 2024

blakeli0 Mar 11, 2024

blakeli0 Mar 11, 2024

JoeWang1127 Mar 11, 2024

blakeli0 left a comment

blakeli0 Mar 11, 2024

blakeli0 Mar 11, 2024

JoeWang1127 Mar 11, 2024

blakeli0 Mar 11, 2024

blakeli0 Mar 11, 2024

JoeWang1127 Mar 11, 2024

diegomarquezp Mar 12, 2024

JoeWang1127 Mar 12, 2024

		@@ -30,3 +72,40 @@ jobs:
		-e "REPO_BINDING_VOLUMES=${repo_volumes}" \
		gcr.io/cloud-devrel-public-resources/java-library-generation:latest \

feat: create pr after library generation #10503

feat: create pr after library generation #10503

Conversation

JoeWang1127 commented Mar 8, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suztomo Mar 11, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blakeli0 commented Mar 11, 2024

JoeWang1127 commented Mar 11, 2024

JoeWang1127 commented Mar 11, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diegomarquezp Mar 11, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blakeli0 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoeWang1127 commented Mar 8, 2024 •

edited

suztomo Mar 11, 2024 •

edited

JoeWang1127 commented Mar 11, 2024 •

edited

diegomarquezp Mar 11, 2024 •

edited