-
Notifications
You must be signed in to change notification settings - Fork 4.5k
airbyte-ci format: make most inputs optional in run-airbyte-ci #37683
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,16 +9,16 @@ inputs: | |
required: true | ||
github_token: | ||
description: "GitHub token" | ||
required: true | ||
required: false | ||
dagger_cloud_token: | ||
description: "Dagger Cloud token" | ||
required: true | ||
required: false | ||
docker_hub_username: | ||
description: "Dockerhub username" | ||
required: true | ||
required: false | ||
docker_hub_password: | ||
description: "Dockerhub password" | ||
required: true | ||
required: false | ||
options: | ||
description: "Options for the subcommand" | ||
required: false | ||
|
@@ -93,15 +93,14 @@ runs: | |
- name: Get start timestamp | ||
id: get-start-timestamp | ||
shell: bash | ||
run: echo "name=start-timestamp=$(date +%s)" >> $GITHUB_OUTPUT | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lol, how did this even work then? |
||
|
||
run: echo "start-timestamp=$(date +%s)" >> $GITHUB_OUTPUT | ||
- name: Docker login | ||
id: docker-login | ||
uses: docker/login-action@v3 | ||
if: ${{ inputs.docker_hub_username != '' && inputs.docker_hub_password != '' }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do no perform docker login if no docker creds were passed by the calling workflow |
||
with: | ||
username: ${{ inputs.docker_hub_username }} | ||
password: ${{ inputs.docker_hub_password }} | ||
|
||
- name: Install Airbyte CI | ||
id: install-airbyte-ci | ||
uses: ./.github/actions/install-airbyte-ci | ||
|
@@ -111,19 +110,19 @@ runs: | |
- name: Run airbyte-ci | ||
id: run-airbyte-ci | ||
shell: bash | ||
run: | | ||
airbyte-ci --disable-update-check --disable-dagger-run --is-ci --gha-workflow-run-id=${{ github.run_id }} ${{ inputs.subcommand }} ${{ inputs.options }} | ||
env: | ||
CI: "True" | ||
CI_GIT_USER: ${{ github.repository_owner }} | ||
CI_PIPELINE_START_TIMESTAMP: ${{ steps.get-start-timestamp.outputs.start-timestamp }} | ||
PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }} | ||
# Next environment variables are workflow inputs based and can be set with empty values if the inputs are not required and passed | ||
CI_CONTEXT: "${{ inputs.context }}" | ||
CI_GIT_REPO_URL: ${{ inputs.git_repo_url }} | ||
CI_GIT_BRANCH: ${{ inputs.git_branch || github.head_ref }} | ||
CI_GIT_REPO_URL: ${{ inputs.git_repo_url }} | ||
CI_GIT_REVISION: ${{ inputs.git_revision || github.sha }} | ||
CI_GIT_USER: ${{ github.repository_owner }} | ||
CI_GITHUB_ACCESS_TOKEN: ${{ inputs.github_token }} | ||
CI_JOB_KEY: ${{ inputs.ci_job_key }} | ||
CI_PIPELINE_START_TIMESTAMP: ${{ steps.get-start-timestamp.outputs.start-timestamp }} | ||
CI_REPORT_BUCKET_NAME: ${{ inputs.report_bucket_name }} | ||
CI: "True" | ||
DAGGER_CLOUD_TOKEN: "${{ inputs.dagger_cloud_token }}" | ||
DOCKER_HUB_PASSWORD: ${{ inputs.docker_hub_password }} | ||
DOCKER_HUB_USERNAME: ${{ inputs.docker_hub_username }} | ||
|
@@ -133,47 +132,46 @@ runs: | |
METADATA_SERVICE_BUCKET_NAME: ${{ inputs.metadata_service_bucket_name }} | ||
METADATA_SERVICE_GCS_CREDENTIALS: ${{ inputs.metadata_service_gcs_credentials }} | ||
PRODUCTION: ${{ inputs.production }} | ||
PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }} | ||
PYTHON_REGISTRY_TOKEN: ${{ inputs.python_registry_token }} | ||
PYTHON_REGISTRY_URL: ${{ inputs.python_registry_url }} | ||
PYTHON_REGISTRY_CHECK_URL: ${{ inputs.python_registry_check_url }} | ||
S3_BUILD_CACHE_ACCESS_KEY_ID: ${{ inputs.s3_build_cache_access_key_id }} | ||
S3_BUILD_CACHE_SECRET_KEY: ${{ inputs.s3_build_cache_secret_key }} | ||
SENTRY_DSN: ${{ inputs.sentry_dsn }} | ||
SENTRY_ENVIRONMENT: ${{ steps.determine-install-mode.outputs.install-mode }} | ||
SLACK_WEBHOOK: ${{ inputs.slack_webhook_url }} | ||
SPEC_CACHE_BUCKET_NAME: ${{ inputs.spec_cache_bucket_name }} | ||
SPEC_CACHE_GCS_CREDENTIALS: ${{ inputs.spec_cache_gcs_credentials }} | ||
# give the Dagger Engine more time to push cache data to Dagger Cloud | ||
run: | | ||
airbyte-ci --disable-update-check --disable-dagger-run --is-ci --gha-workflow-run-id=${{ github.run_id }} ${{ inputs.subcommand }} ${{ inputs.options }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TIL there's |
||
- name: Stop Engine | ||
id: stop-engine | ||
if: always() | ||
shell: bash | ||
run: | | ||
mapfile -t containers < <(docker ps --filter name="dagger-engine-*" -q) | ||
if [[ "${#containers[@]}" -gt 0 ]]; then | ||
# give 5mn to the Dagger Engine to push cache data to Dagger Cloud | ||
docker stop -t 300 "${containers[@]}"; | ||
fi | ||
|
||
- name: Collect docker logs on failure | ||
id: collect-docker-logs | ||
- name: Collect dagger engine logs | ||
id: collect-dagger-engine-logs | ||
if: always() | ||
uses: jwalton/gh-docker-logs@v2 | ||
with: | ||
dest: "./docker_logs" | ||
dest: "./dagger_engine_logs" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is essentially just a rename of the output file, right? Still same / similar logs? |
||
images: "registry.dagger.io/engine" | ||
|
||
- name: Tar logs | ||
id: tar-logs | ||
if: always() | ||
shell: bash | ||
run: tar cvzf ./docker_logs.tgz ./docker_logs | ||
run: tar cvzf ./dagger_engine_logs.tgz ./dagger_engine_logs | ||
|
||
- name: Upload logs to GitHub | ||
id: upload-docker-logs | ||
id: upload-dagger-engine-logs | ||
if: always() | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
name: docker_logs.tgz | ||
path: ./docker_logs.tgz | ||
name: dagger_engine_logs.tgz | ||
path: ./dagger_engine_logs.tgz | ||
retention-days: 7 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,43 @@ on: | |
- "master" | ||
|
||
jobs: | ||
format_check: | ||
# IMPORTANT: This name must match the require check name on the branch protection settings | ||
name: "Check for formatting errors" | ||
if: github.event.pull_request.head.repo.fork == true | ||
environment: community-ci-auto | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this make it "gated" by a human button press? Do we care? |
||
runs-on: community-tooling-test-small | ||
timeout-minutes: 30s | ||
env: | ||
MAIN_BRANCH_NAME: "master" | ||
steps: | ||
# This checkouts a fork which can contain untrusted code | ||
# It's deemed safe as the formatter are not executing any checked out code | ||
- name: Checkout fork | ||
uses: actions/checkout@v4 | ||
with: | ||
repository: ${{ github.event.pull_request.head.repo.full_name }} | ||
ref: ${{ github.event.pull_request.head.sha }} | ||
fetch-depth: 1 | ||
|
||
# This will sync the .github folder of the main repo with the fork | ||
# This allows us to use up to date actions from the main repo | ||
- name: Pull .github folder from main repository | ||
id: pull_github_folder | ||
run: | | ||
git remote add main https://github.com/airbytehq/airbyte.git | ||
git fetch main ${MAIN_BRANCH_NAME} | ||
git checkout main/${MAIN_BRANCH_NAME} -- .github | ||
|
||
- name: Run airbyte-ci format check all | ||
# This path refers to the fork .github folder. | ||
# We make sure its content is in sync with the main repo .github folder by pulling it in the previous step | ||
uses: ./.github/actions/run-airbyte-ci | ||
with: | ||
context: "pull_request" | ||
sentry_dsn: ${{ secrets.SENTRY_AIRBYTE_CI_DSN }} | ||
subcommand: "format check all" | ||
is_fork: "true" | ||
connectors_test: | ||
name: Run connectors tests on fork | ||
if: github.event.pull_request.head.repo.fork == true | ||
|
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 moved this here because I figure it was not properly declared in
run-airbyte-ci
. It make more sense to declare this env var here:When we're developing on airbyte-ci we don't want to create noise in sentry so we set the sentry env to dev.
When we're using airbyte-ci binary (not developing on it) we want to set the sentry env to prod.