-
Notifications
You must be signed in to change notification settings - Fork 111
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
Use CodeBuild-hosted runners for perf testing and automated publishing #801
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #801 +/- ##
============================================
+ Coverage 67.23% 67.43% +0.19%
- Complexity 5484 5522 +38
============================================
Files 159 160 +1
Lines 23025 23077 +52
Branches 4126 4126
============================================
+ Hits 15481 15562 +81
+ Misses 6262 6243 -19
+ Partials 1282 1272 -10 ☔ View full report in Codecov by Sentry. |
# of the project you created. | ||
name: Select Runner Type | ||
runs-on: ubuntu-latest | ||
# We want to run on external PRs, but not on internal ones as push automatically builds |
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.
Can you help me understand this better? Where exactly do we not want to use CodeBuild, and why? And we fall back to ubuntu-latest
when CodeBuild is disabled?
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 thought that this would be enough to explain. Feel free to suggest additions.
If you want to use codebuild runners for your personal fork, follow the instructions to set
up a codebuild project. https://docs.aws.amazon.com/codebuild/latest/userguide/action-runner.html
Then, create a repository variable for your fork namedCODEBUILD_PROJECT_NAME
with the name
of the project you created.
Basically, if you want the perf workflow to run when you push to your fork of the repo, either you need to have codebuild runners setup (which will cost money), or we need to have a fallback option.
I suppose, though, that we might decide that any results on a different runner type are too different to be even the slightest bit valid. In that case, we might rewrite this workflow so that it is entirely skipped if there are no codebuild runners.
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 part of my confusion is around the fact that I don't know the intended definition of "external" and "internal" PRs. Looking more closely, it looks like "internal" PRs are ones submitted directly against amazon-ion/ion-java
, while "external" PRs covers everything else. And so this block is here just to help, e.g., users with forks still be able to run the perf workflow, falling back to Ubuntu if they don't have CodeBuild. Is that right?
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.
Oh, the "internal"/"external" PRs is just to prevent duplicate workflows, and it's something that is already used in our other workflows. When you create a PR from a fork repo to a source repo and a workflow has both push
and pull_request
triggers, it kicks off the Github workflow for both the push (GitHub pushes PR commits to the target repo somehow) and the actual pull request. The workaround is actually on L58.
- uses: gradle/gradle-build-action@8baac4c8ef753599f92eeb509c246d09d6250fa6 # v3.3.0 | ||
with: | ||
arguments: build cyclonedxBom | ||
arguments: build cyclonedxBom publishToSonatype closeAndReleaseSonatypeStagingRepository |
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.
This is the main difference, right? If I understand correctly it means we won't have to log into the web UI and manually click through to release the staged repo, right?
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.
Yes—that's correct.
Issue #, if available:
None
Description of changes:
I must qualify this by saying that I have not tested the publish workflow because to do so I would have to create a new release. It is possible to fallback to the existing manual release process, and I will validate the automated process when we publish the next release.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.