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.

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

Use CodeBuild-hosted runners for perf testing and automated publishing #801

Merged
merged 4 commits into from
Apr 23, 2024
Merged

Use CodeBuild-hosted runners for perf testing and automated publishing #801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ad9b321
Use CodeBuild-hosted GitHub Actions runners for perf test and automat…
popematt Apr 22, 2024
bc84ae5
Update a src file to trigger the performance workflow
popematt Apr 22, 2024
09c34f4
Fix incorrect string quoting in performance workflow
popematt Apr 22, 2024
495e813
Merge branch 'master' into codebuild-runners
popematt Apr 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions .github/workflows/ion-java-performance-regression-detector.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,42 @@ jobs:
name: ${{env.test_data_id}}
path: testData

select-runner-type:
# 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 named `CODEBUILD_PROJECT_NAME` with the name
# 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
Copy link
Contributor

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?

Copy link
Contributor Author

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 named CODEBUILD_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.

Copy link
Contributor

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?

Copy link
Contributor Author

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.

# H/T: https://github.com/Dart-Code/Dart-Code/commit/612732d5879730608baa9622bf7f5e5b7b51ae65
if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name != 'amazon-ion/ion-java'
env:
use-codebuild: ${{ vars.CODEBUILD_PROJECT_NAME != '' || github.repository_owner == 'amazon-ion' }}
codebuild-project-name: ${{ vars.CODEBUILD_PROJECT_NAME != '' && vars.CODEBUILD_PROJECT_NAME || 'ion-java' }}
al2-arm: "codebuild-${{ vars.CODEBUILD_PROJECT_NAME != '' && vars.CODEBUILD_PROJECT_NAME || 'ion-java' }}-${{ github.run_id }}-${{ github.run_attempt }}-arm-3.0-large"
outputs:
runner-type: ${{ env.use-codebuild && env.al2_arm || 'ubuntu-latest' }}
steps:
- name: Dump Config
run: echo '${{ toJSON(env) }}'

detect-regression:
name: Detect Regression
runs-on: ubuntu-latest
needs: generate-test-data
runs-on: ${{ needs.select-runner-type.outputs.runner-type }}
needs: [ generate-test-data, select-runner-type ]
strategy:
matrix:
test-data: ['nestedStruct', 'nestedList', 'sexp', 'realWorldDataSchema01', 'realWorldDataSchema02', 'realWorldDataSchema03']
usage-patterns: [' read --mode AverageTime --forks 3 --warmups 2 --iterations 10 --api streaming ',' read --mode AverageTime --forks 3 --warmups 2 --iterations 10 --api dom ', ' write --mode AverageTime --forks 3 --warmups 2 --iterations 10 --api streaming --io-type buffer --ion-length-preallocation 1 ', ' write --mode AverageTime --forks 3 --warmups 2 --iterations 10 --api dom --io-type buffer --ion-length-preallocation 1 ']
fail-fast: false

steps:
- name: Set up JDK 11
- name: Set up JDK 17
uses: actions/[email protected]
with:
distribution: 'corretto'
java-version: 11
java-version: 17

- name: Checkout ion-java-benchmark-cli
uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
Expand Down
26 changes: 9 additions & 17 deletions .github/workflows/publish-release-artifacts.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,16 @@ permissions:
contents: read

jobs:
check-tag:
# First, a sanity check to ensure that the library version matches the release version
runs-on: ubuntu-latest
publish-to-github-release:
# only run if `check-tag` completes successfully
needs: check-tag
runs-on: "codebuild-ion-java-${{ github.run_id }}-${{ github.run_attempt }}-arm-3.0-large"
permissions:
contents: write
steps:
- uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v3.6.0
with:
submodules: recursive
- name: Validate project version matches tag
shell: bash
run: |
Expand All @@ -39,21 +44,9 @@ jobs:
echo "Project Version: $PROJECT_VERSION"
echo "Release Tag: $RELEASE_TAG"
[ "$PROJECT_VERSION" = "$RELEASE_TAG" ] || exit 1
publish-to-github-release:
# only run if `check-tag` completes successfully
needs: check-tag
runs-on: ubuntu-latest
permissions:
contents: write
steps:
# TODO: replace with artifact upload/download -- make sure there's no race condition with other builds also
# uploading an artifact.
- uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v3.6.0
with:
submodules: recursive
- uses: gradle/gradle-build-action@8baac4c8ef753599f92eeb509c246d09d6250fa6 # v3.3.0
with:
arguments: build cyclonedxBom
arguments: build cyclonedxBom publishToSonatype closeAndReleaseSonatypeStagingRepository
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes—that's correct.

