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

Try enabling gradle enterprise #4923

Merged
merged 1 commit into from
Jun 8, 2023
Merged

Conversation

jrhee17
Copy link
Contributor

@jrhee17 jrhee17 commented Jun 5, 2023

Motivation:

Note that github secrets are not available for actions triggered by forked pull requests: ref

We have two choices

  1. Using pull_request events:
  • For push (merge) events, secrets are applied and the build cache is updated
  • For pull requests from forked repositories, the build cache isn't updated. This means we can't take advantage of the build cache when checking out and reviewing pull requests locally.
  • We can expect fewer cache uploaded, but higher hit rate
  1. Using pull_request_target events:
  • For all opened pull requests, secrets are applied and the build cache is updated
  • For this type of event, the .github workflow files from the base commit is used. This means updating the .github workflow file can be tedious. (e.g. we need to merge changes we want to test to our forked repo, and then open a PR to verify changes)

I think it's fine to go with option 1 for now. Perhaps we can pivot to option 2 later depending on how much pressure our gradle enterprise instance is experiencing.
Unfortunately, the only way to test if this PR "works" is to push this commit to the main branch.

Test result from my forked repo:

The options provided in settings.gradle are straightforward from Step 1 in the provided manual.

Modifications:

  • Add a environment variable for gradle enterprise access key
  • Add gradle enterprise configurations in settings.gradle

Result:

  • Gradle enterprise integration

@jrhee17 jrhee17 force-pushed the poc/gradle-enterprise branch 3 times, most recently from 4e12352 to a2e4737 Compare June 7, 2023 06:00
@jrhee17 jrhee17 added this to the 1.24.0 milestone Jun 7, 2023
@jrhee17 jrhee17 marked this pull request as ready for review June 7, 2023 14:54
@jrhee17 jrhee17 modified the milestones: 1.24.0, 1.25.0 Jun 7, 2023
Copy link
Member

@trustin trustin left a comment

Choose a reason for hiding this comment

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

🚀

Copy link
Member

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

👍 👍 👍

Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

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

Thanks, @jrhee17! 👍

@@ -18,6 +18,7 @@ concurrency:
env:
LC_ALL: "en_US.UTF-8"
BUILD_JDK_VERSION: "19"
GRADLE_ENTERPRISE_ACCESS_KEY: ${{ secrets.GRADLE_ENTERPRISE_ACCESS_KEY }}
Copy link
Contributor

Choose a reason for hiding this comment

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

@ikhoon ikhoon merged commit b808c3e into line:main Jun 8, 2023
@ikhoon ikhoon modified the milestones: 1.25.0, 1.24.0 Jun 8, 2023
ikhoon pushed a commit to be-hase/armeria that referenced this pull request Jun 12, 2023
Motivation:

Note that github secrets are not available for actions triggered by forked pull requests: [ref](https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/#improvements-for-public-repository-forks)

We have two choices
1. Using `pull_request` events:
- For push (merge) events, secrets are applied and the build cache is updated
- For pull requests from forked repositories, the build cache isn't updated. This means we can't take advantage of the build cache when checking out and reviewing pull requests locally.
- We can expect fewer cache uploaded, but higher hit rate
2. Using `pull_request_target` events:
- For all opened pull requests, secrets are applied and the build cache is updated
- For this type of event, the `.github` workflow files from the base commit is used. This means updating the `.github` workflow file can be tedious. (e.g. we need to merge changes we want to test to our forked repo, and then open a PR to verify changes)

I think it's fine to go with option 1 for now. Perhaps we can pivot to option 2 later depending on how much pressure our gradle enterprise instance is experiencing.
Unfortunately, the only way to test if this PR "works" is to push this commit to the `main` branch.

Test result from my forked repo:
- PR: jrhee17#10
- Build scan: https://ge.armeria.dev/s/hcdbl7vdcqbhc
- CI: https://github.com/jrhee17/armeria/actions/runs/5201312954/jobs/9381342458#step:5:1601

The options provided in `settings.gradle` are straightforward from Step 1 in the provided manual.

Modifications:

- Add a environment variable for gradle enterprise access key
- Add gradle enterprise configurations in `settings.gradle`

Result:

- Gradle enterprise integration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants