-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
[native] Convert CircleCI jobs for PrestoC++ to Github actions #24236
Merged
tdcmeehan
merged 1 commit into
prestodb:master
from
czentgr:cz_test_standard_runner_build
Dec 14, 2024
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| name: prestocpp-format-and-header-check | ||
|
|
||
| on: | ||
| workflow_dispatch: | ||
| pull_request: | ||
| paths: | ||
| - 'presto-native-execution/**' | ||
| - '.github/workflows/prestocpp-format-and-header-check.yml' | ||
|
|
||
| jobs: | ||
| prestocpp-format-and-header-check: | ||
| runs-on: ubuntu-latest | ||
| container: | ||
| image: public.ecr.aws/oss-presto/velox-dev:check | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Fix git permissions | ||
| # Usually actions/checkout does this but as we run in a container | ||
| # it doesn't work | ||
| run: git config --global --add safe.directory ${GITHUB_WORKSPACE} | ||
|
|
||
| - name: Check formatting | ||
| run: | | ||
| git fetch origin master | ||
| cd presto-native-execution | ||
| make format-check | ||
|
|
||
| - name: Check license headers | ||
| run: | | ||
| git fetch origin master | ||
| cd presto-native-execution | ||
| make header-check |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| name: prestocpp-linux-adapters-build | ||
|
|
||
| on: | ||
| workflow_dispatch: | ||
| # Disable the automatic execution on PR because it currently will run out of disk space and | ||
| # we will address this subsequently. | ||
| # - use smaller image - in the works | ||
| # - remove the adapters downloaded files after install - JWT needs fixing because of the cmake files end up | ||
| #pull_request: | ||
| # paths: | ||
| # - 'presto-native-execution/scripts/**' | ||
| # - '.github/workflows/prestocpp-linux-adapters-build.yml' | ||
|
|
||
| jobs: | ||
| prestocpp-linux-adapters-build: | ||
| runs-on: ubuntu-22.04 | ||
| container: | ||
| image: prestodb/presto-native-dependency:0.290-20241014120930-e1fc090 | ||
| env: | ||
| CCACHE_DIR: "${{ github.workspace }}/ccache" | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Fix git permissions | ||
| # Usually actions/checkout does this but as we run in a container | ||
| # it doesn't work | ||
| run: git config --global --add safe.directory ${GITHUB_WORKSPACE} | ||
|
|
||
| - name: Update submodules | ||
| run: | | ||
| cd presto-native-execution | ||
| make submodules | ||
|
|
||
| - name: Build all adapter dependencies | ||
| run: | | ||
| mkdir -p ${GITHUB_WORKSPACE}/adapter-deps/install | ||
| mkdir -p ${GITHUB_WORKSPACE}/adapter-deps/download | ||
| source /opt/rh/gcc-toolset-12/enable | ||
| set -xu | ||
| cd presto-native-execution | ||
| export DEPENDENCY_DIR=${GITHUB_WORKSPACE}/adapter-deps/download | ||
| export INSTALL_PREFIX=${GITHUB_WORKSPACE}/adapter-deps/install | ||
| PROMPT_ALWAYS_RESPOND=n ./velox/scripts/setup-adapters.sh | ||
| PROMPT_ALWAYS_RESPOND=n ./scripts/setup-adapters.sh | ||
|
|
||
| - name: Install Github CLI for using apache/infrastructure-actions/stash | ||
| run: | | ||
| curl -L https://github.com/cli/cli/releases/download/v2.63.2/gh_2.63.2_linux_amd64.rpm > gh_2.63.2_linux_amd64.rpm | ||
| rpm -iv gh_2.63.2_linux_amd64.rpm | ||
|
|
||
| - uses: apache/infrastructure-actions/stash/restore@4ab8682fbd4623d2b4fc1c98db38aba5091924c3 | ||
| with: | ||
| path: '${{ env.CCACHE_DIR }}' | ||
| key: ccache-prestocpp-linux-adapters-build | ||
|
|
||
| - name: Zero ccache statistics | ||
| run: ccache -sz | ||
|
|
||
| - name: Build engine | ||
| run: | | ||
| source /opt/rh/gcc-toolset-12/enable | ||
| cd presto-native-execution | ||
| cmake \ | ||
| -B _build/release \ | ||
| -GNinja \ | ||
| -DTREAT_WARNINGS_AS_ERRORS=1 \ | ||
| -DENABLE_ALL_WARNINGS=1 \ | ||
| -DCMAKE_BUILD_TYPE=Release \ | ||
| -DPRESTO_ENABLE_PARQUET=ON \ | ||
| -DPRESTO_ENABLE_S3=ON \ | ||
| -DPRESTO_ENABLE_GCS=ON \ | ||
| -DPRESTO_ENABLE_ABFS=OFF \ | ||
| -DPRESTO_ENABLE_HDFS=ON \ | ||
| -DPRESTO_ENABLE_REMOTE_FUNCTIONS=ON \ | ||
| -DPRESTO_ENABLE_JWT=ON \ | ||
| -DPRESTO_STATS_REPORTER_TYPE=PROMETHEUS \ | ||
| -DPRESTO_MEMORY_CHECKER_TYPE=LINUX_MEMORY_CHECKER \ | ||
| -DPRESTO_ENABLE_TESTING=OFF \ | ||
| -DCMAKE_PREFIX_PATH=/usr/local \ | ||
| -DThrift_ROOT=/usr/local \ | ||
| -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \ | ||
| -DMAX_LINK_JOBS=4 | ||
| ninja -C _build/release -j 4 | ||
|
|
||
| - name: Ccache after | ||
| run: ccache -s | ||
|
|
||
| - uses: apache/infrastructure-actions/stash/save@4ab8682fbd4623d2b4fc1c98db38aba5091924c3 | ||
| with: | ||
| path: '${{ env.CCACHE_DIR }}' | ||
| key: ccache-prestocpp-linux-adapters-build | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
We can remove this job. I am adding the adapters to the dependency image.
#24251
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 job is here so we can check if changes in scripts trigger a failure with the adapters. The triggers is only changes in the script dir.
I can change it to specifically only trigger on changes to setup-adapter.sh. We don't want to find out issues when building the image after having changed the setup-adapters. I think it is useful to have. It won't run very often.
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.
Actually, lets make it to run when we update velox or setup-adapters.sh.
The job is one of the quicker ones. That way we don't get nasty surprises.
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.
But we don't need to install
/velox/scripts/setup-adapters.shsince that is tested on the Velox CI.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.
We also don't need this on a Velox update then.
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 was thinking if we could have a situation that isn't tested in Velox but might be used in PrestoC++. But I guess we'd need a full adapters build and PrestoC++ build. Which was what one CircleCI job originally did.