This is an automated email from the ASF dual-hosted git repository. potiuk pushed a commit to branch v2-8-test in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 0538ef3edb0a6db85c8d308f52ef11b22f44f882 Author: Jarek Potiuk <[email protected]> AuthorDate: Sat Jan 13 17:44:28 2024 +0100 Simplify how mypy "folder" checks are run (#36760) The #36638 change introduced "full package" checks - where in case of CI we run mypy checks separately from regular static checks, for the whole folders. However it's been a little convoluted on how the checks were run, with a separate env variable. Instead we can actually have multiple mypy-* checks (same as we have for local pre-commit runs) as mypy allows to have multiple checks with the same name in various stages. This change simplifies the setup a bit: * we name the checks "folder" checks because this is what they are * we name the check names consistent ("airflow", "providers", "docs", "dev") with mypy-folders output * we have separate small script to run the folder checks * we map "providers" into "airflow/providers" in the pre-commit (cherry picked from commit a912948b51cc50ae6c92496e11abebbba0c647e5) --- .github/workflows/ci.yml | 10 +-- .pre-commit-config.yaml | 43 +++++++++-- STATIC_CODE_CHECKS.rst | 27 ++++--- dev/breeze/src/airflow_breeze/pre_commit_ids.py | 3 +- .../src/airflow_breeze/utils/selective_checks.py | 16 ++-- dev/breeze/tests/test_selective_checks.py | 86 +++++++++++----------- images/breeze/output_static-checks.svg | 4 +- images/breeze/output_static-checks.txt | 2 +- scripts/ci/pre_commit/pre_commit_mypy.py | 12 --- ...re_commit_mypy.py => pre_commit_mypy_folder.py} | 33 +++++---- 10 files changed, 131 insertions(+), 105 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b6cd83c0fb..8a178ef61a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -107,7 +107,7 @@ jobs: ci-image-build: ${{ steps.selective-checks.outputs.ci-image-build }} prod-image-build: ${{ steps.selective-checks.outputs.prod-image-build }} docs-build: ${{ steps.selective-checks.outputs.docs-build }} - mypy-packages: ${{ steps.selective-checks.outputs.mypy-packages }} + mypy-folders: ${{ steps.selective-checks.outputs.mypy-folders }} needs-mypy: ${{ steps.selective-checks.outputs.needs-mypy }} needs-helm-tests: ${{ steps.selective-checks.outputs.needs-helm-tests }} needs-api-tests: ${{ steps.selective-checks.outputs.needs-api-tests }} @@ -564,7 +564,7 @@ jobs: strategy: fail-fast: false matrix: - mypy-package: ${{fromJson(needs.build-info.outputs.mypy-packages)}} + mypy-folder: ${{fromJson(needs.build-info.outputs.mypy-folders)}} env: RUNS_ON: "${{needs.build-info.outputs.runs-on}}" PYTHON_MAJOR_MINOR_VERSION: "${{needs.build-info.outputs.default-python-version}}" @@ -592,17 +592,17 @@ jobs: restore-keys: | pre-commit-${{steps.breeze.outputs.host-python-version}}- if: needs.build-info.outputs.needs-mypy == 'true' - - name: "MyPy checks for ${{ matrix.mypy-package }}" + - name: "MyPy checks for ${{ matrix.mypy-folder }}" run: | pip install pre-commit - pre-commit run --color always --verbose --hook-stage manual mypy --all-files + pre-commit run --color always --verbose --hook-stage manual mypy-${{matrix.mypy-folder}} --all-files env: VERBOSE: "false" COLUMNS: "250" SKIP_GROUP_OUTPUT: "true" DEFAULT_BRANCH: ${{ needs.build-info.outputs.default-branch }} RUFF_FORMAT: "github" - MYPY_PACKAGES: ${{ matrix.mypy-package }} + if: needs.build-info.outputs.needs-mypy == 'true' # Those checks are run if no image needs to be built for checks. This is for simple changes that diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f7b4d0ae6c..69560e43e9 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1091,15 +1091,33 @@ repos: entry: ./scripts/ci/pre_commit/pre_commit_mypy.py files: ^dev/.*\.py$ require_serial: true - additional_dependencies: ['rich>=12.4.4', 'inputimeout', 'pyyaml', 'jsonschema', 'filelock', 'markdown-it-py'] - - id: mypy-core - name: Run mypy for core + additional_dependencies: ['rich>=12.4.4'] + - id: mypy-dev + stages: [ 'manual' ] + name: Run mypy for dev (manual) + language: python + entry: ./scripts/ci/pre_commit/pre_commit_mypy_folder.py dev + pass_filenames: false + files: ^.*\.py$ + require_serial: true + additional_dependencies: [ 'rich>=12.4.4' ] + - id: mypy-airflow + name: Run mypy for airflow language: python entry: ./scripts/ci/pre_commit/pre_commit_mypy.py --namespace-packages files: \.py$ exclude: ^.*/.*_vendor/|^airflow/migrations|^airflow/providers|^dev|^docs|^provider_packages|^tests/providers|^tests/system/providers|^tests/dags/test_imports.py require_serial: true - additional_dependencies: ['rich>=12.4.4', 'inputimeout', 'pyyaml', 'jsonschema', 'filelock', 'markdown-it-py'] + additional_dependencies: ['rich>=12.4.4'] + - id: mypy-airflow + stages: [ 'manual' ] + name: Run mypy for airflow (manual) + language: python + entry: ./scripts/ci/pre_commit/pre_commit_mypy_folder.py airflow + pass_filenames: false + files: ^.*\.py$ + require_serial: true + additional_dependencies: [ 'rich>=12.4.4' ] - id: mypy-providers name: Run mypy for providers language: python @@ -1107,7 +1125,16 @@ repos: files: ^airflow/providers/.*\.py$|^tests/providers/.*\.py$|^tests/system/providers/.*\.py$ exclude: ^.*/.*_vendor/ require_serial: true - additional_dependencies: ['rich>=12.4.4', 'inputimeout', 'pyyaml', 'jsonschema', 'filelock', 'markdown-it-py'] + additional_dependencies: ['rich>=12.4.4'] + - id: mypy-providers + stages: ['manual'] + name: Run mypy for providers (manual) + language: python + entry: ./scripts/ci/pre_commit/pre_commit_mypy_folder.py airflow/providers + pass_filenames: false + files: ^.*\.py$ + require_serial: true + additional_dependencies: ['rich>=12.4.4'] - id: mypy-docs name: Run mypy for /docs/ folder language: python @@ -1116,11 +1143,11 @@ repos: exclude: ^docs/rtd-deprecation require_serial: true additional_dependencies: ['rich>=12.4.4'] - - id: mypy + - id: mypy-docs stages: ['manual'] - name: Run mypy for specified packages (manual) + name: Run mypy for /docs/ folder (manual) language: python - entry: ./scripts/ci/pre_commit/pre_commit_mypy.py + entry: ./scripts/ci/pre_commit/pre_commit_mypy_folder.py docs pass_filenames: false files: ^.*\.py$ require_serial: true diff --git a/STATIC_CODE_CHECKS.rst b/STATIC_CODE_CHECKS.rst index 3f748579b0..b784711825 100644 --- a/STATIC_CODE_CHECKS.rst +++ b/STATIC_CODE_CHECKS.rst @@ -136,21 +136,28 @@ require Breeze Docker image to be built locally. .. note:: Mypy checks - When we run mypy checks locally when committing a change, one of the ``mypy-*`` checks is run, ``mypy-core``, + When we run mypy checks locally when committing a change, one of the ``mypy-*`` checks is run, ``mypy-airflow``, ``mypy-dev``, ``mypy-providers``, ``mypy-docs``, depending on the files you are changing. The mypy checks are run by passing those changed files to mypy. This is way faster than running checks for all files (even if mypy cache is used - especially when you change a file in airflow core that is imported and used by many files). However, in some cases, it produces different results than when running checks for the whole set of files, because ``mypy`` does not even know that some types are defined in other files and it might not be able to follow imports properly if they are dynamic. Therefore in CI we run ``mypy`` check for whole - directories (``airflow`` - excluding providers, ``airflow/providers``, ``dev`` and ``docs``) to make sure + directories (``airflow`` - excluding providers, ``providers``, ``dev`` and ``docs``) to make sure that we catch all ``mypy`` errors - so you can experience different results when running mypy locally and in CI. If you want to run mypy checks for all files locally, you can do it by running the following command (example for ``airflow`` files): .. code-block:: bash - MYPY_PACKAGES="airflow" pre-commit run --hook-stage manual mypy --all-files + pre-commit run --hook-stage manual mypy-<FOLDER> --all-files + + For example: + + .. code-block:: bash + + pre-commit run --hook-stage manual mypy-airflow --all-files + .. note:: Mypy volume cache @@ -342,15 +349,17 @@ require Breeze Docker image to be built locally. +-----------------------------------------------------------+--------------------------------------------------------------+---------+ | mixed-line-ending | Detect if mixed line ending is used (\r vs. \r\n) | | +-----------------------------------------------------------+--------------------------------------------------------------+---------+ -| mypy | Run mypy for specified packages (manual) | * | -+-----------------------------------------------------------+--------------------------------------------------------------+---------+ -| mypy-core | Run mypy for core | * | +| mypy-airflow | * Run mypy for airflow | * | +| | * Run mypy for airflow (manual) | | +-----------------------------------------------------------+--------------------------------------------------------------+---------+ -| mypy-dev | Run mypy for dev | * | +| mypy-dev | * Run mypy for dev | * | +| | * Run mypy for dev (manual) | | +-----------------------------------------------------------+--------------------------------------------------------------+---------+ -| mypy-docs | Run mypy for /docs/ folder | * | +| mypy-docs | * Run mypy for /docs/ folder | * | +| | * Run mypy for /docs/ folder (manual) | | +-----------------------------------------------------------+--------------------------------------------------------------+---------+ -| mypy-providers | Run mypy for providers | * | +| mypy-providers | * Run mypy for providers | * | +| | * Run mypy for providers (manual) | | +-----------------------------------------------------------+--------------------------------------------------------------+---------+ | pretty-format-json | Format JSON files | | +-----------------------------------------------------------+--------------------------------------------------------------+---------+ diff --git a/dev/breeze/src/airflow_breeze/pre_commit_ids.py b/dev/breeze/src/airflow_breeze/pre_commit_ids.py index 8e64e536c7..3c79efeae2 100644 --- a/dev/breeze/src/airflow_breeze/pre_commit_ids.py +++ b/dev/breeze/src/airflow_breeze/pre_commit_ids.py @@ -103,8 +103,7 @@ PRE_COMMIT_LIST = [ "lint-markdown", "lint-openapi", "mixed-line-ending", - "mypy", - "mypy-core", + "mypy-airflow", "mypy-dev", "mypy-docs", "mypy-providers", diff --git a/dev/breeze/src/airflow_breeze/utils/selective_checks.py b/dev/breeze/src/airflow_breeze/utils/selective_checks.py index 3d23647392..62d6f5a7b0 100644 --- a/dev/breeze/src/airflow_breeze/utils/selective_checks.py +++ b/dev/breeze/src/airflow_breeze/utils/selective_checks.py @@ -582,41 +582,41 @@ class SelectiveChecks: return False @cached_property - def mypy_packages(self) -> list[str]: - packages_to_run: list[str] = [] + def mypy_folders(self) -> list[str]: + folders_to_check: list[str] = [] if ( self._matching_files( FileGroupForCi.ALL_AIRFLOW_PYTHON_FILES, CI_FILE_GROUP_MATCHES, CI_FILE_GROUP_EXCLUDES ) or self.full_tests_needed ): - packages_to_run.append("airflow") + folders_to_check.append("airflow") if ( self._matching_files( FileGroupForCi.ALL_PROVIDERS_PYTHON_FILES, CI_FILE_GROUP_MATCHES, CI_FILE_GROUP_EXCLUDES ) or self._are_all_providers_affected() ) and self._default_branch == "main": - packages_to_run.append("airflow/providers") + folders_to_check.append("providers") if ( self._matching_files( FileGroupForCi.ALL_DOCS_PYTHON_FILES, CI_FILE_GROUP_MATCHES, CI_FILE_GROUP_EXCLUDES ) or self.full_tests_needed ): - packages_to_run.append("docs") + folders_to_check.append("docs") if ( self._matching_files( FileGroupForCi.ALL_DEV_PYTHON_FILES, CI_FILE_GROUP_MATCHES, CI_FILE_GROUP_EXCLUDES ) or self.full_tests_needed ): - packages_to_run.append("dev") - return packages_to_run + folders_to_check.append("dev") + return folders_to_check @cached_property def needs_mypy(self) -> bool: - return self.mypy_packages != [] + return self.mypy_folders != [] @cached_property def needs_python_scans(self) -> bool: diff --git a/dev/breeze/tests/test_selective_checks.py b/dev/breeze/tests/test_selective_checks.py index 2db8f657ae..4cfea3740a 100644 --- a/dev/breeze/tests/test_selective_checks.py +++ b/dev/breeze/tests/test_selective_checks.py @@ -112,7 +112,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "upgrade-to-newer-dependencies": "false", "parallel-test-types-list-as-string": None, "needs-mypy": "false", - "mypy-packages": "[]", + "mypy-folders": "[]", }, id="No tests on simple change", ) @@ -137,7 +137,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "upgrade-to-newer-dependencies": "false", "parallel-test-types-list-as-string": "API Always", "needs-mypy": "true", - "mypy-packages": "['airflow']", + "mypy-folders": "['airflow']", }, id="Only API tests and DOCS should run", ) @@ -162,7 +162,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "upgrade-to-newer-dependencies": "false", "parallel-test-types-list-as-string": "Always Operators", "needs-mypy": "true", - "mypy-packages": "['airflow']", + "mypy-folders": "['airflow']", }, id="Only Operator tests and DOCS should run", ) @@ -188,7 +188,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "parallel-test-types-list-as-string": "Always BranchExternalPython BranchPythonVenv " "ExternalPython Operators PythonVenv", "needs-mypy": "true", - "mypy-packages": "['airflow']", + "mypy-folders": "['airflow']", }, id="Only Python tests", ) @@ -213,7 +213,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "upgrade-to-newer-dependencies": "false", "parallel-test-types-list-as-string": "Always Serialization", "needs-mypy": "true", - "mypy-packages": "['airflow']", + "mypy-folders": "['airflow']", }, id="Only Serialization tests", ) @@ -243,7 +243,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "parallel-test-types-list-as-string": "API Always Providers[amazon] " "Providers[common.sql,openlineage,pgvector,postgres] Providers[google]", "needs-mypy": "true", - "mypy-packages": "['airflow', 'airflow/providers']", + "mypy-folders": "['airflow', 'providers']", }, id="API and providers tests and docs should run", ) @@ -269,7 +269,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "upgrade-to-newer-dependencies": "false", "parallel-test-types-list-as-string": "Always Providers[apache.beam] Providers[google]", "needs-mypy": "true", - "mypy-packages": "['airflow/providers']", + "mypy-folders": "['providers']", }, id="Selected Providers and docs should run", ) @@ -295,7 +295,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "upgrade-to-newer-dependencies": "false", "parallel-test-types-list-as-string": None, "needs-mypy": "false", - "mypy-packages": "[]", + "mypy-folders": "[]", }, id="Only docs builds should run - no tests needed", ) @@ -325,7 +325,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "parallel-test-types-list-as-string": "Always Providers[amazon] " "Providers[common.sql,openlineage,pgvector,postgres] Providers[google]", "needs-mypy": "true", - "mypy-packages": "['airflow/providers']", + "mypy-folders": "['providers']", }, id="Helm tests, providers (both upstream and downstream)," "kubernetes tests and docs should run", @@ -357,7 +357,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "parallel-test-types-list-as-string": "Always " "Providers[airbyte,apache.livy,dbt.cloud,dingding,discord,http] Providers[amazon]", "needs-mypy": "true", - "mypy-packages": "['airflow/providers']", + "mypy-folders": "['providers']", }, id="Helm tests, http and all relevant providers, kubernetes tests and " "docs should run even if unimportant files were added", @@ -387,7 +387,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "upgrade-to-newer-dependencies": "false", "parallel-test-types-list-as-string": "Always Providers[airbyte,http]", "needs-mypy": "true", - "mypy-packages": "['airflow/providers']", + "mypy-folders": "['providers']", }, id="Helm tests, airbyte/http providers, kubernetes tests and " "docs should run even if unimportant files were added", @@ -418,7 +418,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "upgrade-to-newer-dependencies": "false", "parallel-test-types-list-as-string": "Always", "needs-mypy": "true", - "mypy-packages": "['airflow']", + "mypy-folders": "['airflow']", }, id="Docs should run even if unimportant files were added and prod image " "should be build for chart changes", @@ -444,7 +444,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "upgrade-to-newer-dependencies": "true", "parallel-test-types-list-as-string": ALL_CI_SELECTIVE_TEST_TYPES, "needs-mypy": "true", - "mypy-packages": "['airflow', 'airflow/providers', 'docs', 'dev']", + "mypy-folders": "['airflow', 'providers', 'docs', 'dev']", }, id="Everything should run - including all providers and upgrading to " "newer requirements as pyproject.toml changed and all Python versions", @@ -470,7 +470,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "upgrade-to-newer-dependencies": "true", "parallel-test-types-list-as-string": ALL_CI_SELECTIVE_TEST_TYPES, "needs-mypy": "true", - "mypy-packages": "['airflow', 'airflow/providers', 'docs', 'dev']", + "mypy-folders": "['airflow', 'providers', 'docs', 'dev']", }, id="Everything should run and upgrading to newer requirements as dependencies change", ) @@ -498,7 +498,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "Providers[apache.hive,cncf.kubernetes,common.sql,exasol,ftp,http," "imap,microsoft.azure,mongo,mysql,openlineage,postgres,salesforce,ssh] Providers[google]", "needs-mypy": "true", - "mypy-packages": "['airflow/providers']", + "mypy-folders": "['providers']", }, id="Providers tests run including amazon tests if amazon provider files changed", ), @@ -521,7 +521,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "upgrade-to-newer-dependencies": "false", "parallel-test-types-list-as-string": "Always Providers[airbyte,http]", "needs-mypy": "true", - "mypy-packages": "['airflow/providers']", + "mypy-folders": "['providers']", }, id="Providers tests run without amazon tests if no amazon file changed", ), @@ -548,7 +548,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "Providers[apache.hive,cncf.kubernetes,common.sql,exasol,ftp,http," "imap,microsoft.azure,mongo,mysql,openlineage,postgres,salesforce,ssh] Providers[google]", "needs-mypy": "true", - "mypy-packages": "['airflow/providers']", + "mypy-folders": "['providers']", }, id="Providers tests run including amazon tests if amazon provider files changed", ), @@ -575,7 +575,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str): "upgrade-to-newer-dependencies": "false", "parallel-test-types-list-as-string": "Always Providers[common.io]", "needs-mypy": "true", - "mypy-packages": "['airflow', 'airflow/providers']", + "mypy-folders": "['airflow', 'providers']", }, id="Only Always and Common.IO tests should run when only common.io and tests/always changed", ), @@ -619,7 +619,7 @@ def test_expected_output_pull_request_main( "upgrade-to-newer-dependencies": "false", "parallel-test-types-list-as-string": ALL_CI_SELECTIVE_TEST_TYPES, "needs-mypy": "true", - "mypy-packages": "['airflow', 'airflow/providers', 'docs', 'dev']", + "mypy-folders": "['airflow', 'providers', 'docs', 'dev']", }, id="Everything should run including all providers when full tests are needed", ) @@ -648,7 +648,7 @@ def test_expected_output_pull_request_main( "upgrade-to-newer-dependencies": "false", "parallel-test-types-list-as-string": ALL_CI_SELECTIVE_TEST_TYPES, "needs-mypy": "true", - "mypy-packages": "['airflow', 'airflow/providers', 'docs', 'dev']", + "mypy-folders": "['airflow', 'providers', 'docs', 'dev']", }, id="Everything should run including full providers when full " "tests are needed even with different label set as well", @@ -675,7 +675,7 @@ def test_expected_output_pull_request_main( "upgrade-to-newer-dependencies": "false", "parallel-test-types-list-as-string": ALL_CI_SELECTIVE_TEST_TYPES, "needs-mypy": "true", - "mypy-packages": "['airflow', 'airflow/providers', 'docs', 'dev']", + "mypy-folders": "['airflow', 'providers', 'docs', 'dev']", }, id="Everything should run including full providers when" "full tests are needed even if no files are changed", @@ -705,7 +705,7 @@ def test_expected_output_pull_request_main( "BranchPythonVenv CLI Core ExternalPython Operators Other PlainAsserts " "PythonVenv Serialization WWW", "needs-mypy": "true", - "mypy-packages": "['airflow', 'docs', 'dev']", + "mypy-folders": "['airflow', 'docs', 'dev']", }, id="Everything should run except Providers and lint pre-commit " "when full tests are needed for non-main branch", @@ -751,7 +751,7 @@ def test_expected_output_full_tests_needed( "skip-provider-tests": "true", "parallel-test-types-list-as-string": None, "needs-mypy": "false", - "mypy-packages": "[]", + "mypy-folders": "[]", }, id="Nothing should run if only non-important files changed", ), @@ -781,7 +781,7 @@ def test_expected_output_full_tests_needed( "skip-provider-tests": "true", "parallel-test-types-list-as-string": "Always", "needs-mypy": "false", - "mypy-packages": "[]", + "mypy-folders": "[]", }, id="No Helm tests, No providers no lint charts, should run if " "only chart/providers changed in non-main but PROD image should be built", @@ -812,7 +812,7 @@ def test_expected_output_full_tests_needed( "skip-provider-tests": "true", "parallel-test-types-list-as-string": "Always CLI", "needs-mypy": "true", - "mypy-packages": "['airflow']", + "mypy-folders": "['airflow']", }, id="Only CLI tests and Kubernetes tests should run if cli/chart files changed in non-main branch", ), @@ -839,7 +839,7 @@ def test_expected_output_full_tests_needed( "parallel-test-types-list-as-string": "API Always BranchExternalPython BranchPythonVenv " "CLI Core ExternalPython Operators Other PlainAsserts PythonVenv Serialization WWW", "needs-mypy": "true", - "mypy-packages": "['airflow']", + "mypy-folders": "['airflow']", }, id="All tests except Providers and helm lint pre-commit " "should run if core file changed in non-main branch", @@ -880,7 +880,7 @@ def test_expected_output_pull_request_v2_7( "skip-provider-tests": "true", "parallel-test-types-list-as-string": None, "needs-mypy": "false", - "mypy-packages": "[]", + "mypy-folders": "[]", }, id="Nothing should run if only non-important files changed", ), @@ -901,7 +901,7 @@ def test_expected_output_pull_request_v2_7( "skip-provider-tests": "true", "parallel-test-types-list-as-string": "Always", "needs-mypy": "true", - "mypy-packages": "['airflow']", + "mypy-folders": "['airflow']", }, id="Only Always and docs build should run if only system tests changed", ), @@ -936,7 +936,7 @@ def test_expected_output_pull_request_v2_7( "microsoft.azure,microsoft.mssql,mysql,openlineage,oracle,postgres,presto,salesforce," "samba,sftp,ssh,trino] Providers[google]", "needs-mypy": "true", - "mypy-packages": "['airflow', 'airflow/providers']", + "mypy-folders": "['airflow', 'providers']", }, id="CLI tests and Google-related provider tests should run if cli/chart files changed but " "prod image should be build too and k8s tests too", @@ -964,7 +964,7 @@ def test_expected_output_pull_request_v2_7( "skip-provider-tests": "true", "parallel-test-types-list-as-string": "API Always CLI Operators WWW", "needs-mypy": "true", - "mypy-packages": "['airflow']", + "mypy-folders": "['airflow']", }, id="No providers tests should run if only CLI/API/Operators/WWW file changed", ), @@ -986,7 +986,7 @@ def test_expected_output_pull_request_v2_7( "skip-provider-tests": "false", "parallel-test-types-list-as-string": ALL_CI_SELECTIVE_TEST_TYPES, "needs-mypy": "true", - "mypy-packages": "['airflow', 'airflow/providers']", + "mypy-folders": "['airflow', 'providers']", }, id="Tests for all providers should run if model file changed", ), @@ -1008,7 +1008,7 @@ def test_expected_output_pull_request_v2_7( "skip-provider-tests": "false", "parallel-test-types-list-as-string": ALL_CI_SELECTIVE_TEST_TYPES, "needs-mypy": "true", - "mypy-packages": "['airflow', 'airflow/providers']", + "mypy-folders": "['airflow', 'providers']", }, id="Tests for all providers should run if any other than API/WWW/CLI/Operators file changed.", ), @@ -1049,7 +1049,7 @@ def test_expected_output_pull_request_target( "upgrade-to-newer-dependencies": "true", "parallel-test-types-list-as-string": ALL_CI_SELECTIVE_TEST_TYPES, "needs-mypy": "true", - "mypy-packages": "['airflow', 'airflow/providers', 'docs', 'dev']", + "mypy-folders": "['airflow', 'providers', 'docs', 'dev']", }, id="All tests run on push even if unimportant file changed", ), @@ -1072,7 +1072,7 @@ def test_expected_output_pull_request_target( "parallel-test-types-list-as-string": "API Always BranchExternalPython BranchPythonVenv " "CLI Core ExternalPython Operators Other PlainAsserts PythonVenv Serialization WWW", "needs-mypy": "true", - "mypy-packages": "['airflow', 'docs', 'dev']", + "mypy-folders": "['airflow', 'docs', 'dev']", }, id="All tests except Providers and Helm run on push" " even if unimportant file changed in non-main branch", @@ -1095,7 +1095,7 @@ def test_expected_output_pull_request_target( "upgrade-to-newer-dependencies": "true", "parallel-test-types-list-as-string": ALL_CI_SELECTIVE_TEST_TYPES, "needs-mypy": "true", - "mypy-packages": "['airflow', 'airflow/providers', 'docs', 'dev']", + "mypy-folders": "['airflow', 'providers', 'docs', 'dev']", }, id="All tests run on push if core file changed", ), @@ -1150,7 +1150,7 @@ def test_no_commit_provided_trigger_full_build_for_any_event_type(github_event): else "false", "parallel-test-types-list-as-string": ALL_CI_SELECTIVE_TEST_TYPES, "needs-mypy": "true", - "mypy-packages": "['airflow', 'airflow/providers', 'docs', 'dev']", + "mypy-folders": "['airflow', 'providers', 'docs', 'dev']", }, str(stderr), ) @@ -1640,7 +1640,7 @@ def test_provider_compatibility_checks(labels: tuple[str, ...], expected_outputs ("README.md",), { "needs-mypy": "false", - "mypy-packages": "[]", + "mypy-folders": "[]", }, "main", (), @@ -1650,7 +1650,7 @@ def test_provider_compatibility_checks(labels: tuple[str, ...], expected_outputs ("airflow/cli/file.py",), { "needs-mypy": "true", - "mypy-packages": "['airflow']", + "mypy-folders": "['airflow']", }, "main", (), @@ -1660,7 +1660,7 @@ def test_provider_compatibility_checks(labels: tuple[str, ...], expected_outputs ("airflow/models/file.py",), { "needs-mypy": "true", - "mypy-packages": "['airflow', 'airflow/providers']", + "mypy-folders": "['airflow', 'providers']", }, "main", (), @@ -1670,7 +1670,7 @@ def test_provider_compatibility_checks(labels: tuple[str, ...], expected_outputs ("airflow/providers/a_file.py",), { "needs-mypy": "true", - "mypy-packages": "['airflow/providers']", + "mypy-folders": "['providers']", }, "main", (), @@ -1680,7 +1680,7 @@ def test_provider_compatibility_checks(labels: tuple[str, ...], expected_outputs ("docs/a_file.py",), { "needs-mypy": "true", - "mypy-packages": "['docs']", + "mypy-folders": "['docs']", }, "main", (), @@ -1690,7 +1690,7 @@ def test_provider_compatibility_checks(labels: tuple[str, ...], expected_outputs ("dev/a_package/a_file.py",), { "needs-mypy": "true", - "mypy-packages": "['airflow', 'airflow/providers', 'docs', 'dev']", + "mypy-folders": "['airflow', 'providers', 'docs', 'dev']", }, "main", (), @@ -1700,7 +1700,7 @@ def test_provider_compatibility_checks(labels: tuple[str, ...], expected_outputs ("readme.md",), { "needs-mypy": "true", - "mypy-packages": "['airflow', 'airflow/providers', 'docs', 'dev']", + "mypy-folders": "['airflow', 'providers', 'docs', 'dev']", }, "main", ("full tests needed",), diff --git a/images/breeze/output_static-checks.svg b/images/breeze/output_static-checks.svg index acbc21e6b4..3979036fae 100644 --- a/images/breeze/output_static-checks.svg +++ b/images/breeze/output_static-checks.svg @@ -339,8 +339,8 @@ </text><text class="breeze-static-checks-r5" x="0" y="898.4" textLength="12.2" clip-path="url(#breeze-static-checks-line-36)">│</text><text class="breeze-static-checks-r7" x="451.4" y="898.4" textLength="988.2" clip-path="url(#breeze-static-checks-line-36)">detect-private-key | doctoc | end-of-file-fixer | fix-encoding-pragma | flynt |  </text><text class="breeze-static-checks-r5" x="1451.8" y="898.4" textLength="12.2" clip-path="url [...] </text><text class="breeze-static-checks-r5" x="0" y="922.8" textLength="12.2" clip-path="url(#breeze-static-checks-line-37)">│</text><text class="breeze-static-checks-r7" x="451.4" y="922.8" textLength="988.2" clip-path="url(#breeze-static-checks-line-37)">generate-airflow-diagrams | generate-pypi-readme | identity | insert-license |   </text><text class="breeze-static-checks-r5" x="1451.8" y="922.8" textLength="12.2" clip-path="url(#bre [...] </text><text class="breeze-static-checks-r5" x="0" y="947.2" textLength="12.2" clip-path="url(#breeze-static-checks-line-38)">│</text><text class="breeze-static-checks-r7" x="451.4" y="947.2" textLength="988.2" clip-path="url(#breeze-static-checks-line-38)">lint-chart-schema | lint-css | lint-dockerfile | lint-helm-chart |               </text><text class="breeze-static-checks-r5 [...] -</text><text class="breeze-static-checks-r5" x="0" y="971.6" textLength="12.2" clip-path="url(#breeze-static-checks-line-39)">│</text><text class="breeze-static-checks-r7" x="451.4" y="971.6" textLength="988.2" clip-path="url(#breeze-static-checks-line-39)">lint-json-schema | lint-markdown | lint-openapi | mixed-line-ending | mypy |     </text><text class="breeze-static-checks-r5" x="1451.8" y="971.6" textLength="12.2" [...] -</text><text class="breeze-static-checks-r5" x="0" y="996" textLength="12.2" clip-path="url(#breeze-static-checks-line-40)">│</text><text class="breeze-static-checks-r7" x="451.4" y="996" textLength="988.2" clip-path="url(#breeze-static-checks-line-40)">mypy-core | mypy-dev | mypy-docs | mypy-providers | pretty-format-json |         </text><text class="breeze-static-checks-r5" x="1451.8" y="996" tex [...] +</text><text class="breeze-static-checks-r5" x="0" y="971.6" textLength="12.2" clip-path="url(#breeze-static-checks-line-39)">│</text><text class="breeze-static-checks-r7" x="451.4" y="971.6" textLength="988.2" clip-path="url(#breeze-static-checks-line-39)">lint-json-schema | lint-markdown | lint-openapi | mixed-line-ending |            </text><text class="breeze-static-checks-r5" x="1451.8" y= [...] +</text><text class="breeze-static-checks-r5" x="0" y="996" textLength="12.2" clip-path="url(#breeze-static-checks-line-40)">│</text><text class="breeze-static-checks-r7" x="451.4" y="996" textLength="988.2" clip-path="url(#breeze-static-checks-line-40)">mypy-airflow | mypy-dev | mypy-docs | mypy-providers | pretty-format-json |      </text><text class="breeze-static-checks-r5" x="1451.8" y="996" textLength="12.2" [...] </text><text class="breeze-static-checks-r5" x="0" y="1020.4" textLength="12.2" clip-path="url(#breeze-static-checks-line-41)">│</text><text class="breeze-static-checks-r7" x="451.4" y="1020.4" textLength="988.2" clip-path="url(#breeze-static-checks-line-41)">python-no-log-warn | replace-bad-characters | rst-backticks | ruff | ruff-format </text><text class="breeze-static-checks-r5" x="1451.8" y="1020.4" textLength="12.2" clip-path="url(#breez [...] </text><text class="breeze-static-checks-r5" x="0" y="1044.8" textLength="12.2" clip-path="url(#breeze-static-checks-line-42)">│</text><text class="breeze-static-checks-r7" x="451.4" y="1044.8" textLength="988.2" clip-path="url(#breeze-static-checks-line-42)">| shellcheck | trailing-whitespace | ts-compile-format-lint-www |                </text><text class="breeze-static-checks- [...] </text><text class="breeze-static-checks-r5" x="0" y="1069.2" textLength="12.2" clip-path="url(#breeze-static-checks-line-43)">│</text><text class="breeze-static-checks-r7" x="451.4" y="1069.2" textLength="988.2" clip-path="url(#breeze-static-checks-line-43)">update-black-version | update-breeze-cmd-output |                             [...] diff --git a/images/breeze/output_static-checks.txt b/images/breeze/output_static-checks.txt index b1b9c6d6be..46441fa038 100644 --- a/images/breeze/output_static-checks.txt +++ b/images/breeze/output_static-checks.txt @@ -1 +1 @@ -d466e07e997920503e147e798ed2b353 +37c100ea5b6e52e061d6fc9651bd0390 diff --git a/scripts/ci/pre_commit/pre_commit_mypy.py b/scripts/ci/pre_commit/pre_commit_mypy.py index 8f38f27ac4..fde1bd8551 100755 --- a/scripts/ci/pre_commit/pre_commit_mypy.py +++ b/scripts/ci/pre_commit/pre_commit_mypy.py @@ -18,7 +18,6 @@ from __future__ import annotations import os -import shlex import sys from pathlib import Path @@ -34,9 +33,6 @@ from common_precommit_utils import ( initialize_breeze_precommit(__name__, __file__) files_to_test = pre_process_files(sys.argv[1:]) -mypy_packages = os.environ.get("MYPY_PACKAGES") -if mypy_packages: - files_to_test += shlex.split(mypy_packages) if files_to_test == ["--namespace-packages"] or files_to_test == []: print("No files to tests. Quitting") sys.exit(0) @@ -57,14 +53,6 @@ res = run_command_via_breeze_shell( ) ci_environment = os.environ.get("CI") if res.returncode != 0: - if mypy_packages and ci_environment: - console.print( - "[yellow]You are running mypy with the packages selected. If you want to" - "reproduce it locally, you need to run the following command:\n" - ) - console.print( - f'MYPY_PACKAGES="{mypy_packages}" pre-commit run --hook-stage manual mypy --all-files\n' - ) upgrading = os.environ.get("UPGRADE_TO_NEWER_DEPENDENCIES", "false") != "false" if upgrading: console.print( diff --git a/scripts/ci/pre_commit/pre_commit_mypy.py b/scripts/ci/pre_commit/pre_commit_mypy_folder.py similarity index 77% copy from scripts/ci/pre_commit/pre_commit_mypy.py copy to scripts/ci/pre_commit/pre_commit_mypy_folder.py index 8f38f27ac4..9a22a07c75 100755 --- a/scripts/ci/pre_commit/pre_commit_mypy.py +++ b/scripts/ci/pre_commit/pre_commit_mypy_folder.py @@ -18,7 +18,6 @@ from __future__ import annotations import os -import shlex import sys from pathlib import Path @@ -27,24 +26,30 @@ sys.path.insert(0, str(Path(__file__).parent.resolve())) from common_precommit_utils import ( console, initialize_breeze_precommit, - pre_process_files, run_command_via_breeze_shell, ) initialize_breeze_precommit(__name__, __file__) -files_to_test = pre_process_files(sys.argv[1:]) -mypy_packages = os.environ.get("MYPY_PACKAGES") -if mypy_packages: - files_to_test += shlex.split(mypy_packages) -if files_to_test == ["--namespace-packages"] or files_to_test == []: - print("No files to tests. Quitting") - sys.exit(0) +ALLOWED_FOLDERS = ["airflow", "airflow/providers", "dev", "docs"] + +if len(sys.argv) < 2: + console.print(f"[yellow]You need to specify the folder to test as parameter: {ALLOWED_FOLDERS}\n") + sys.exit(1) + +mypy_folder = sys.argv[1] +if mypy_folder not in ALLOWED_FOLDERS: + console.print(f"[yellow]Wrong folder {mypy_folder}. It should be one of those: {ALLOWED_FOLDERS}\n") + sys.exit(1) + +arguments = [mypy_folder] +if mypy_folder == "airflow/providers": + arguments.append("--namespace-packages") res = run_command_via_breeze_shell( [ "/opt/airflow/scripts/in_container/run_mypy.sh", - *files_to_test, + *arguments, ], warn_image_upgrade_needed=True, extra_env={ @@ -57,14 +62,12 @@ res = run_command_via_breeze_shell( ) ci_environment = os.environ.get("CI") if res.returncode != 0: - if mypy_packages and ci_environment: + if ci_environment: console.print( - "[yellow]You are running mypy with the packages selected. If you want to" + "[yellow]You are running mypy with the folders selected. If you want to" "reproduce it locally, you need to run the following command:\n" ) - console.print( - f'MYPY_PACKAGES="{mypy_packages}" pre-commit run --hook-stage manual mypy --all-files\n' - ) + console.print("pre-commit run --hook-stage manual mypy-<folder> --all-files\n") upgrading = os.environ.get("UPGRADE_TO_NEWER_DEPENDENCIES", "false") != "false" if upgrading: console.print(
