Skip to content

Commit 8836376

Browse files
authored
Switch mypy prek hooks to be executed on pre-push, not on pre-commit (apache#56829)
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
1 parent 8cd988d commit 8836376

File tree

5 files changed

+47
-63
lines changed

5 files changed

+47
-63
lines changed

.pre-commit-config.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1349,6 +1349,7 @@ repos:
13491349
## ADD MOST PREK HOOK ABOVE THAT LINE
13501350
# The below prek hooks are those requiring CI image to be built
13511351
- id: mypy-dev
1352+
stages: ['pre-push']
13521353
name: Run mypy for dev
13531354
language: python
13541355
entry: ./scripts/ci/prek/mypy.py
@@ -1363,6 +1364,7 @@ repos:
13631364
files: ^.*\.py$
13641365
require_serial: true
13651366
- id: mypy-airflow-core
1367+
stages: ['pre-push']
13661368
name: Run mypy for airflow-core
13671369
language: python
13681370
entry: ./scripts/ci/prek/mypy.py
@@ -1377,6 +1379,7 @@ repos:
13771379
files: ^airflow-core/.*\.py$
13781380
require_serial: true
13791381
- id: mypy-providers
1382+
stages: ['pre-push']
13801383
name: Run mypy for providers
13811384
language: python
13821385
entry: ./scripts/ci/prek/mypy.py
@@ -1391,6 +1394,7 @@ repos:
13911394
files: ^.*\.py$
13921395
require_serial: true
13931396
- id: mypy-task-sdk
1397+
stages: ['pre-push']
13941398
name: Run mypy for task-sdk
13951399
language: python
13961400
entry: ./scripts/ci/prek/mypy.py
@@ -1405,6 +1409,7 @@ repos:
14051409
files: ^.*\.py$
14061410
require_serial: true
14071411
- id: mypy-devel-common
1412+
stages: ['pre-push']
14081413
name: Run mypy for devel-common
14091414
language: python
14101415
entry: ./scripts/ci/prek/mypy.py
@@ -1419,6 +1424,7 @@ repos:
14191424
files: ^.*\.py$
14201425
require_serial: true
14211426
- id: mypy-airflow-ctl
1427+
stages: ['pre-push']
14221428
name: Run mypy for airflow-ctl
14231429
language: python
14241430
entry: ./scripts/ci/prek/mypy.py

contributing-docs/03a_contributors_quick_start_beginners.rst

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ For Breeze (local development):
5050
.. code-block:: bash
5151
5252
uv tool install prek
53-
prek install -f
53+
prek install --force
54+
prek install --force --hook-type pre-push
55+
5456
* 4GB RAM, 40GB disk space, and at least 2 CPU cores
5557

5658
.. note::
@@ -151,7 +153,8 @@ Option B – One-Click GitHub Codespaces
151153
152154
curl -LsSf https://astral.sh/uv/install.sh | sh
153155
uv tool install prek
154-
prek install -f
156+
prek install --force
157+
prek install --force --hook-type pre-push # for running mypy checks when pushing to repo
155158
uv tool install -e ./dev/breeze
156159
uv run setup_vscode.py
157160
breeze start-airflow

contributing-docs/08_static_code_checks.rst

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ The static code checks in Airflow are used to verify that the code meets certain
2222
All the static code checks can be run through prek hooks.
2323

2424
The prek hooks perform all the necessary installation when you run them
25-
for the first time. See the table below to identify which prek checks require the Breeze Docker images.
25+
for the first time.
2626

2727
You can also run the checks via `Breeze <../dev/breeze/doc/README.rst>`_ environment.
2828

@@ -116,7 +116,7 @@ To install the checks also for ``pre-push`` operations, enter:
116116

117117
.. code-block:: bash
118118
119-
prek install -t pre-push
119+
prek install --hook-type pre-push
120120
121121
For details on advanced usage of the install method, use:
122122

@@ -157,7 +157,12 @@ Using prek
157157
----------
158158

159159
After installation, prek hooks are run automatically when you commit the
160-
code. But you can run prek hooks manually as needed.
160+
code or push it to the repository (depending on stages configured for the hooks). Some of the
161+
hooks are configured to run on "manual" stage only and are not run automatically.
162+
163+
By default when you run ``prek``, the ``pre-commit`` stage hooks are run.
164+
165+
But you can run prek hooks manually as needed.
161166

162167
- Run all checks on your staged files by using:
163168

@@ -170,33 +175,36 @@ code. But you can run prek hooks manually as needed.
170175

171176
.. code-block:: bash
172177
173-
prek mypy-airflow-core mypy-dev
178+
prek mypy-airflow-core mypy-dev --hook-stage pre-push
174179
175180
- Run only mypy airflow checks on all "airflow-core" files by using:
176181

177182
.. code-block:: bash
178183
179-
prek mypy-airflow-core --all-files
184+
prek mypy-airflow-core --all-files --hook-stage pre-push
180185
181-
- Run all checks on all files by using:
186+
- Run all pre-commit stage hooks on all files by using:
182187

183188
.. code-block:: bash
184189
185190
prek --all-files
186191
187-
- Run all checks only on files modified in the last locally available commit in your checked out branch:
192+
- Run all pre-commit stage hooks only on files modified in the last locally available
193+
commit in your checked out branch:
188194

189195
.. code-block:: bash
190196
191197
prek --last-commit
192198
193-
- Run all checks only on files modified in your last branch that is targeted to be merged into the main branch:
199+
- Run all pre-commit stage hooks only on files modified in your last branch that is targeted
200+
to be merged into the main branch:
194201

195202
.. code-block:: bash
196203
197204
prek --from-ref main
198205
199-
- Show files modified automatically by prek when prek automatically fix errors
206+
- Show files modified automatically by prek when prek automatically fixes errors (after running all
207+
``pre-commit`` stage hooks on locally modified files):
200208

201209
.. code-block:: bash
202210
@@ -207,7 +215,7 @@ code. But you can run prek hooks manually as needed.
207215

208216
.. code-block:: bash
209217
210-
SKIP=mypy-airflow-core,ruff prek --all-files
218+
SKIP=ruff,rst-backticks prek --all-files
211219
212220
213221
You can always skip running the tests by providing ``--no-verify`` flag to the
@@ -262,9 +270,8 @@ enter the terminal.
262270
Manual prek hooks
263271
-----------------
264272

265-
Most of the checks we run are configured to run automatically when you commit the code. However,
266-
there are some checks that are not run automatically and you need to run them manually. Those
267-
checks are marked with ``manual`` in the ``Description`` column in the table below. You can run
273+
Most of the checks we run are configured to run automatically when you commit the code or push PR. However,
274+
there are some checks that are not run automatically and you need to run them manually. You can run
268275
them manually by running ``prek --hook-stage manual <hook-id>``.
269276

270277
Special pin-versions prek
@@ -288,11 +295,13 @@ manually by running:
288295
Mypy checks
289296
-----------
290297

291-
When we run mypy checks locally when committing a change, one of the ``mypy-*`` checks is run, ``mypy-airflow``,
298+
When we run mypy checks locally when pushing a change to PR, the ``mypy-*`` checks is run, ``mypy-airflow``,
292299
``mypy-dev``, ``mypy-providers``, ``mypy-airflow-ctl``, depending on the files you are changing. The mypy checks
293300
are run by passing those changed files to mypy. This is way faster than running checks for all files (even
294301
if mypy cache is used - especially when you change a file in Airflow core that is imported and used by many
295-
files). However, in some cases, it produces different results than when running checks for the whole set
302+
files). You also need to have ``breeze ci-image build --python 3.10`` built locally to run the mypy checks.
303+
304+
However, in some cases, it produces different results than when running checks for the whole set
296305
of files, because ``mypy`` does not even know that some types are defined in other files and it might not
297306
be able to follow imports properly if they are dynamic. Therefore in CI we run ``mypy`` check for whole
298307
directories (``airflow`` - excluding providers, ``providers``, ``dev`` and ``docs``) to make sure

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

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,23 +1215,6 @@ def docs_list_as_string(self) -> str | None:
12151215
def skip_prek_hooks(self) -> str:
12161216
prek_hooks_to_skip = set()
12171217
prek_hooks_to_skip.add("identity")
1218-
# Skip all mypy "individual" file checks if we are running mypy checks in CI
1219-
# In the CI we always run mypy for the whole "package" rather than for `--all-files` because
1220-
# The prek will semi-randomly skip such list of files into several groups and we want
1221-
# to make sure that such checks are always run in CI for whole "group" of files - i.e.
1222-
# whole package rather than for individual files. That's why we skip those checks in CI
1223-
# and run them via `mypy-all` command instead and dedicated CI job in matrix
1224-
# This will also speed up static-checks job usually as the jobs will be running in parallel
1225-
prek_hooks_to_skip.update(
1226-
{
1227-
"mypy-providers",
1228-
"mypy-airflow-core",
1229-
"mypy-dev",
1230-
"mypy-task-sdk",
1231-
"mypy-airflow-ctl",
1232-
"mypy-devel-common",
1233-
}
1234-
)
12351218
if self._default_branch != "main":
12361219
# Skip those tests on all "release" branches
12371220
prek_hooks_to_skip.update(

dev/breeze/tests/test_selective_checks.py

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -105,56 +105,40 @@
105105
)
106106

