airbytehq · octavia-approvington · Nov 14, 2023 · Oct 19, 2023 · Oct 19, 2023 · Oct 19, 2023
@@ -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:
@@ -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:
@@ -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.
@@ -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/wretry.action@v1.0.42
-        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: octavia-squidington-iii@users.noreply.github.com
 
-      - 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

@@ -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:

diff --git a/airbyte-cdk/python/airbyte_cdk/utils/analytics_message.py b/airbyte-cdk/python/airbyte_cdk/utils/analytics_message.py
@@ -1,3 +1,5 @@
+# Copyright (c) 2023 Airbyte, Inc., all rights reserved.
+
 import time
 from typing import Any, Optional
 

diff --git a/airbyte-cdk/python/unit_tests/destinations/vector_db_based/config_test.py b/airbyte-cdk/python/unit_tests/destinations/vector_db_based/config_test.py
@@ -1,3 +1,7 @@
+#
+# Copyright (c) 2023 Airbyte, Inc., all rights reserved.
+#
+
 from typing import Union
 
 import dpath.util

@@ -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)
@@ -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"]
@@ -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:
@@ -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.                                                        |

@@ -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",

@@ -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)

@@ -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
-    },
+    }
 }
 
 
@@ -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.
 
@@ -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),

@@ -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."""
 
@@ -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()]
@@ -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