Skip to content
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
4f23c7b
Init
Jul 4, 2025
48dff13
Temporaryily remove all other workflows for faster testing
Jul 4, 2025
9bd9207
Try fix
Jul 4, 2025
4997a02
Merge branch 'release-0.293-clp-connector' into xwei/add-release-imag…
anlowee Jul 7, 2025
d74b885
Fix
Jul 7, 2025
4815351
Fix
Jul 7, 2025
e7fe20c
Still build the runtime image when updating the PR, but only publish …
Jul 7, 2025
fc2cc18
Build worker image
Jul 8, 2025
cc2c510
Merge branch 'release-0.293-clp-connector' into xwei/add-release-imag…
anlowee Jul 8, 2025
60dd19d
Restore workflows
Jul 9, 2025
a534bec
Remove the uncessary action
Jul 10, 2025
9e17bc0
Use docker build-and-push action
Jul 14, 2025
aeb4e48
Merge branch 'release-0.293-clp-connector' into xwei/add-release-imag…
anlowee Jul 14, 2025
dbb28fa
Fix
Jul 14, 2025
1e69681
Fix
Jul 14, 2025
4ca014c
Fix
Jul 14, 2025
fec665a
Fix
Jul 14, 2025
20a7054
Merge branch 'release-0.293-clp-connector' into xwei/add-release-imag…
anlowee Jul 15, 2025
283b751
Fix
Jul 15, 2025
64983f0
Merge branch 'xwei/add-release-image-publish-workflow' of github.com:…
Jul 15, 2025
a6ebbaa
Fix
Jul 15, 2025
5ca0420
Apply suggestions from code review
anlowee Jul 18, 2025
fa06d23
Address comments
Jul 18, 2025
d120672
Merge branch 'release-0.293-clp-connector' into xwei/add-release-imag…
anlowee Jul 18, 2025
4c90aad
Merge branch 'release-0.293-clp-connector' into xwei/add-release-imag…
anlowee Jul 18, 2025
3b76c12
Address comments
Jul 21, 2025
b55aeda
Merge branch 'xwei/add-release-image-publish-workflow' of github.com:…
Jul 21, 2025
74787c7
Remove TODO
Jul 21, 2025
313a648
Address comments
Jul 21, 2025
47b9c10
Merge branch 'release-0.293-clp-connector' into xwei/add-release-imag…
anlowee Jul 21, 2025
ceaa809
Minor refactoring.
kirkrodrigues Jul 21, 2025
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
64 changes: 64 additions & 0 deletions .github/workflows/maven-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,69 @@ jobs:
run: |
export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}"
./mvnw install -B -V -T 1C -DskipTests -Dmaven.javadoc.skip=true --no-transfer-progress -P ci -pl '!presto-test-coverage,!:presto-docs'
- name: Upload presto-server tarball
if: matrix.java == '8.0.442'
uses: actions/upload-artifact@v4
with:
name: presto-server
path: presto-server/target/presto-server-0.293.tar.gz
- name: Upload presto-cli executable
if: matrix.java == '8.0.442'
uses: actions/upload-artifact@v4
with:
name: presto-cli
path: presto-cli/target/presto-cli-0.293-executable.jar
- name: Clean Maven Output
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Hard-coding 0.293 couples the workflow to a single release

The tarball/jar paths and the later ./build.sh 0.293 call embed the version string three times.
When upgrading Presto the CI will silently break until every occurrence is updated.

Introduce a single environment variable at the top of the workflow:

env:
  PRESTO_VERSION: "0.293"

and replace the literals with ${{ env.PRESTO_VERSION }} (including the build-script argument).
This keeps bumps to one-line changes.

🤖 Prompt for AI Agents
In .github/workflows/maven-checks.yml around lines 49 to 61, the Presto version
"0.293" is hard-coded multiple times in artifact paths and build script calls,
causing maintenance issues. Define a single environment variable PRESTO_VERSION
at the top of the workflow with the version string, then replace all occurrences
of the literal "0.293" in paths and script arguments with the variable reference
${{ env.PRESTO_VERSION }} to centralize version management and simplify updates.

