Skip to content

Commit

Permalink
airbyte-ci: add format commands (airbytehq#31831)
Browse files Browse the repository at this point in the history
Co-authored-by: Ben Church <[email protected]>
Co-authored-by: bnchrch <[email protected]>
Co-authored-by: alafanechere <[email protected]>
Co-authored-by: Augustin <[email protected]>
Co-authored-by: Marius Posta <[email protected]>
Co-authored-by: alafanechere <[email protected]>
  • Loading branch information
7 people authored Nov 14, 2023
1 parent c75325b commit ac3eb28
Show file tree
Hide file tree
Showing 46 changed files with 1,104 additions and 631 deletions.
62 changes: 20 additions & 42 deletions .github/workflows/format.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
name: Format Code (Python + Java)
name: Check for formatting errors

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
# Cancel any previous runs on the same branch if they are still in progress
cancel-in-progress: true

on:
workflow_dispatch:
Expand All @@ -10,8 +12,8 @@ on:
- master
pull_request:
jobs:
format-and-commit:
runs-on: ubuntu-latest
format-fix:
runs-on: "conn-prod-xlarge-runner"
name: "Apply All Formatting Rules"
timeout-minutes: 40
steps:
Expand All @@ -20,16 +22,16 @@ jobs:
with:
ref: ${{ github.head_ref }}
# Important that this is set so that CI checks are triggered again
# Without this we would be be forever waiting on required checks to pass
# Without this we would be forever waiting on required checks to pass
token: ${{ secrets.GH_PAT_APPROVINGTON_OCTAVIA }}

# IMPORTANT! This is nessesary to make sure that a status is reported on the PR
# even if the workflow is skipped. If we used github actions filters, the workflow
# IMPORTANT! This is necessary to make sure that a status is reported on the PR
# even if the workflow is skipped. If we used GitHub Actions filters, the workflow
# would not be reported as skipped, but instead would be forever pending.
#
# I KNOW THIS SOUNDS CRAZY, BUT IT IS TRUE.
#
# Also it gets worse
# Also, it gets worse
#
# IMPORTANT! DO NOT CHANGE THE QUOTES AROUND THE GLOBS. THEY ARE REQUIRED.
# MAKE SURE TO TEST ANY SYNTAX CHANGES BEFORE MERGING.
Expand All @@ -42,59 +44,35 @@ jobs:
- '**/*'
- '!**/*.md'
- uses: actions/setup-java@v3
with:
distribution: "zulu"
java-version: "17"

- uses: actions/setup-python@v4
- name: Run airbyte-ci format fix
uses: ./.github/actions/run-dagger-pipeline
with:
python-version: "3.10"

- name: Set up CI Gradle Properties
run: |
mkdir -p ~/.gradle/
cat > ~/.gradle/gradle.properties <<EOF
org.gradle.jvmargs=-Xmx8g -Xss4m \
--add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
--add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
--add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
--add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
--add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
org.gradle.workers.max=8
org.gradle.vfs.watch=false
EOF
- name: Format
if: steps.changes.outputs.format_any_changed == 'true'
uses: Wandalen/[email protected]
with:
command: ./gradlew format --scan --info --stacktrace
attempt_limit: 3
attempt_delay: 5000 # in ms
context: "pull_request"
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 fix all"

# This is helpful in the case that we change a previously committed generated file to be ignored by git.
- name: Remove any files that have been gitignored
run: git ls-files -i -c --exclude-from=.gitignore | xargs -r git rm --cached

- name: Commit Formatting Changes (PR)
uses: stefanzweifel/git-auto-commit-action@v4
uses: stefanzweifel/git-auto-commit-action@v5
# do not commit if master branch
if: github.ref != 'refs/heads/master'
with:
commit_message: Automated Commit - Formatting Changes
commit_user_name: Octavia Squidington III
commit_user_email: [email protected]

- name: "Fail on Formatting Changes (Master)"
if: github.ref == 'refs/heads/master'
run: git --no-pager diff && test -z "$(git --no-pager diff)"

notify-failure-slack-channel:
name: "Notify Slack Channel on Build Failures"
runs-on: ubuntu-latest
needs:
- format-and-commit
- format-fix
if: ${{ failure() && github.ref == 'refs/heads/master' }}
steps:
- name: Checkout Airbyte
Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/gradle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,7 @@ jobs:
with:
read-only: ${{ github.ref != 'refs/heads/master' }}
# TODO: be able to remove the skipSlowTests property
# TODO: remove the format task ASAP
arguments: --scan --no-daemon --no-watch-fs format check -DskipSlowTests=true
arguments: --scan --no-daemon --no-watch-fs check -DskipSlowTests=true

# In case of self-hosted EC2 errors, remove this block.
stop-check-runner:
Expand Down
2 changes: 2 additions & 0 deletions airbyte-cdk/python/airbyte_cdk/utils/analytics_message.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Copyright (c) 2023 Airbyte, Inc., all rights reserved.

import time
from typing import Any, Optional

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
#
# Copyright (c) 2023 Airbyte, Inc., all rights reserved.
#

from typing import Union

import dpath.util
Expand Down
30 changes: 28 additions & 2 deletions airbyte-ci/connectors/pipelines/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ At this point you can run `airbyte-ci` commands.
- [`connectors bump_version` command](#connectors-bump_version)
- [`connectors upgrade_base_image` command](#connectors-upgrade_base_image)
- [`connectors migrate_to_base_image` command](#connectors-migrate_to_base_image)
- [`format` command subgroup](#format-subgroup)
* [`format check` command](#format-check-command)
* [`format fix` command](#format-fix-command)
- [`metadata` command subgroup](#metadata-command-subgroup)
- [`metadata validate` command](#metadata-validate-command)
* [Example](#example)
Expand Down Expand Up @@ -205,7 +208,6 @@ flowchart TD
entrypoint[[For each selected connector]]
subgraph static ["Static code analysis"]
qa[Run QA checks]
fmt[Run code format checks]
sem["Check version follows semantic versionning"]
incr["Check version is incremented"]
metadata_validation["Run metadata validation on metadata.yaml"]
Expand Down Expand Up @@ -369,6 +371,29 @@ Migrate source-openweather to use the base image: `airbyte-ci connectors --name=
| --------------------- | ----------------------------------------------------------- |
| `PULL_REQUEST_NUMBER` | The GitHub pull request number, used in the changelog entry |

### <a id="format-subgroup"></a>`format` command subgroup

Available commands:
* `airbyte-ci format check all`
* `airbyte-ci format fix all`

### Examples
- Check for formatting errors in the repository: `airbyte-ci format check all`
- Fix formatting for only python files: `airbyte-ci format fix python`

### <a id="format-check-command"></a>`format check all` command

This command runs formatting checks, but does not format the code in place. It will exit 1 as soon as a failure is encountered. To fix errors, use `airbyte-ci format fix all`.

Running `airbyte-ci format check` will run checks on all different types of code. Run `airbyte-ci format check --help` for subcommands to check formatting for only certain types of files.

### <a id="format-fix-command"></a>`format fix all` command

This command runs formatting checks and reformats any code that would be reformatted, so it's recommended to stage changes you might have before running this command.

Running `airbyte-ci format fix all` will format all of the different types of code. Run `airbyte-ci format fix --help` for subcommands to format only certain types of files.


### <a id="metadata-validate-command-subgroup"></a>`metadata` command subgroup

Available commands:
Expand Down Expand Up @@ -408,7 +433,8 @@ This command runs the Python tests for a airbyte-ci poetry package.
## Changelog
| Version | PR | Description |
| ------- | ---------------------------------------------------------- | --------------------------------------------------------------------------------------------------------- |
| 2.5.9 | [#32427](https://github.com/airbytehq/airbyte/pull/32427) | Re-enable caching for source-postgres |
| 2.6.0 | [#31831](https://github.com/airbytehq/airbyte/pull/31831) | Add `airbyte-ci format` commands, remove connector-specific formatting check |
| 2.5.9 | [#32427](https://github.com/airbytehq/airbyte/pull/32427) | Re-enable caching for source-postgres |
| 2.5.8 | [#32402](https://github.com/airbytehq/airbyte/pull/32402) | Set Dagger Cloud token for airbyters only |
| 2.5.7 | [#31628](https://github.com/airbytehq/airbyte/pull/31628) | Add ClickPipelineContext class |
| 2.5.6 | [#32139](https://github.com/airbytehq/airbyte/pull/32139) | Test coverage report on Python connector UnitTest. |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def should_use_remote_secrets(use_remote_secrets: Optional[bool]) -> bool:
lazy_subcommands={
"build": "pipelines.airbyte_ci.connectors.build_image.commands.build",
"test": "pipelines.airbyte_ci.connectors.test.commands.test",
"list": "pipelines.airbyte_ci.connectors.list.commands.list",
"list": "pipelines.airbyte_ci.connectors.list.commands.list_connectors",
"publish": "pipelines.airbyte_ci.connectors.publish.commands.publish",
"bump_version": "pipelines.airbyte_ci.connectors.bump_version.commands.bump_version",
"migrate_to_base_image": "pipelines.airbyte_ci.connectors.migrate_to_base_image.commands.migrate_to_base_image",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
from rich.text import Text


@click.command(cls=DaggerPipelineCommand, help="List all selected connectors.")
@click.command(cls=DaggerPipelineCommand, help="List all selected connectors.", name="list")
@click.pass_context
async def list(
async def list_connectors(
ctx: click.Context,
):
selected_connectors = sorted(ctx.obj["selected_connectors_with_modified_files"], key=lambda x: x.technical_name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,7 @@
ConnectorLanguage.PYTHON: python_connectors.run_all_tests,
ConnectorLanguage.LOW_CODE: python_connectors.run_all_tests,
ConnectorLanguage.JAVA: java_connectors.run_all_tests,
},
"run_code_format_checks": {
ConnectorLanguage.PYTHON: python_connectors.run_code_format_checks,
ConnectorLanguage.LOW_CODE: python_connectors.run_code_format_checks,
# ConnectorLanguage.JAVA: java_connectors.run_code_format_checks
},
}
}


Expand Down Expand Up @@ -65,22 +60,6 @@ async def run_qa_checks(context: ConnectorContext) -> List[StepResult]:
return [await QaChecks(context).run()]


async def run_code_format_checks(context: ConnectorContext) -> List[StepResult]:
"""Run the code format checks according to the connector language.
Args:
context (ConnectorContext): The current connector context.
Returns:
List[StepResult]: The results of the code format checks steps.
"""
if _run_code_format_checks := LANGUAGE_MAPPING["run_code_format_checks"].get(context.connector.language):
return await _run_code_format_checks(context)
else:
context.logger.warning(f"No code format checks defined for connector language {context.connector.language}!")
return []


async def run_all_tests(context: ConnectorContext) -> List[StepResult]:
"""Run all the tests steps according to the connector language.
Expand Down Expand Up @@ -111,10 +90,7 @@ async def run_connector_test_pipeline(context: ConnectorContext, semaphore: anyi
async with semaphore:
async with context:
async with asyncer.create_task_group() as task_group:
tasks = [
task_group.soonify(run_all_tests)(context),
task_group.soonify(run_code_format_checks)(context),
]
tasks = [task_group.soonify(run_all_tests)(context)]
if not context.code_tests_only:
tasks += [
task_group.soonify(run_metadata_validation)(context),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,47 +14,11 @@
from pipelines.airbyte_ci.connectors.build_image.steps.python_connectors import BuildConnectorImages
from pipelines.airbyte_ci.connectors.context import ConnectorContext
from pipelines.airbyte_ci.connectors.test.steps.common import AcceptanceTests, CheckBaseImageIsUsed
from pipelines.consts import LOCAL_BUILD_PLATFORM, PYPROJECT_TOML_FILE_PATH
from pipelines.consts import LOCAL_BUILD_PLATFORM
from pipelines.dagger.actions import secrets
from pipelines.models.steps import Step, StepResult, StepStatus


class CodeFormatChecks(Step):
"""A step to run the code format checks on a Python connector using Black, Isort and Flake."""

title = "Code format checks"

RUN_BLACK_CMD = ["python", "-m", "black", f"--config=/{PYPROJECT_TOML_FILE_PATH}", "--check", "."]
RUN_ISORT_CMD = ["python", "-m", "isort", f"--settings-file=/{PYPROJECT_TOML_FILE_PATH}", "--check-only", "--diff", "."]
RUN_FLAKE_CMD = ["python", "-m", "pflake8", f"--config=/{PYPROJECT_TOML_FILE_PATH}", "."]

async def _run(self) -> StepResult:
"""Run a code format check on the container source code.
We call black, isort and flake commands:
- Black formats the code: fails if the code is not formatted.
- Isort checks the import orders: fails if the import are not properly ordered.
- Flake enforces style-guides: fails if the style-guide is not followed.
Args:
context (ConnectorContext): The current test context, providing a connector object, a dagger client and a repository directory.
step (Step): The step in which the code format checks are run. Defaults to Step.CODE_FORMAT_CHECKS
Returns:
StepResult: Failure or success of the code format checks with stdout and stderr.
"""
connector_under_test = pipelines.dagger.actions.python.common.with_python_connector_source(self.context)

formatter = (
connector_under_test.with_exec(["echo", "Running black"])
.with_exec(self.RUN_BLACK_CMD)
.with_exec(["echo", "Running Isort"])
.with_exec(self.RUN_ISORT_CMD)
.with_exec(["echo", "Running Flake"])
.with_exec(self.RUN_FLAKE_CMD)
)
return await self.get_step_result(formatter)


class PytestStep(Step, ABC):
"""An abstract class to run pytest tests and evaluate success or failure according to pytest logs."""

Expand Down Expand Up @@ -261,15 +225,3 @@ async def run_all_tests(context: ConnectorContext) -> List[StepResult]:
]

return step_results + [task.value for task in tasks]


async def run_code_format_checks(context: ConnectorContext) -> List[StepResult]:
"""Run the code format check steps for Python connectors.
Args:
context (ConnectorContext): The current connector context.
Returns:
List[StepResult]: Results of the code format checks.
"""
return [await CodeFormatChecks(context).run()]
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
#
# Copyright (c) 2023 Airbyte, Inc., all rights reserved.
#

from typing import List

import dagger
from pipelines.helpers.utils import sh_dash_c


def run_check(
container: dagger.Container,
check_commands: List[str],
) -> dagger.Container:
"""Checks whether the repository is formatted correctly.
Args:
container: (dagger.Container): The container to run the formatting check in
check_commands (List[str]): The list of commands to run to check the formatting
"""
return container.with_exec(sh_dash_c(check_commands), skip_entrypoint=True)


async def run_format(
container: dagger.Container,
format_commands: List[str],
) -> dagger.Container:
"""Formats the repository.
Args:
container: (dagger.Container): The container to run the formatter in
format_commands (List[str]): The list of commands to run to format the repository
"""
format_container = container.with_exec(sh_dash_c(format_commands), skip_entrypoint=True)
return await format_container.directory("/src").export(".")


def mount_repo_for_formatting(
dagger_client: dagger.Client,
container: dagger.Container,
include: List[str],
) -> dagger.Container:
"""Mounts the relevant parts of the repository: the code to format and the formatting config
Args:
container: (dagger.Container): The container to mount the repository in
include (List[str]): The list of files to include in the container
"""
container = container.with_mounted_directory(
"/src",
dagger_client.host().directory(
".",
include=include,
),
).with_workdir("/src")

return container
Loading

0 comments on commit ac3eb28

Please sign in to comment.