107107
ALL_SKIPPED_COMMITS_ON_NO_CI_IMAGE = (
108-
"check-provider-yaml-valid,flynt,identity,lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,"
109-
"mypy-dev,mypy-devel-common,mypy-providers,mypy-task-sdk,"
108+
"check-provider-yaml-valid,flynt,identity,lint-helm-chart,"
110109
"ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
111110
)
112111

113-
ALL_SKIPPED_COMMITS_BY_DEFAULT_ON_ALL_TESTS_NEEDED = (
114-
"identity,mypy-airflow-core,mypy-airflow-ctl,mypy-dev,mypy-devel-common,mypy-providers,mypy-task-sdk"
115-
)
112+
ALL_SKIPPED_COMMITS_BY_DEFAULT_ON_ALL_TESTS_NEEDED = "identity"
116113

117-
ALL_SKIPPED_COMMITS_IF_NO_UI = (
118-
"identity,mypy-airflow-core,mypy-airflow-ctl,mypy-dev,mypy-devel-common,"
119-
"mypy-providers,mypy-task-sdk,ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
120-
)
121-
ALL_SKIPPED_COMMITS_IF_NO_HELM_TESTS = (
122-
"identity,lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,mypy-dev,mypy-devel-common,"
123-
"mypy-providers,mypy-task-sdk"
124-
)
114+
ALL_SKIPPED_COMMITS_IF_NO_UI = "identity,ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
115+
ALL_SKIPPED_COMMITS_IF_NO_HELM_TESTS = "identity,lint-helm-chart"
125116

