-
Notifications
You must be signed in to change notification settings - Fork 103
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
ci(spanner): use airlock for presubmit jobs #2199
base: main
Are you sure you want to change the base?
ci(spanner): use airlock for presubmit jobs #2199
Conversation
Warning: This pull request is touching the following templated files: |
137e50c
to
08dddf6
Compare
3ef6509
to
ffaf8e6
Compare
ffaf8e6
to
d851939
Compare
# Setup service account credentials. | ||
export GOOGLE_APPLICATION_CREDENTIALS=${KOKORO_GFILE_DIR}/secret_manager/long-door-651-kokoro-system-test-service-account | ||
export GCLOUD_PROJECT=long-door-651 |
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.
Does this change (moving the lines) really matter?
@@ -16,11 +16,9 @@ | |||
|
|||
set -eo pipefail | |||
|
|||
export NPM_CONFIG_PREFIX=${HOME}/.npm-global |
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.
Does this change (moving the lines) really matter?
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. it depends on which SA Airlock uses. I am using Kokoro SA here. Moving line below makes to use Kokoro GCP pool SA.
@@ -164,7 +164,8 @@ if [[ -n "${KOKORO_BUILD_ID:-}" ]]; then | |||
"KOKORO_GITHUB_PULL_REQUEST_COMMIT" | |||
# For flakybot | |||
"KOKORO_GITHUB_COMMIT_URL" | |||
"KOKORO_GITHUB_PULL_REQUEST_URL" | |||
"KOKORO_GITHUB_PULL_REQUEST_URL", | |||
"ENABLE_AIRLOCK" |
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.
USE_AIRLOCK
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.
will change this.
} | ||
|
||
env_vars: { | ||
key: "ENABLE_AIRLOCK", |
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.
Shall we use USE_AIRLOCK as consistent environment variable?
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. I will change this.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