You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by po...@apache.org on 2025/10/18 21:53:40 UTC
(airflow) branch v3-1-test updated: [v3-1-test] Switch mypy prek hooks to be executed on pre-push, not on pre-commit (#56829) (#56830)
This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch v3-1-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/v3-1-test by this push:
new 30a1b22d7f8 [v3-1-test] Switch mypy prek hooks to be executed on pre-push, not on pre-commit (#56829) (#56830)
30a1b22d7f8 is described below
commit 30a1b22d7f8dfb678397d57bbbc298279b55dc7a
Author: Jarek Potiuk <ja...@potiuk.com>
AuthorDate: Sat Oct 18 23:53:33 2025 +0200
[v3-1-test] Switch mypy prek hooks to be executed on pre-push, not on pre-commit (#56829) (#56830)
The MyPy checks are slow and they require breeze image to be built,
so they are slowing down commit operations when you have prek
installed and your image is not built or your change contains
a number of files and mypy cache is not warmed up.
Moving mypy checks to pre-push makes it easier to accept by
developers to run `prek install` - and they might also opt-in to
use `prek install --hook-type pre-push` if they want to (by default)
run mypy checks when pushing their changes.
Still - even if you install prek with hook-stage `pre-push` you can
skip it by `git push --no-verify` - same as in case of commit.
This should make "prek" experience quite a bit better for casual usage
or when people are not keeping their breeze image updated.
This should also help in case of cherry-picking, when cherry-picking,
and you need to resolve conflicts, git does not have option to run
`cherry-pick --continue --no-verify` and you need to manually
uninstall prek to skip running prek hooks - which often is slow
due to image not being fresh enough and mypy being generally slow.
This might be quite a QOL improvement for many contributors
(cherry picked from commit 8836376a778ac2e9b39a2dcacb6262aab5591529)
---
.pre-commit-config.yaml | 6 ++++
contributing-docs/08_static_code_checks.rst | 39 ++++++++++++--------
.../src/airflow_breeze/utils/selective_checks.py | 17 ---------
dev/breeze/tests/test_selective_checks.py | 41 +++++++---------------
4 files changed, 42 insertions(+), 61 deletions(-)
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 7f234f8cf75..315202cad69 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1343,6 +1343,7 @@ repos:
## ADD MOST PREK HOOK ABOVE THAT LINE
# The below prek hooks are those requiring CI image to be built
- id: mypy-dev
+ stages: ['pre-push']
name: Run mypy for dev
language: python
entry: ./scripts/ci/prek/mypy.py
@@ -1357,6 +1358,7 @@ repos:
files: ^.*\.py$
require_serial: true
- id: mypy-airflow-core
+ stages: ['pre-push']
name: Run mypy for airflow-core
language: python
entry: ./scripts/ci/prek/mypy.py
@@ -1371,6 +1373,7 @@ repos:
files: ^airflow-core/.*\.py$
require_serial: true
- id: mypy-providers
+ stages: ['pre-push']
name: Run mypy for providers
language: python
entry: ./scripts/ci/prek/mypy.py
@@ -1385,6 +1388,7 @@ repos:
files: ^.*\.py$
require_serial: true
- id: mypy-task-sdk
+ stages: ['pre-push']
name: Run mypy for task-sdk
language: python
entry: ./scripts/ci/prek/mypy.py
@@ -1399,6 +1403,7 @@ repos:
files: ^.*\.py$
require_serial: true
- id: mypy-devel-common
+ stages: ['pre-push']
name: Run mypy for devel-common
language: python
entry: ./scripts/ci/prek/mypy.py
@@ -1413,6 +1418,7 @@ repos:
files: ^.*\.py$
require_serial: true
- id: mypy-airflow-ctl
+ stages: ['pre-push']
name: Run mypy for airflow-ctl
language: python
entry: ./scripts/ci/prek/mypy.py
diff --git a/contributing-docs/08_static_code_checks.rst b/contributing-docs/08_static_code_checks.rst
index 297b2492830..3760a864a8d 100644
--- a/contributing-docs/08_static_code_checks.rst
+++ b/contributing-docs/08_static_code_checks.rst
@@ -22,7 +22,7 @@ The static code checks in Airflow are used to verify that the code meets certain
All the static code checks can be run through prek hooks.
The prek hooks perform all the necessary installation when you run them
-for the first time. See the table below to identify which prek checks require the Breeze Docker images.
+for the first time.
You can also run the checks via `Breeze <../dev/breeze/doc/README.rst>`_ environment.
@@ -116,7 +116,7 @@ To install the checks also for ``pre-push`` operations, enter:
.. code-block:: bash
- prek install -t pre-push
+ prek install --hook-type pre-push
For details on advanced usage of the install method, use:
@@ -157,7 +157,12 @@ Using prek
----------
After installation, prek hooks are run automatically when you commit the
-code. But you can run prek hooks manually as needed.
+code or push it to the repository (depending on stages configured for the hooks). Some of the
+hooks are configured to run on "manual" stage only and are not run automatically.
+
+By default when you run ``prek``, the ``pre-commit`` stage hooks are run.
+
+But you can run prek hooks manually as needed.
- Run all checks on your staged files by using:
@@ -170,33 +175,36 @@ code. But you can run prek hooks manually as needed.
.. code-block:: bash
- prek mypy-airflow-core mypy-dev
+ prek mypy-airflow-core mypy-dev --hook-stage pre-push
- Run only mypy airflow checks on all "airflow-core" files by using:
.. code-block:: bash
- prek mypy-airflow-core --all-files
+ prek mypy-airflow-core --all-files --hook-stage pre-push
-- Run all checks on all files by using:
+- Run all pre-commit stage hooks on all files by using:
.. code-block:: bash
prek --all-files
-- Run all checks only on files modified in the last locally available commit in your checked out branch:
+- Run all pre-commit stage hooks only on files modified in the last locally available
+ commit in your checked out branch:
.. code-block:: bash
prek --last-commit
-- Run all checks only on files modified in your last branch that is targeted to be merged into the main branch:
+- Run all pre-commit stage hooks only on files modified in your last branch that is targeted
+ to be merged into the main branch:
.. code-block:: bash
prek --from-ref main
-- Show files modified automatically by prek when prek automatically fix errors
+- Show files modified automatically by prek when prek automatically fixes errors (after running all
+ ``pre-commit`` stage hooks on locally modified files):
.. code-block:: bash
@@ -207,7 +215,7 @@ code. But you can run prek hooks manually as needed.
.. code-block:: bash
- SKIP=mypy-airflow-core,ruff prek --all-files
+ SKIP=ruff,rst-backticks prek --all-files
You can always skip running the tests by providing ``--no-verify`` flag to the
@@ -262,9 +270,8 @@ enter the terminal.
Manual prek hooks
-----------------
-Most of the checks we run are configured to run automatically when you commit the code. However,
-there are some checks that are not run automatically and you need to run them manually. Those
-checks are marked with ``manual`` in the ``Description`` column in the table below. You can run
+Most of the checks we run are configured to run automatically when you commit the code or push PR. However,
+there are some checks that are not run automatically and you need to run them manually. You can run
them manually by running ``prek --hook-stage manual <hook-id>``.
Special pin-versions prek
@@ -288,11 +295,13 @@ manually by running:
Mypy checks
-----------
-When we run mypy checks locally when committing a change, one of the ``mypy-*`` checks is run, ``mypy-airflow``,
+When we run mypy checks locally when pushing a change to PR, the ``mypy-*`` checks is run, ``mypy-airflow``,
``mypy-dev``, ``mypy-providers``, ``mypy-airflow-ctl``, 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
+files). You also need to have ``breeze ci-image build --python 3.10`` built locally to run the mypy checks.
+
+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, ``providers``, ``dev`` and ``docs``) to make sure
diff --git a/dev/breeze/src/airflow_breeze/utils/selective_checks.py b/dev/breeze/src/airflow_breeze/utils/selective_checks.py
index 5b92a89309a..04824055dc8 100644
--- a/dev/breeze/src/airflow_breeze/utils/selective_checks.py
+++ b/dev/breeze/src/airflow_breeze/utils/selective_checks.py
@@ -1215,23 +1215,6 @@ class SelectiveChecks:
def skip_prek_hooks(self) -> str:
prek_hooks_to_skip = set()
prek_hooks_to_skip.add("identity")
- # Skip all mypy "individual" file checks if we are running mypy checks in CI
- # In the CI we always run mypy for the whole "package" rather than for `--all-files` because
- # The prek will semi-randomly skip such list of files into several groups and we want
- # to make sure that such checks are always run in CI for whole "group" of files - i.e.
- # whole package rather than for individual files. That's why we skip those checks in CI
- # and run them via `mypy-all` command instead and dedicated CI job in matrix
- # This will also speed up static-checks job usually as the jobs will be running in parallel
- prek_hooks_to_skip.update(
- {
- "mypy-providers",
- "mypy-airflow-core",
- "mypy-dev",
- "mypy-task-sdk",
- "mypy-airflow-ctl",
- "mypy-devel-common",
- }
- )
if self._default_branch != "main":
# Skip those tests on all "release" branches
prek_hooks_to_skip.update(
diff --git a/dev/breeze/tests/test_selective_checks.py b/dev/breeze/tests/test_selective_checks.py
index 154ad377ab1..98d2dbdc68a 100644
--- a/dev/breeze/tests/test_selective_checks.py
+++ b/dev/breeze/tests/test_selective_checks.py
@@ -104,56 +104,40 @@ ALL_MYPY_CHECKS_EXCEPT_PROVIDERS = str(
)
ALL_SKIPPED_COMMITS_ON_NO_CI_IMAGE = (
- "check-provider-yaml-valid,flynt,identity,lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,"
- "mypy-dev,mypy-devel-common,mypy-providers,mypy-task-sdk,"
+ "check-provider-yaml-valid,flynt,identity,lint-helm-chart,"
"ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
)
-ALL_SKIPPED_COMMITS_BY_DEFAULT_ON_ALL_TESTS_NEEDED = (
- "identity,mypy-airflow-core,mypy-airflow-ctl,mypy-dev,mypy-devel-common,mypy-providers,mypy-task-sdk"
-)
+ALL_SKIPPED_COMMITS_BY_DEFAULT_ON_ALL_TESTS_NEEDED = "identity"
-ALL_SKIPPED_COMMITS_IF_NO_UI = (
- "identity,mypy-airflow-core,mypy-airflow-ctl,mypy-dev,mypy-devel-common,"
- "mypy-providers,mypy-task-sdk,ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
-)
-ALL_SKIPPED_COMMITS_IF_NO_HELM_TESTS = (
- "identity,lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,mypy-dev,mypy-devel-common,"
- "mypy-providers,mypy-task-sdk"
-)
+ALL_SKIPPED_COMMITS_IF_NO_UI = "identity,ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
+ALL_SKIPPED_COMMITS_IF_NO_HELM_TESTS = "identity,lint-helm-chart"
ALL_SKIPPED_COMMITS_IF_NO_UI_AND_HELM_TESTS = (
- "identity,lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,mypy-dev,mypy-devel-common,"
- "mypy-providers,mypy-task-sdk,ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
+ "identity,lint-helm-chart,ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
)
ALL_SKIPPED_COMMITS_IF_NO_PROVIDERS_AND_UI = (
- "check-provider-yaml-valid,identity,mypy-airflow-core,mypy-airflow-ctl,"
- "mypy-dev,mypy-devel-common,mypy-providers,mypy-task-sdk,"
- "ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
+ "check-provider-yaml-valid,identity,ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
)
ALL_SKIPPED_COMMITS_IF_NO_PROVIDERS = (
- "check-provider-yaml-valid,identity,lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,"
- "mypy-dev,mypy-devel-common,mypy-providers,mypy-task-sdk,"
+ "check-provider-yaml-valid,identity,lint-helm-chart,"
"ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
)
ALL_SKIPPED_COMMITS_IF_NO_PROVIDERS_UI_AND_HELM_TESTS = (
- "check-provider-yaml-valid,identity,lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,"
- "mypy-dev,mypy-devel-common,mypy-providers,mypy-task-sdk,"
+ "check-provider-yaml-valid,identity,lint-helm-chart,"
"ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
)
ALL_SKIPPED_COMMITS_IF_NO_CODE_PROVIDERS_AND_HELM_TESTS = (
- "check-provider-yaml-valid,flynt,identity,lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,"
- "mypy-dev,mypy-devel-common,mypy-providers,mypy-task-sdk"
+ "check-provider-yaml-valid,flynt,identity,lint-helm-chart"
)
ALL_SKIPPED_COMMITS_IF_NOT_IMPORTANT_FILES_CHANGED = (
- "check-provider-yaml-valid,flynt,identity,lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,"
- "mypy-dev,mypy-devel-common,mypy-providers,mypy-task-sdk,"
+ "check-provider-yaml-valid,flynt,identity,lint-helm-chart,"
"ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
)
@@ -162,8 +146,7 @@ All_SKIPPED_COMMITS_IF_NON_MAIN_BRANCH = (
"check-airflow-provider-compatibility,check-airflow-providers-bug-report-template,"
"check-extra-packages-references,check-provider-yaml-valid,"
"compile-fab-assets,generate-openapi-spec-fab,identity,"
- "lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,mypy-dev,"
- "mypy-devel-common,mypy-providers,mypy-task-sdk,validate-operators-init"
+ "lint-helm-chart,validate-operators-init"
)
@@ -1108,7 +1091,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str):
"run-unit-tests": "true",
"run-amazon-tests": "false",
"docs-build": "true",
- "skip-prek-hooks": "check-provider-yaml-valid,flynt,identity,mypy-airflow-core,mypy-airflow-ctl,mypy-dev,mypy-devel-common,mypy-providers,mypy-task-sdk,ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui",
+ "skip-prek-hooks": "check-provider-yaml-valid,flynt,identity,ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui",
"upgrade-to-newer-dependencies": "false",
"core-test-types-list-as-strings-in-json": None,
"providers-test-types-list-as-strings-in-json": None,