Add CI infrastructure for trino-redshift#15817
Add CI infrastructure for trino-redshift#15817findinpath wants to merge 1 commit intotrinodb:masterfrom
trino-redshift#15817Conversation
findinpath
force-pushed
the
findinpath/trino-redshift-ci
branch
3 times, most recently
from
e7351ec to
8f0fba3
Compare
findinpath
force-pushed
the
findinpath/trino-redshift-ci
branch
from
8f0fba3 to
e461288
Compare
.github/workflows/ci.yml
Outdated
There was a problem hiding this comment.
There are Trino forks which may be using this Amazon Redshift cluster instances which are not necessarily publicly accessible.
There was a problem hiding this comment.
If we think of a fork, it can do whatever changes it wants, so we don't need hooks in ci.yml for that.
if we want a fork to be able to run the tests without modifying the ci.yml, the REDSHIFT_PUBLICLY_ACCESSIBLE would need to become a repo var (or secret)
bin/redshift/run_mvn_tests_on_aws.sh
Outdated
There was a problem hiding this comment.
instead of this, just invoke ./mvnw with exec
There was a problem hiding this comment.
Would the command
trap - INT TERM EXIT
be still executed afterwards in case of a problem occurring during the tests?
bin/redshift/setup_aws_redshift.sh
Outdated
There was a problem hiding this comment.
it has to be, otherwise CI won't work
.github/workflows/ci.yml
Outdated
There was a problem hiding this comment.
The names TRINO_AWS_ACCESS_KEY_ID and TRINO_AWS_SECRET_ACCESS_KEY are too generic. There's currently no Trino-owned AWS infra and no guarantee that the keys used for redshift are appropriate for other tests, so they should be named specifically for redshift.
There was a problem hiding this comment.
Good point. We'll create a special AWS IAM user for testing trino-redshift
bin/redshift/delete_aws_redshift.sh
Outdated
There was a problem hiding this comment.
no need for pwd, see
trino/plugin/trino-hive-hadoop2/bin/common.sh
Lines 103 to 105 in dc03cff
bin/redshift/delete_aws_redshift.sh
Outdated
There was a problem hiding this comment.
always put variables in " quotes. the code here would break if the path contained a whitespace
also, put variable name in ${...} braces (i am following https://google.github.io/styleguide/shellguide.html#variable-expansion here)
There was a problem hiding this comment.
|| exit 1
this reminds me that all scripts should start with
set -xeuo pipefailthen you don't need exit code checks on individual statements like here
findinpath
force-pushed
the
findinpath/trino-redshift-ci
branch
5 times, most recently
from
f53b2c5 to
657f5e8
Compare
.github/workflows/ci.yml
Outdated
Use an ephemeral/throw-away Redshift cluster for running integration tests on `trino-redshift` connector. Once the tests are run, the testing Redshift cluster is being reclaimed. The Redshift cluster is publicly accessible in order to be accessible from the general purpose Github runners.
findinpath
force-pushed
the
findinpath/trino-redshift-ci
branch
from
657f5e8 to
2b3864d
Compare
|
Superseded by #16142 |
3 participants
Description
This PR adds CI infrastructure for running the integration tests on
trino-redshift.The code spins up on the fly an ephemeral AWS Redshift Cluster before running the tests and deletes it after running the tests.
Additional context and related issues
Release notes
(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: