Skip to content

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions .github/actions/install-airbyte-ci/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ runs:
if [[ "${{ github.ref }}" != "refs/heads/master" ]] && [[ "${{ steps.changes.outputs.pipelines_any_changed }}" == "true" ]]; then
echo "Making changes to Airbyte CI on a non-master branch. Airbyte-CI will be installed from source."
echo "install-mode=source" >> $GITHUB_OUTPUT
echo "SENTRY_ENVIRONMENT=dev" >> $GITHUB_ENV
Copy link
Contributor Author

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.

else
echo "install-mode=binary" >> $GITHUB_OUTPUT
echo "SENTRY_ENVIRONMENT=production" >> $GITHUB_ENV
fi

- name: Install Airbyte CI from binary
Expand Down
46 changes: 22 additions & 24 deletions .github/actions/run-airbyte-ci/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -93,15 +93,14 @@ runs:
- name: Get start timestamp
id: get-start-timestamp
shell: bash
run: echo "name=start-timestamp=$(date +%s)" >> $GITHUB_OUTPUT
Copy link
Contributor Author

Choose a reason for hiding this comment

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

name= was a typo...

Copy link
Contributor

Choose a reason for hiding this comment

The 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 != '' }}
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand All @@ -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 }}
Expand All @@ -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 }}
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL there's disable-dagger-run!

- 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"
Copy link
Contributor

Choose a reason for hiding this comment

The 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
37 changes: 37 additions & 0 deletions .github/workflows/community_ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/connectors_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ jobs:
# We only run the Connectors CI job if there are changes to the connectors on a non-forked PR
# Forked PRs are handled by the community_ci.yml workflow
# If the condition is not met the job will be skipped (it will not fail)
if: needs.changes.outputs.connectors == 'true' && github.event.pull_request.head.repo.fork != true
if: (github.event_name == 'pull_request' && needs.changes.outputs.connectors == 'true' && github.event.pull_request.head.repo.fork != true) || github.event_name == 'workflow_dispatch'
name: Connectors CI
runs-on: connector-test-large
timeout-minutes: 360 # 6 hours
Expand Down
49 changes: 1 addition & 48 deletions .github/workflows/format_check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,7 @@ jobs:
continue-on-error: true
with:
context: "master"
dagger_cloud_token: ${{ secrets.DAGGER_CLOUD_TOKEN_2 }}
docker_hub_password: ${{ secrets.DOCKER_HUB_PASSWORD }}
docker_hub_username: ${{ secrets.DOCKER_HUB_USERNAME }}
gcs_credentials: ${{ secrets.METADATA_SERVICE_PROD_GCS_CREDENTIALS }}
sentry_dsn: ${{ secrets.SENTRY_AIRBYTE_CI_DSN }}
github_token: ${{ secrets.GH_PAT_MAINTENANCE_OCTAVIA }}
subcommand: "format check all"

- name: Run airbyte-ci format check [PULL REQUEST]
Expand All @@ -45,12 +40,7 @@ jobs:
continue-on-error: false
with:
context: "pull_request"
dagger_cloud_token: ${{ secrets.DAGGER_CLOUD_TOKEN_2 }}
docker_hub_password: ${{ secrets.DOCKER_HUB_PASSWORD }}
docker_hub_username: ${{ secrets.DOCKER_HUB_USERNAME }}
gcs_credentials: ${{ secrets.METADATA_SERVICE_PROD_GCS_CREDENTIALS }}
sentry_dsn: ${{ secrets.SENTRY_AIRBYTE_CI_DSN }}
github_token: ${{ secrets.GH_PAT_MAINTENANCE_OCTAVIA }}
subcommand: "format check all"

- name: Run airbyte-ci format check [WORKFLOW DISPATCH]
Expand All @@ -60,12 +50,7 @@ jobs:
continue-on-error: false
with:
context: "manual"
dagger_cloud_token: ${{ secrets.DAGGER_CLOUD_TOKEN_2 }}
docker_hub_password: ${{ secrets.DOCKER_HUB_PASSWORD }}
docker_hub_username: ${{ secrets.DOCKER_HUB_USERNAME }}
gcs_credentials: ${{ secrets.METADATA_SERVICE_PROD_GCS_CREDENTIALS }}
sentry_dsn: ${{ secrets.SENTRY_AIRBYTE_CI_DSN }}
github_token: ${{ secrets.GH_PAT_MAINTENANCE_OCTAVIA }}
subcommand: "format check all"

- name: Match GitHub User to Slack User [MASTER]
Expand All @@ -77,7 +62,7 @@ jobs:
GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- name: Format Failure on Master Slack Channel [MASTER]
if: steps.airbyte_ci_format_check_all.outcome == 'failure' && github.ref == 'refs/heads/master'
if: steps.airbyte_ci_format_check_all_master.outcome == 'failure' && github.ref == 'refs/heads/master'
uses: abinoda/slack-action@master
env:
SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN_AIRBYTE_TEAM }}
Expand All @@ -90,35 +75,3 @@ jobs:
{\"type\":\"section\",\"text\":{\"type\":\"mrkdwn\",\"text\":\"<@${{ steps.match-github-to-slack-user.outputs.slack_user_ids }}> \n\"}},
{\"type\":\"section\",\"text\":{\"type\":\"mrkdwn\",\"text\":\" :octavia-shocked: <https://github.com/${{github.repository}}/actions/runs/${{github.run_id}}|View Action Run> :octavia-shocked: \n\"}},
{\"type\":\"divider\"}]}

# TODO alafanechere: move it to community_ci.yml and make it use the run-airbyte-ci action
format-check-from-forks:
# Same-named job as above, in order to ensure 'required checks' pass either way.
# This should run all the same checks as above, except not requiring any credentials.
name: "Check for formatting errors"
if: >
github.event.pull_request.head.repo.fork == true
runs-on: ubuntu-latest
steps:
# We have no creds. Ignore docker caching and just run the CLI.
- name: Checkout code (Unprivileged)
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.ref }}
repository: ${{ github.event.pull_request.head.repo.full_name }}
fetch-depth: 1
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: "3.10"

- name: Install Airbyte CI from binary
id: install-airbyte-ci-binary
shell: bash
run: |
curl -sSL "https://connectors.airbyte.com/airbyte-ci/releases/ubuntu/latest/airbyte-ci" --output airbyte-ci-bin
sudo mv airbyte-ci-bin /usr/local/bin/airbyte-ci
sudo chmod +x /usr/local/bin/airbyte-ci
- name: Run format checks
run: |
/usr/local/bin/airbyte-ci --disable-update-check format check all
Loading