Skip to content
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

Patch: AJ's pre-commit updates #49846

Merged
merged 10 commits into from
Dec 18, 2024
Merged
57 changes: 14 additions & 43 deletions .github/workflows/format_check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,61 +10,32 @@ on:

jobs:
format-check:
# IMPORTANT: This name must match the require check name on the branch protection settings
name: "Check for formatting errors"
# Do not run this job on forks
# Forked PRs are handled by the community_ci.yml workflow
if: github.event.pull_request.head.repo.fork != true
runs-on: tooling-test-small
runs-on: ubuntu-latest
steps:
- name: Checkout Airbyte (with credentials)
uses: actions/checkout@v4
- uses: actions/checkout@v4
- uses: actions/[email protected]
- uses: pre-commit/[email protected]
id: format-check
with:
ref: ${{ github.head_ref }}
token: ${{ secrets.GH_PAT_APPROVINGTON_OCTAVIA }}
fetch-depth: 1
- name: Run airbyte-ci format check [MASTER]
id: airbyte_ci_format_check_all_master
if: github.ref == 'refs/heads/master'
uses: ./.github/actions/run-airbyte-ci
continue-on-error: true
with:
context: "master"
sentry_dsn: ${{ secrets.SENTRY_AIRBYTE_CI_DSN }}
subcommand: "format check all"

- name: Run airbyte-ci format check [PULL REQUEST]
id: airbyte_ci_format_check_all_pr
if: github.event_name == 'pull_request'
uses: ./.github/actions/run-airbyte-ci
continue-on-error: false
with:
context: "pull_request"
sentry_dsn: ${{ secrets.SENTRY_AIRBYTE_CI_DSN }}
subcommand: "format check all"
dagger_cloud_token: ${{ secrets.DAGGER_CLOUD_TOKEN_CACHE_2 }}

- name: Run airbyte-ci format check [WORKFLOW DISPATCH]
id: airbyte_ci_format_check_all_manual
if: github.event_name == 'workflow_dispatch'
uses: ./.github/actions/run-airbyte-ci
continue-on-error: false
with:
context: "manual"
sentry_dsn: ${{ secrets.SENTRY_AIRBYTE_CI_DSN }}
subcommand: "format check all"
dagger_cloud_token: ${{ secrets.DAGGER_CLOUD_TOKEN_CACHE_2 }}
extra_args: --all-files

- name: Match GitHub User to Slack User [MASTER]
if: github.ref == 'refs/heads/master'
if: >
always() && steps.format-check.outcome == 'failure' &&
github.ref == 'refs/heads/master' &&
github.event.pull_request.head.repo.fork == false
id: match-github-to-slack-user
uses: ./.github/actions/match-github-to-slack-user
env:
AIRBYTE_TEAM_BOT_SLACK_TOKEN: ${{ secrets.SLACK_AIRBYTE_TEAM_READ_USERS }}
GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- name: Format Failure on Master Slack Channel [MASTER]
if: steps.airbyte_ci_format_check_all_master.outcome == 'failure' && github.ref == 'refs/heads/master'
if: >
always() && steps.format-check.outcome == 'failure' &&
github.ref == 'refs/heads/master' &&
github.event.pull_request.head.repo.fork == false
uses: abinoda/slack-action@master
env:
SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN_AIRBYTE_TEAM }}
Expand Down
10 changes: 5 additions & 5 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,19 @@ exclude: |
^.*/__init__\.py$|
^.*?/\.venv/.*$|
^.*?/node_modules/.*$|

^.*?/charts/.*$|
^airbyte-integrations/bases/base-normalization/.*$|
^.*?/normalization_test_output/.*$|

^.*?/pnpm-lock\.yaml$|
^.*?/source-amplitude/unit_tests/api_data/zipped\.json$|
Copy link
Contributor

Choose a reason for hiding this comment

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

oooof
thank you


# Generated/test files
^airbyte-ci/connectors/metadata_service/lib/metadata_service/models/generated/.*$|
^.*?/airbyte-ci/connectors/metadata_service/lib/tests/fixtures/.*/invalid/.*$|
^airbyte-ci/connectors/pipelines/tests/test_format/non_formatted_code/.*$|
^.*?/airbyte-ci/connectors/pipelines/tests/test_format/non_formatted_code/.*$|
^.*?/airbyte-integrations/connectors/destination-.*/expected-spec\.json$
)

Expand All @@ -25,8 +26,7 @@ repos:
hooks:
- id: black
language_version: python3.10
verbose: true
args: ["--verbose"]
args: [--config=pyproject.toml]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The --config-pyproject.toml arg might not be needed, but consistency seemed worthwhile.

Also - I'm okay bringing back verbose if we need it, but was very spammy on my machine when --all-files was passed as an arg.

(I also tried sending an explicit config path to isort - but this had an adverse affect so I reverted it.)


- repo: https://github.com/pycqa/isort
rev: 5.12.0
Expand Down
3 changes: 3 additions & 0 deletions spotless-maven-pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
<includes>
<include>**/*.java</include>
</includes>
<excludes>
<exclude>**/non_formatted_code/*</exclude>
</excludes>
Comment on lines +29 to +31
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was necessary because this file would otherwise be formatted by spotless when running pre-commit with the --all-files flag.

Since spotless doesn't receive individual files from pre-commit, the pre-commit exclude list doesn't apply.

<importOrder />
<eclipse>
<version>4.21</version>
Expand Down
Loading