run: ./mvnw clean -pl '!:presto-server,!:presto-cli,!presto-test-coverage'

presto-coordinator-with-clp-connector-runtime-image:
Copy link
Member

Choose a reason for hiding this comment

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

How about presto-coordinator-image?

name: presto-coordinator-runtime-image
needs: maven-checks
runs-on: ubuntu-22.04
if: ${{ always() && success() }}
Copy link
Member

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?

Copy link
Author

Choose a reason for hiding this comment

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

if the previous step failed then this step should not be executed, because this step needs the artifacts built in the last step

Copy link
Member

Choose a reason for hiding this comment

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

Pretty sure if you remove this line, the jobs will work in the same way. I.e., if B "needs" A, then B won't run if A fails.

  • success() is the default:

    A default status check of success() is applied unless you include one of these functions.

  • always() is the opposite of what you want, where it will ignore whether the first job fails.

steps:
- uses: "actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683"
with:
submodules: "recursive"

- name: Download presto-server
uses: actions/download-artifact@v4
with:
name: presto-server
path: ./docker

- name: Download presto-cli
uses: actions/download-artifact@v4
with:
name: presto-cli
path: ./docker

- name: "Login to image registry"
uses: "docker/login-action@74a5d142397b4f367a81961eba4e8cd7edddf772"
with:
registry: ghcr.io
username: ${{github.actor}}
password: ${{secrets.GITHUB_TOKEN}}

- name: "Update Metadata"
id: "meta"
uses: "docker/metadata-action@902fa8ec7d6ecbf8d84d538b9b233a880e428804"
with:
images: >-
ghcr.io/${{github.repository}}/coordinator-with-clp-connector-runtime
tags: |
type=raw,value=centos9

- name: "Build and push"
uses: "docker/build-push-action@471d1dc4e07e5cdedd4c2171150001c434f0b7a4"
with:
build-args: |-
PRESTO_VERSION=0.293
JMX_PROMETHEUS_JAVA_AGENT_VERSION=0.20.0
context: "./docker/"
file: "./docker/Dockerfile"
push: >-
${{github.event_name != 'pull_request'
&& github.ref == 'refs/heads/release-0.293-clp-connector'}}
tags: "${{steps.meta.outputs.tags}}"
labels: "${{steps.meta.outputs.labels}}"
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
name: prestocpp-worker-with-clp-connector-runtime-image-build
Copy link
Member

@kirkrodrigues kirkrodrigues Jul 18, 2025

Choose a reason for hiding this comment

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

How about prestissimo-worker-images-build?


on:
# TODO: specifiy the branch to the release-0.293 when finalize the PR
pull_request:
push:

Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Stale TODO & typo; tighten branch filters instead of post-build conditions

The comment still says “specifiy” and the TODO is no longer needed—the branch name is already hard-coded further down.
While you are here, move the branch restriction into the on.push.branches filter so the workflow does not run (and waste minutes) on every push.

-# TODO: specifiy the branch to the release-0.293 when finalize the PR
+# Workflow only runs for the release branch
+
+push:
+  branches:
+    - release-0.293-clp-connector

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In .github/workflows/prestocpp-worker-with-clp-connector-runtime-image-build.yml
around lines 4 to 7, remove the outdated TODO comment with the typo "specifiy"
since the branch is already specified later. Then, add the branch restriction
directly under the on.push.branches filter to prevent the workflow from running
on every push, optimizing build time by limiting runs to the intended branch
only.

Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

TODO & broad trigger – restrict the workflow before merge

The comment is still TODO and the push trigger has no branch filter, so every branch push will execute an expensive multi-image build.
Either:

on:
  push:
    branches: [ "release-0.293-clp-connector" ]
  pull_request:
    branches: [ "release-0.293-clp-connector" ]

or drop the TODO entirely.

