Add better diagnostics capabilities for pre-commits run via CI image (#…

…23980)

The pre-commits that require CI image run docker command under
the hood that is highly optimized for performance (only mounts
files that are necessary to be mounted) - in order to improve
performance on Mac OS and make sure that artifacts are not left
in the source code of Airflow.

However that makes the command slightly more difficult to debug
because they generate dynamically the docker command used,
including the volumens that should be mounted when the docker
command is run.

This PR adds better diagnostics to the pre-commit scripts
allowing VERBOSE="true" and DRY_RUN="true" variables that can
help with diagnosing problems such as running the scripts on
WSL2.

It also fixes a few documentation bugs that have been missed
after changing names of the image-related static checks and
thanks to separating the common code to utility function
it allows to set SKIP_IMAGE_PRE_COMMITS variable to true
which will skip running all pre-commit checks that require
breeze image to be available locally.

.github/workflows/ci.yml

@@ -637,9 +637,9 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
       - name: "Static checks"
         run: breeze static-checks --all-files --show-diff-on-failure --color always
         env:
-          VERBOSE: false
+          VERBOSE: "false"
           SKIP: "identity"
-          COLUMNS: 250
+          COLUMNS: "250"
       - name: "Fix ownership"
         run: breeze fix-ownership
         if: always()
@@ -654,8 +654,6 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
     needs: [build-info]
     env:
       RUNS_ON: ${{ fromJson(needs.build-info.outputs.runsOn) }}
-      SKIP: "build,run-mypy,run-flake8,lint-javascript,update-migration-references,identity"
-      MOUNT_SELECTED_LOCAL_SOURCES: "true"
     if: needs.build-info.outputs.basic-checks-only == 'true'
     steps:
       - name: Cleanup repo
@@ -695,7 +693,10 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
           breeze static-checks --all-files --show-diff-on-failure --color always
           --commit-ref "${{ github.sha }}"
         env:
-          VERBOSE: false
+          VERBOSE: "false"
+          SKIP_IMAGE_PRE_COMMITS: "true"
+          SKIP: "identity"
+          COLUMNS: "250"
       - name: "Fix ownership"
         run: breeze fix-ownership
         if: always()

STATIC_CODE_CHECKS.rst

@@ -111,8 +111,8 @@ For details on advanced usage of the install method, use:
 Available pre-commit checks
 ...........................
 
-This table lists pre-commit hooks used by Airflow. The ``Breeze`` column indicates which hooks
-require Breeze Docker images to be installed locally.
+This table lists pre-commit hooks used by Airflow. The ``Image`` column indicates which hooks
+require Breeze Docker image to be build locally.
 
 .. note:: Disabling particular checks
 
@@ -123,6 +123,10 @@ require Breeze Docker images to be installed locally.
   ``export SKIP=run-flake8,run-mypy``. You can also add this to your ``.bashrc`` or ``.zshrc`` if you
   do not want to set it manually every time you enter the terminal.
 
+  In case you do not have breeze image configured locally, you can also disable all checks that require
+  the image by setting ``SKIP_IMAGE_PRE_COMMITS`` to "true". This will mark the tests as "green" automatically
+  when run locally (note that those checks will anyway run in CI).
+
   .. BEGIN AUTO-GENERATED STATIC CHECK LIST
 
 +--------------------------------------------------------+------------------------------------------------------------------+---------+
@@ -242,7 +246,7 @@ require Breeze Docker images to be installed locally.
 +--------------------------------------------------------+------------------------------------------------------------------+---------+
 | lint-helm-chart                                        | Lint Helm Chart                                                  |         |
 +--------------------------------------------------------+------------------------------------------------------------------+---------+
-| lint-javascript                                        | * ESLint against airflow/ui                                      |         |
+| lint-javascript                                        | * ESLint against airflow/ui                                      | *       |
 |                                                        | * ESLint against current UI JavaScript files                     |         |
 +--------------------------------------------------------+------------------------------------------------------------------+---------+
 | lint-json-schema                                       | * Lint JSON Schema files with JSON Schema                        |         |
@@ -269,9 +273,9 @@ require Breeze Docker images to be installed locally.
 +--------------------------------------------------------+------------------------------------------------------------------+---------+
 | rst-backticks                                          | Check if RST files use double backticks for code                 |         |
 +--------------------------------------------------------+------------------------------------------------------------------+---------+
-| run-flake8                                             | Run flake8                                                       |         |
+| run-flake8                                             | Run flake8                                                       | *       |
 +--------------------------------------------------------+------------------------------------------------------------------+---------+
-| run-mypy                                               | * Run mypy for dev                                               |         |
+| run-mypy                                               | * Run mypy for dev                                               | *       |
 |                                                        | * Run mypy for core                                              |         |
 |                                                        | * Run mypy for providers                                         |         |
 |                                                        | * Run mypy for /docs/ folder                                     |         |
@@ -294,7 +298,7 @@ require Breeze Docker images to be installed locally.
 +--------------------------------------------------------+------------------------------------------------------------------+---------+
 | update-local-yml-file                                  | Update mounts in the local yml file                              |         |
 +--------------------------------------------------------+------------------------------------------------------------------+---------+
-| update-migration-references                            | Update migration ref doc                                         |         |
+| update-migration-references                            | Update migration ref doc                                         | *       |
 +--------------------------------------------------------+------------------------------------------------------------------+---------+
 | update-providers-dependencies                          | Update cross-dependencies for providers packages                 |         |
 +--------------------------------------------------------+------------------------------------------------------------------+---------+
@@ -387,49 +391,63 @@ Run the ``mypy`` check for the currently staged changes:
 
 .. code-block:: bash
 
