Skip to content

Commit

Permalink
Create pre-commit script to prevent creating FAB related migrations i…
Browse files Browse the repository at this point in the history
…n core Airflow (#35927)
  • Loading branch information
vincbeck authored Nov 28, 2023
1 parent d37b91c commit 6393b35
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 2 deletions.
32 changes: 32 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,38 @@ repos:
- "B301,B324,B403,B404,B603"
- "--severity-level"
- "high" # TODO: remove this line when we fix all the issues
- id: check-fab-migrations
language: pygrep
name: Check no migration is done on FAB related table
description: >
FAB tables are no longer used in core Airflow but in FAB provider.
As such, it is forbidden to create migrations related to FAB tables in core Airflow.
Such migrations should be in FAB provider. To achieve this, a new capability must be implemented:
support migrations for providers. In other words, providers need to be able to specify migrations
so that, any FAB related migration (besides the legacy ones) is defined in FAB provider.
See https://github.com/apache/airflow/issues/32210
entry: >
(?ix)
\bab_permission\b|
\bab_view_menu\b|
\bab_role\b|
\bab_permission_view\b|
\bab_permission_view_role\b|
\bab_user\b|
\bab_user_role\b|
\bab_register_user\b
pass_filenames: true
files: ^airflow/migrations/versions/.*\.py$
# These migrations contain FAB related changes but existed before moving FAB auth manager
# to its own provider
exclude: >
(?ix)
^airflow/migrations/versions/00.*\.py$|
^airflow/migrations/versions/0106.*\.py$|
^airflow/migrations/versions/0118.*\.py$|
^airflow/migrations/versions/0119.*\.py$|
^airflow/migrations/versions/0121.*\.py$|
^airflow/migrations/versions/0124.*\.py$
## ADD MOST PRE-COMMITS ABOVE THAT LINE
# The below pre-commits are those requiring CI image to be built
- id: mypy-dev
Expand Down
2 changes: 2 additions & 0 deletions STATIC_CODE_CHECKS.rst
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ require Breeze Docker image to be built locally.
+-----------------------------------------------------------+--------------------------------------------------------------+---------+
| check-extras-order | Check order of extras in Dockerfile | |
+-----------------------------------------------------------+--------------------------------------------------------------+---------+
| check-fab-migrations | Check no migration is done on FAB related table | |
+-----------------------------------------------------------+--------------------------------------------------------------+---------+
| check-for-inclusive-language | Check for language that we do not accept as community | |
+-----------------------------------------------------------+--------------------------------------------------------------+---------+
| check-google-re2-as-dependency | Check google-re2 is declared as dependency when needed | |
Expand Down
1 change: 1 addition & 0 deletions dev/breeze/src/airflow_breeze/pre_commit_ids.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
"check-executables-have-shebangs",
"check-extra-packages-references",
"check-extras-order",
"check-fab-migrations",
"check-for-inclusive-language",
"check-google-re2-as-dependency",
"check-hooks-apply",
Expand Down
2 changes: 1 addition & 1 deletion images/breeze/output_static-checks.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion images/breeze/output_static-checks.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
51f4354ec1eaa5a77cfc3dfe43ef1faa
26c037ac78063fdb0ff40832867e678d

0 comments on commit 6393b35

Please sign in to comment.