🤖 Prompt for AI Agents
In .github/workflows/prestocpp-worker-with-clp-connector-runtime-image-build.yml
around lines 3 to 7, the workflow trigger is too broad because the push event
has no branch filter, causing expensive builds on every branch push. To fix
this, restrict the push and pull_request triggers to only the
"release-0.293-clp-connector" branch by adding a branches filter under each
trigger or remove the TODO comment if no restriction is needed.

jobs:
prestocpp-worker-with-clp-connector-runtime-image:
Copy link
Member

Choose a reason for hiding this comment

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

How about prestissimo-worker-images-build?

name: prestocpp-worker-with-clp-connector-runtime-image
runs-on: ubuntu-22.04
steps:
- uses: "actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683"
with:
submodules: "recursive"

- name: "Login to Image Registry"
uses: "docker/login-action@74a5d142397b4f367a81961eba4e8cd7edddf772"
with:
registry: ghcr.io
username: ${{github.actor}}
password: ${{secrets.GITHUB_TOKEN}}

- name: "Update Metadata for Dependency Image"
Copy link
Member

Choose a reason for hiding this comment

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

Can we reorder these steps so the order is:

  • Set up metadata for deps image
  • Build deps image
  • Set up metadata for runtime image
  • Build runtime image

id: "meta-dependency"
Copy link
Member

Choose a reason for hiding this comment

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

How about metadata-deps-image?

uses: "docker/metadata-action@902fa8ec7d6ecbf8d84d538b9b233a880e428804"
with:
images: >-
ghcr.io/${{github.repository}}/prestissimo-with-clp-connector-dependency
tags: |
type=raw,value=ubuntu-22.04
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Image repository is not lower-cased – GHCR rejects mixed-case paths
${{ github.repository }} preserves capital letters in the org/user name. GHCR (and Docker) require all-lowercase repositories, so pushes fail for maintainers with uppercase chars.

-            ghcr.io/${{github.repository}}/prestissimo-with-clp-connector-dependency
+            ghcr.io/${{ github.repository  | toLower }} /prestissimo-with-clp-connector-dependency

(The same change is needed for the runtime image block below.)
GitHub Actions lacks a built-in toLower function; one common workaround is:

env:
  REPO_LOWER: ${{ github.repository }} 
...
          images: ghcr.io/${{ env.REPO_LOWER,, }}/prestissimo-with-clp-connector-dependency

Please adopt a lower-casing strategy before merging.

Also applies to: 48-51

🤖 Prompt for AI Agents
In .github/workflows/prestocpp-worker-with-clp-connector-runtime-image-build.yml
around lines 28 to 31 and also lines 48 to 51, the image repository path uses
`${{ github.repository }}` which can contain uppercase letters, causing GHCR
push failures. Fix this by defining an environment variable that stores the
lowercase version of the repository name using shell parameter expansion (e.g.,
`REPO_LOWER: ${{ github.repository }}`) and then reference this variable with
lowercase conversion syntax `${{ env.REPO_LOWER,, }}` in the image paths to
ensure all repository names are lowercase.

- name: "Build and push"
uses: "docker/build-push-action@471d1dc4e07e5cdedd4c2171150001c434f0b7a4"
with:
context: "./presto-native-execution/"
file: "./presto-native-execution/scripts/dockerfiles/ubuntu-22.04-dependency.dockerfile"
push: >-
${{github.event_name != 'pull_request'
&& github.ref == 'refs/heads/release-0.293-clp-connector'}}
tags: "${{steps.meta-dependency.outputs.tags}}"
labels: "${{steps.meta-dependency.outputs.labels}}"
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Add Docker layer caching to speed up CI

Both images rebuild from scratch on every run. Enable the built-in cache for docker/build-push-action to cut runtime by ~50 %.

       - name: "Build and push"
         uses: "docker/build-push-action@471d1dc4e07e5cdedd4c2171150001c434f0b7a4"
         with:
+          cache-from: type=registry,ref=ghcr.io/${{github.repository}}/prestissimo-with-clp-connector-dependency:cache
+          cache-to:   type=registry,ref=ghcr.io/${{github.repository}}/prestissimo-with-clp-connector-dependency:cache,mode=max
           context: "./presto-native-execution/"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: "Build and push"