- name: Upload Jar to GitHub release
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Expand All @@ -64,4 +57,3 @@ jobs:
run: |
gh release upload "v$(<project.version)" "build/libs/ion-java-$(<project.version).jar"
gh release upload "v$(<project.version)" "build/reports/bom.json"
# TODO: Add `publish-to-maven-central` job
34 changes: 27 additions & 7 deletions build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import org.gradle.kotlin.dsl.support.zipTo
import org.jetbrains.kotlin.gradle.dsl.KotlinCompile
import org.jetbrains.kotlin.gradle.dsl.KotlinJvmOptions
import proguard.gradle.ProGuardTask
import java.net.URI
import java.time.Instant
import java.util.Properties

Expand All @@ -23,6 +22,10 @@ plugins {
kotlin("jvm") version "1.9.0"
java
`maven-publish`

// There are newer versions available, but they are not guaranteed to support Java 8.
id("io.github.gradle-nexus.publish-plugin") version "1.3.0"

jacoco
signing
id("com.github.johnrengelman.shadow") version "8.1.1"
Expand Down Expand Up @@ -69,6 +72,7 @@ group = "com.amazon.ion"
version = File(project.rootDir.path + "/project.version").readLines().single()
description = "A Java implementation of the Amazon Ion data notation."

val isCI: Boolean = System.getenv("CI") == "true"
val githubRepositoryUrl = "https://github.com/amazon-ion/ion-java/"
val isReleaseVersion: Boolean = !version.toString().endsWith("SNAPSHOT")
val generatedResourcesDir = "$buildDir/generated/main/resources"
Expand Down Expand Up @@ -115,7 +119,7 @@ val sourceRepoRemoteName: String by lazy {

var name = "$git remote".runCommand().lines().firstOrNull { it.isSourceRepo() }

if (System.getenv("CI") == "true") {
if (isCI) {
// When running on a CI environment e.g. GitHub Actions, we might need to automatically add the remote
if (name == null) {
name = "ci_source_repository"
Expand Down Expand Up @@ -504,12 +508,20 @@ publishing {
}
}
}
repositories.mavenCentral {
credentials {
username = properties["ossrhUsername"].toString()
password = properties["ossrhPassword"].toString()
}

nexusPublishing {
// Documentation for this plugin, see https://github.com/gradle-nexus/publish-plugin/blob/v1.3.0/README.md
this.repositories {
sonatype {
nexusUrl.set(uri("https://aws.oss.sonatype.org/service/local/staging/deploy/maven2/"))
// For CI environments, the username and password should be stored in
// ORG_GRADLE_PROJECT_sonatypeUsername and ORG_GRADLE_PROJECT_sonatypePassword respectively.
if (!isCI) {
username.set(properties["ossrhUsername"].toString())
password.set(properties["ossrhPassword"].toString())
}
}
url = URI.create("https://aws.oss.sonatype.org/service/local/staging/deploy/maven2")
}
}

Expand All @@ -519,5 +531,13 @@ signing {
// if signing.keyId, signing.password, signing.secretKeyRingFile, etc are
// not present in gradle.properties.
isRequired = isReleaseVersion

if (isCI) {
val signingKeyId: String? by project
val signingKey: String? by project
val signingPassword: String? by project
useInMemoryPgpKeys(signingKeyId, signingKey, signingPassword)
}

sign(publishing.publications["IonJava"])
}
17 changes: 2 additions & 15 deletions src/main/java/com/amazon/ion/Span.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,5 @@
/*
* Copyright 2007-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package com.amazon.ion;

import com.amazon.ion.facet.Faceted;
Expand Down
Loading