From 016d6306e41d4d43d4ec733e8a91464e70cbfa6b Mon Sep 17 00:00:00 2001 From: LIU ZHE YOU Date: Sat, 23 May 2026 12:43:32 +0800 Subject: [PATCH 1/2] Add prek hook to enforce HTTPException is imported from fastapi --- .pre-commit-config.yaml | 14 ++ ...heck_http_exception_import_from_fastapi.py | 129 ++++++++++++++++++ 2 files changed, 143 insertions(+) create mode 100755 scripts/ci/prek/check_http_exception_import_from_fastapi.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6b3635df81011..b48efb7b56c0d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -490,6 +490,20 @@ repos: ^providers/.*/src/.*\.py$| ^task-sdk/src/.*\.py$| ^shared/.*/src/.*\.py$ + - id: check-http-exception-import-from-fastapi + name: Check HTTPException is imported from fastapi in fastapi-using trees + entry: ./scripts/ci/prek/check_http_exception_import_from_fastapi.py + language: python + pass_filenames: true + files: > + (?x) + ^airflow-core/src/airflow/api_fastapi/.*\.py$| + ^airflow-core/tests/unit/api_fastapi/.*\.py$| + ^providers/amazon/(src|tests)/.*\.py$| + ^providers/common/ai/(src|tests)/.*\.py$| + ^providers/edge3/(src|tests)/.*\.py$| + ^providers/fab/(src|tests)/.*\.py$| + ^providers/keycloak/(src|tests)/.*\.py$ - id: check-secrets-search-path-sync name: Check sync between sdk and core entry: ./scripts/ci/prek/check_secrets_search_path_sync.py diff --git a/scripts/ci/prek/check_http_exception_import_from_fastapi.py b/scripts/ci/prek/check_http_exception_import_from_fastapi.py new file mode 100755 index 0000000000000..86c92871f41da --- /dev/null +++ b/scripts/ci/prek/check_http_exception_import_from_fastapi.py @@ -0,0 +1,129 @@ +#!/usr/bin/env python +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Check that ``HTTPException`` is imported from ``fastapi`` in fastapi-using trees. + +In code that targets FastAPI (``airflow-core/src/airflow/api_fastapi/``, the +matching tests under ``airflow-core/tests/unit/api_fastapi/``, and the +provider trees that wire FastAPI apps), every ``HTTPException`` must come +from ``fastapi`` (which re-exports the Starlette class). Two common mistakes +this hook catches: + +* ``from starlette.exceptions import HTTPException`` — a different class at + runtime; ``isinstance(exc, fastapi.HTTPException)`` and + ``pytest.raises(fastapi.HTTPException)`` will not match it. +* ``from http.client import HTTPException`` — an unrelated stdlib exception + whose constructor signature differs, so the route returns 500 instead of + the intended HTTP status. +""" + +# /// script +# requires-python = ">=3.10,<3.11" +# dependencies = [ +# "rich>=13.6.0", +# ] +# /// +from __future__ import annotations + +import argparse +import ast +import sys +from pathlib import Path + +from common_prek_utils import console + + +def _is_fastapi_module(module: str) -> bool: + """Return True if *module* is ``fastapi`` or a submodule of it.""" + return module == "fastapi" or module.startswith("fastapi.") + + +def check_file(file_path: Path) -> list[tuple[int, str]]: + """Return list of ``(line_number, import_statement)`` violations.""" + try: + source = file_path.read_text(encoding="utf-8") + tree = ast.parse(source, filename=str(file_path)) + except (OSError, UnicodeDecodeError, SyntaxError): + return [] + + violations: list[tuple[int, str]] = [] + + for node in ast.walk(tree): + if not isinstance(node, ast.ImportFrom) or not node.module: + continue + if _is_fastapi_module(node.module): + continue + bad_aliases = [alias for alias in node.names if alias.name == "HTTPException"] + if not bad_aliases: + continue + rendered = ", ".join( + alias.name if not alias.asname else f"{alias.name} as {alias.asname}" for alias in bad_aliases + ) + violations.append((node.lineno, f"from {node.module} import {rendered}")) + + return violations + + +def main() -> None: + parser = argparse.ArgumentParser( + description="Check that HTTPException is imported from fastapi" + ) + parser.add_argument("files", nargs="*", help="Files to check") + args = parser.parse_args() + + if not args.files: + return + + total_violations = 0 + + for file_path in [Path(f) for f in args.files]: + violations = check_file(file_path) + if not violations: + continue + if console: + console.print(f"[red]{file_path}[/red]:") + for line_num, statement in violations: + console.print(f" [yellow]Line {line_num}[/yellow]: {statement}") + else: + print(f"{file_path}:") + for line_num, statement in violations: + print(f" Line {line_num}: {statement}") + total_violations += len(violations) + + if total_violations: + message = ( + f"Found {total_violations} HTTPException import(s) not coming from `fastapi`.\n" + "Use `from fastapi import HTTPException` instead. Importing it from " + "`starlette.exceptions`, `http.client`, or any other module yields a " + "different class at runtime and breaks `isinstance` / `pytest.raises` " + "checks against `fastapi.HTTPException` (and, for `http.client`, calls " + "the wrong constructor so the route returns 500 instead of the intended " + "status)." + ) + if console: + console.print() + console.print(f"[red]{message}[/red]") + else: + print() + print(message) + sys.exit(1) + + +if __name__ == "__main__": + main() + sys.exit(0) From 49e36b07b8554a13f1d9ae9ec1ae4d89c5b06d2b Mon Sep 17 00:00:00 2001 From: LIU ZHE YOU Date: Sun, 24 May 2026 18:24:29 +0800 Subject: [PATCH 2/2] Move HTTPException-import hook into per-distribution prek configs Splits the single root-level hook entry into per-distribution `.pre-commit-config.yaml` files (airflow-core, providers/amazon, providers/common/ai, providers/edge3, providers/fab, providers/keycloak). Each entry is scoped to the subtree that actually wires a FastAPI app, so edge3's `cli/` (client) is excluded and only `worker_api/` and `plugins/` are checked. Also fixes a latent bug the hook caught in `airflow-core/src/airflow/api_fastapi/core_api/routes/ui/dags.py`, where `HTTPException` was imported from `http.client` and called with FastAPI's `(status_code, detail)` signature -- the route would return 500 instead of the intended 400 for `dag_id == "~"`. --- .pre-commit-config.yaml | 14 -------- airflow-core/.pre-commit-config.yaml | 9 +++++ providers/amazon/.pre-commit-config.yaml | 35 +++++++++++++++++++ providers/common/ai/.pre-commit-config.yaml | 9 +++++ providers/edge3/.pre-commit-config.yaml | 9 +++++ providers/fab/.pre-commit-config.yaml | 9 +++++ providers/keycloak/.pre-commit-config.yaml | 9 +++++ ...heck_http_exception_import_from_fastapi.py | 18 +++++----- 8 files changed, 90 insertions(+), 22 deletions(-) create mode 100644 providers/amazon/.pre-commit-config.yaml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b48efb7b56c0d..6b3635df81011 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -490,20 +490,6 @@ repos: ^providers/.*/src/.*\.py$| ^task-sdk/src/.*\.py$| ^shared/.*/src/.*\.py$ - - id: check-http-exception-import-from-fastapi - name: Check HTTPException is imported from fastapi in fastapi-using trees - entry: ./scripts/ci/prek/check_http_exception_import_from_fastapi.py - language: python - pass_filenames: true - files: > - (?x) - ^airflow-core/src/airflow/api_fastapi/.*\.py$| - ^airflow-core/tests/unit/api_fastapi/.*\.py$| - ^providers/amazon/(src|tests)/.*\.py$| - ^providers/common/ai/(src|tests)/.*\.py$| - ^providers/edge3/(src|tests)/.*\.py$| - ^providers/fab/(src|tests)/.*\.py$| - ^providers/keycloak/(src|tests)/.*\.py$ - id: check-secrets-search-path-sync name: Check sync between sdk and core entry: ./scripts/ci/prek/check_secrets_search_path_sync.py diff --git a/airflow-core/.pre-commit-config.yaml b/airflow-core/.pre-commit-config.yaml index aea65d35b0ee2..767964f0be5b2 100644 --- a/airflow-core/.pre-commit-config.yaml +++ b/airflow-core/.pre-commit-config.yaml @@ -148,6 +148,15 @@ repos: language: python pass_filenames: true files: ^src/airflow/.*\.py$ + - id: check-http-exception-import-from-fastapi + name: Check HTTPException is imported from fastapi + entry: ../scripts/ci/prek/check_http_exception_import_from_fastapi.py + language: python + pass_filenames: true + files: > + (?x) + ^src/airflow/api_fastapi/.*\.py$| + ^tests/unit/api_fastapi/.*\.py$ - id: create-missing-init-py-files-tests name: Create missing init.py files in tests entry: ../scripts/ci/prek/check_init_in_tests.py diff --git a/providers/amazon/.pre-commit-config.yaml b/providers/amazon/.pre-commit-config.yaml new file mode 100644 index 0000000000000..3e634c3ef2df7 --- /dev/null +++ b/providers/amazon/.pre-commit-config.yaml @@ -0,0 +1,35 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +--- +default_stages: [pre-commit, pre-push] +minimum_prek_version: '0.3.4' +default_language_version: + python: python3 + node: 22.19.0 + golang: 1.24.0 +repos: + - repo: local + hooks: + - id: check-http-exception-import-from-fastapi + name: Check HTTPException is imported from fastapi + entry: ../../scripts/ci/prek/check_http_exception_import_from_fastapi.py + language: python + pass_filenames: true + files: > + (?x) + ^src/airflow/providers/amazon/aws/auth_manager/.*\.py$| + ^tests/unit/amazon/aws/auth_manager/.*\.py$ diff --git a/providers/common/ai/.pre-commit-config.yaml b/providers/common/ai/.pre-commit-config.yaml index 1a7bc73596fcb..9676e56363da3 100644 --- a/providers/common/ai/.pre-commit-config.yaml +++ b/providers/common/ai/.pre-commit-config.yaml @@ -47,3 +47,12 @@ repos: entry: ../../../scripts/ci/prek/compile_provider_assets.py ai pass_filenames: false additional_dependencies: ['pnpm@10.25.0'] + - id: check-http-exception-import-from-fastapi + name: Check HTTPException is imported from fastapi + entry: ../../../scripts/ci/prek/check_http_exception_import_from_fastapi.py + language: python + pass_filenames: true + files: > + (?x) + ^src/airflow/providers/common/ai/plugins/.*\.py$| + ^tests/unit/common/ai/plugins/.*\.py$ diff --git a/providers/edge3/.pre-commit-config.yaml b/providers/edge3/.pre-commit-config.yaml index 90f5a0344da26..ec6accf2a86fa 100644 --- a/providers/edge3/.pre-commit-config.yaml +++ b/providers/edge3/.pre-commit-config.yaml @@ -30,6 +30,15 @@ repos: entry: ../../scripts/ci/prek/generate_openapi_spec_providers.py edge pass_filenames: false files: ^src/airflow/providers/edge3/worker_api/.*\.py$ + - id: check-http-exception-import-from-fastapi + name: Check HTTPException is imported from fastapi + entry: ../../scripts/ci/prek/check_http_exception_import_from_fastapi.py + language: python + pass_filenames: true + files: > + (?x) + ^src/airflow/providers/edge3/(worker_api|plugins)/.*\.py$| + ^tests/unit/edge3/(worker_api|plugins)/.*\.py$ - id: ts-compile-lint-edge-ui name: Compile / format / lint edge UI description: TS types generation / ESLint / Prettier new UI files in Edge Provider diff --git a/providers/fab/.pre-commit-config.yaml b/providers/fab/.pre-commit-config.yaml index 5d59ede01f8c2..4396285f744a1 100644 --- a/providers/fab/.pre-commit-config.yaml +++ b/providers/fab/.pre-commit-config.yaml @@ -42,6 +42,15 @@ repos: entry: ../../scripts/ci/prek/generate_openapi_spec_providers.py fab pass_filenames: false files: ^src/airflow/providers/fab/auth_manager/api_fastapi/.*\.py$ + - id: check-http-exception-import-from-fastapi + name: Check HTTPException is imported from fastapi + entry: ../../scripts/ci/prek/check_http_exception_import_from_fastapi.py + language: python + pass_filenames: true + files: > + (?x) + ^src/airflow/providers/fab/auth_manager/api_fastapi/.*\.py$| + ^tests/unit/fab/auth_manager/api_fastapi/.*\.py$ - id: update-migration-references-fab name: Update migration ref doc for FAB language: python diff --git a/providers/keycloak/.pre-commit-config.yaml b/providers/keycloak/.pre-commit-config.yaml index 68cdb82835ce6..2215a6cd5a0f5 100644 --- a/providers/keycloak/.pre-commit-config.yaml +++ b/providers/keycloak/.pre-commit-config.yaml @@ -30,3 +30,12 @@ repos: entry: ../../scripts/ci/prek/generate_openapi_spec_providers.py keycloak pass_filenames: false files: ^src/airflow/providers/keycloak/auth_manager/.*\.py$ + - id: check-http-exception-import-from-fastapi + name: Check HTTPException is imported from fastapi + entry: ../../scripts/ci/prek/check_http_exception_import_from_fastapi.py + language: python + pass_filenames: true + files: > + (?x) + ^src/airflow/providers/keycloak/auth_manager/.*\.py$| + ^tests/unit/keycloak/auth_manager/.*\.py$ diff --git a/scripts/ci/prek/check_http_exception_import_from_fastapi.py b/scripts/ci/prek/check_http_exception_import_from_fastapi.py index 86c92871f41da..734dfb83b8751 100755 --- a/scripts/ci/prek/check_http_exception_import_from_fastapi.py +++ b/scripts/ci/prek/check_http_exception_import_from_fastapi.py @@ -18,11 +18,15 @@ # under the License. """Check that ``HTTPException`` is imported from ``fastapi`` in fastapi-using trees. -In code that targets FastAPI (``airflow-core/src/airflow/api_fastapi/``, the -matching tests under ``airflow-core/tests/unit/api_fastapi/``, and the -provider trees that wire FastAPI apps), every ``HTTPException`` must come -from ``fastapi`` (which re-exports the Starlette class). Two common mistakes -this hook catches: +The hook is wired into per-distribution ``.pre-commit-config.yaml`` files +(``airflow-core``, ``providers/amazon``, ``providers/common/ai``, +``providers/edge3``, ``providers/fab``, ``providers/keycloak``), each +scoped to the subtree that actually wires a FastAPI app. Provider trees +that mix client and server code (e.g. edge3's ``cli/`` is a client) are +scoped to the server-side subfolders only to avoid false positives on +stdlib HTTP usage in the client. Within those scopes, every +``HTTPException`` must come from ``fastapi`` (which re-exports the +Starlette class). Two common mistakes this hook catches: * ``from starlette.exceptions import HTTPException`` — a different class at runtime; ``isinstance(exc, fastapi.HTTPException)`` and @@ -80,9 +84,7 @@ def check_file(file_path: Path) -> list[tuple[int, str]]: def main() -> None: - parser = argparse.ArgumentParser( - description="Check that HTTPException is imported from fastapi" - ) + parser = argparse.ArgumentParser(description="Check that HTTPException is imported from fastapi") parser.add_argument("files", nargs="*", help="Files to check") args = parser.parse_args()