126117
ALL_SKIPPED_COMMITS_IF_NO_UI_AND_HELM_TESTS = (
127-
"identity,lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,mypy-dev,mypy-devel-common,"
128-
"mypy-providers,mypy-task-sdk,ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
118+
"identity,lint-helm-chart,ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
129119
)
130120

131121
ALL_SKIPPED_COMMITS_IF_NO_PROVIDERS_AND_UI = (
132-
"check-provider-yaml-valid,identity,mypy-airflow-core,mypy-airflow-ctl,"
133-
"mypy-dev,mypy-devel-common,mypy-providers,mypy-task-sdk,"
134-
"ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
122+
"check-provider-yaml-valid,identity,ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
135123
)
136124

137125
ALL_SKIPPED_COMMITS_IF_NO_PROVIDERS = (
138-
"check-provider-yaml-valid,identity,lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,"
139-
"mypy-dev,mypy-devel-common,mypy-providers,mypy-task-sdk,"
126+
"check-provider-yaml-valid,identity,lint-helm-chart,"
140127
"ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
141128
)
142129

143130

144131
ALL_SKIPPED_COMMITS_IF_NO_PROVIDERS_UI_AND_HELM_TESTS = (
145-
"check-provider-yaml-valid,identity,lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,"
146-
"mypy-dev,mypy-devel-common,mypy-providers,mypy-task-sdk,"
132+
"check-provider-yaml-valid,identity,lint-helm-chart,"
147133
"ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
148134
)
149135

150136
ALL_SKIPPED_COMMITS_IF_NO_CODE_PROVIDERS_AND_HELM_TESTS = (
151-
"check-provider-yaml-valid,flynt,identity,lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,"
152-
"mypy-dev,mypy-devel-common,mypy-providers,mypy-task-sdk"
137+
"check-provider-yaml-valid,flynt,identity,lint-helm-chart"
153138
)
154139

155140
ALL_SKIPPED_COMMITS_IF_NOT_IMPORTANT_FILES_CHANGED = (
156-
"check-provider-yaml-valid,flynt,identity,lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,"
157-
"mypy-dev,mypy-devel-common,mypy-providers,mypy-task-sdk,"
141+
"check-provider-yaml-valid,flynt,identity,lint-helm-chart,"
158142
"ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui"
159143
)
160144

@@ -163,8 +147,7 @@
163147
"check-airflow-provider-compatibility,check-airflow-providers-bug-report-template,"
164148
"check-extra-packages-references,check-provider-yaml-valid,"
165149
"compile-fab-assets,generate-openapi-spec-fab,identity,"
166-
"lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,mypy-dev,"
167-
"mypy-devel-common,mypy-providers,mypy-task-sdk,validate-operators-init"
150+
"lint-helm-chart,validate-operators-init"
168151
)
169152

170153

@@ -1112,7 +1095,7 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str):
11121095
"run-unit-tests": "true",
11131096
"run-amazon-tests": "false",
11141097
"docs-build": "true",
1115-
"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",
1098+
"skip-prek-hooks": "check-provider-yaml-valid,flynt,identity,ts-compile-lint-simple-auth-manager-ui,ts-compile-lint-ui",
11161099
"upgrade-to-newer-dependencies": "false",
11171100
"core-test-types-list-as-strings-in-json": None,
11181101
"providers-test-types-list-as-strings-in-json": None,

0 commit comments

Comments
 (0)