uses: "docker/build-push-action@471d1dc4e07e5cdedd4c2171150001c434f0b7a4"
with:
context: "./presto-native-execution/"
file: "./presto-native-execution/scripts/dockerfiles/ubuntu-22.04-dependency.dockerfile"
push: >-
${{github.event_name != 'pull_request'
&& github.ref == 'refs/heads/release-0.293-clp-connector'}}
tags: "${{steps.meta-dependency.outputs.tags}}"
labels: "${{steps.meta-dependency.outputs.labels}}"
- name: "Build and push"
uses: "docker/build-push-action@471d1dc4e07e5cdedd4c2171150001c434f0b7a4"
with:
cache-from: type=registry,ref=ghcr.io/${{github.repository}}/prestissimo-with-clp-connector-dependency:cache
cache-to: type=registry,ref=ghcr.io/${{github.repository}}/prestissimo-with-clp-connector-dependency:cache,mode=max
context: "./presto-native-execution/"
file: "./presto-native-execution/scripts/dockerfiles/ubuntu-22.04-dependency.dockerfile"
push: >-
${{github.event_name != 'pull_request'
&& github.ref == 'refs/heads/release-0.293-clp-connector'}}
tags: "${{steps.meta-dependency.outputs.tags}}"
labels: "${{steps.meta-dependency.outputs.labels}}"
🤖 Prompt for AI Agents
In .github/workflows/prestocpp-worker-with-clp-connector-runtime-image-build.yml
around lines 33 to 42, the Docker build step does not use layer caching, causing
full rebuilds on every run. To fix this, enable the built-in cache for the
docker/build-push-action by adding the 'cache-from' and 'cache-to' options with
appropriate settings to reuse layers between builds and speed up the CI process.


- name: "Update Metadata for Runtime Image"
id: "meta-runtime"
uses: "docker/metadata-action@902fa8ec7d6ecbf8d84d538b9b233a880e428804"
with:
images: >-
ghcr.io/${{github.repository}}/prestissimo-with-clp-connector-runtime
tags: |
type=raw,value=ubuntu-22.04
- name: "Build and push"
uses: "docker/build-push-action@471d1dc4e07e5cdedd4c2171150001c434f0b7a4"
with:
build-args: |-
NUM_THREADS=4
DEPENDENCY_IMAGE=${{ steps.meta-dependency.outputs.tags }}
BASE_IMAGE=ubuntu:22.04
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

DEPENDENCY_IMAGE may contain multiple tags → invalid ARG
steps.meta-dependency.outputs.tags is a newline-separated list (often multiple tags). Passing it verbatim to docker build --build-arg yields unknown flag: –t or pulls the wrong image.

-            DEPENDENCY_IMAGE=${{ steps.meta-dependency.outputs.tags }}
+            # Use the first tag only
+            DEPENDENCY_IMAGE=${{ steps.meta-dependency.outputs.tags%%$'\n'* }}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In .github/workflows/prestocpp-worker-with-clp-connector-runtime-image-build.yml
around lines 56 to 59, the DEPENDENCY_IMAGE build argument is set directly from
steps.meta-dependency.outputs.tags, which can contain multiple newline-separated
tags causing invalid ARG errors. To fix this, modify the workflow to extract and
pass only a single valid tag (e.g., the first tag) from the output instead of
the entire list, ensuring the build-arg receives a proper single image tag
string.

OSNAME=ubuntu
EXTRA_CMAKE_FLAGS=-DPRESTO_ENABLE_TESTING=OFF -DPRESTO_ENABLE_PARQUET=ON -DPRESTO_ENABLE_S3=ON
context: "./presto-native-execution/"
file: "./presto-native-execution/scripts/dockerfiles/prestissimo-runtime.dockerfile"
push: >-
${{github.event_name != 'pull_request'
&& github.ref == 'refs/heads/release-0.293-clp-connector'}}
tags: "${{steps.meta-runtime.outputs.tags}}"
labels: "${{steps.meta-runtime.outputs.labels}}"
Loading