-     breeze static-check --type run-mypy
+     breeze static-checks --type run-mypy
 
 Run the ``mypy`` check for all files:
 
 .. code-block:: bash
 
-     breeze static-check --type run-mypy --all-files
+     breeze static-checks --type run-mypy --all-files
 
 Run the ``flake8`` check for the ``tests.core.py`` file with verbose output:
 
 .. code-block:: bash
 
-     breeze static-check --type run-flake8 --file tests/core.py --verbose
+     breeze static-checks --type run-flake8 --file tests/core.py --verbose
 
 Run the ``flake8`` check for the ``tests.core`` package with verbose output:
 
 .. code-block:: bash
 
-     breeze static-check --type run-flake8 --file tests/core/* --verbose
+     breeze static-checks --type run-flake8 --file tests/core/* --verbose
 
 Run all tests for the currently staged files:
 
 .. code-block:: bash
 
-     breeze static-check --type all
+     breeze static-checks --type all
 
 Run all tests for all files:
 
 .. code-block:: bash
 
-    breeze static-check --type all --all-files
+    breeze static-checks --type all --all-files
 
 Run all tests for last commit :
 
 .. code-block:: bash
 
-     breeze static-check --type all --last-commit
+     breeze static-checks -type all --last-commit
+
+Debugging pre-commit check scripts requiring image
+--------------------------------------------------
+
+Those commits that use Breeze docker image might sometimes fail, depending on your operating system and
+docker setup, so sometimes it might be required to run debugging with the commands. This is done via
+two environment variables ``VERBOSE`` and ``DRY_RUN``. Setting them to "true" will respectively show the
+commands to run before running them or skip running the commands.
+
+Note that you need to run pre-commit with --verbose command to get the output regardless of the status
+of the static check (normally it will only show output on failure).
+
+Printing the commands while executing:
+
+.. code-block:: bash
 
+     VERBOSE="true" pre-commit run --verbose run-flake8
 
-The ``license`` check is run via a separate script and a separate Docker image containing the
-Apache RAT verification tool that checks for Apache-compatibility of licenses within the codebase.
-It does not take pre-commit parameters as extra arguments.
+Just performing dry run:
 
 .. code-block:: bash
 
-     breeze static-check licenses
+     DRY_RUN="true" pre-commit run --verbose run-flake8

dev/breeze/src/airflow_breeze/commands/developer_commands.py

@@ -438,7 +438,7 @@ def build_docs(
     type=BetterChoice(PRE_COMMIT_LIST),
     multiple=True,
 )
-@click.option('-a', '--all-files', help="Run checks on all files.")
+@click.option('-a', '--all-files', help="Run checks on all files.", is_flag=True)
 @click.option('-f', '--file', help="List of files to run the checks on.", type=click.Path(), multiple=True)
 @click.option(
     '-s', '--show-diff-on-failure', help="Show diff for files modified by the checks.", is_flag=True

dev/breeze/src/airflow_breeze/utils/path_utils.py

@@ -62,13 +62,7 @@ def in_help() -> bool:
 
 
 def skip_upgrade_check():
-    return (
-        in_self_upgrade()
-        or in_autocomplete()
-        or in_help()
-        or hasattr(sys, '_called_from_test')
-        or os.environ.get('SKIP_BREEZE_UPGRADE_CHECK')
-    )
+    return in_self_upgrade() or in_autocomplete() or in_help() or hasattr(sys, '_called_from_test')
 
 
 def get_package_setup_metadata_hash() -> str:

dev/breeze/src/airflow_breeze/utils/run_utils.py

@@ -27,6 +27,7 @@
 from re import match
 from typing import Dict, List, Mapping, Optional, Union
 
+from airflow_breeze.branch_defaults import AIRFLOW_BRANCH
 from airflow_breeze.params._common_build_params import _CommonBuildParams
 from airflow_breeze.utils.ci_group import ci_group
 from airflow_breeze.utils.console import get_console
@@ -375,3 +376,32 @@ def filter_out_none(**kwargs) -> dict:
         if kwargs[key] is None:
             kwargs.pop(key)
     return kwargs
+
+
+def fail_if_image_missing(image: str, verbose: bool, dry_run: bool, instruction: str) -> None:
+    skip_image_pre_commits = os.environ.get('SKIP_IMAGE_PRE_COMMITS', "false")
+    if skip_image_pre_commits[0].lower() == "t":
+        get_console().print(
+            f"[info]Skipping image check as SKIP_IMAGE_PRE_COMMITS is set to {skip_image_pre_commits}[/]"
+        )
+        sys.exit(0)
+    cmd_result = run_command(
+        ["docker", "inspect", image], stdout=subprocess.DEVNULL, check=False, verbose=verbose, dry_run=dry_run
+    )
+    if cmd_result.returncode != 0:
+        print(f'[red]The image {image} is not available.[/]\n')
+        print(f"\n[yellow]Please run at the earliest convenience:[/]\n\n{instruction}\n\n")
+        sys.exit(1)
+
+
+def get_runnable_ci_image(verbose: bool, dry_run: bool) -> str:
+    github_repository = os.environ.get('GITHUB_REPOSITORY', "apache/airflow")
+    python_version = "3.7"
+    airflow_image = f"ghcr.io/{github_repository}/{AIRFLOW_BRANCH}/ci/python{python_version}"
+    fail_if_image_missing(
+        image=airflow_image,
+        verbose=verbose,
+        dry_run=dry_run,
+        instruction=f"breeze build-image --python {python_version}",
+    )
+    return airflow_image

images/breeze/output-commands-hash.txt

@@ -1 +1 @@
-5b79e317c30d2dc10bc28f425557af1b
+969f1b4101773456e832c03b8d4e89ef