Skip to content

Add prek hook to enforce HTTPException is imported from fastapi#67367

Open
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:ci/prek-guard-api-exception-import
Open

Add prek hook to enforce HTTPException is imported from fastapi#67367
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:ci/prek-guard-api-exception-import

Conversation

@jason810496
Copy link
Copy Markdown
Member

Why

fastapi.HTTPException, starlette.exceptions.HTTPException, and http.client.HTTPException are three different classes — mixing them silently breaks isinstance / pytest.raises checks, and http.client.HTTPException has a different constructor so routes return 500 instead of the intended status (the exact bug #67363 fixed in core_api/routes/ui/dags.py).

What

  • Add scripts/ci/prek/check_http_exception_import_from_fastapi.py — an AST-based prek hook that flags any from <module> import HTTPException where <module> is not fastapi (or a fastapi.* submodule).
  • Register it in .pre-commit-config.yaml, scoped to the FastAPI-using trees: airflow-core/src/airflow/api_fastapi/, airflow-core/tests/unit/api_fastapi/, and the src/ + tests/ of the providers that wire FastAPI apps (amazon, common/ai, edge3, fab, keycloak).

Was generative AI tooling used to co-author this PR?

@jason810496 jason810496 self-assigned this May 23, 2026
@boring-cyborg boring-cyborg Bot added area:dev-tools backport-to-v3-2-test Mark PR with this label to backport to v3-2-test branch labels May 23, 2026
@jason810496
Copy link
Copy Markdown
Member Author

jason810496 commented May 23, 2026

I expect CI should fail before #67363 get merged.

@jason810496 jason810496 removed the backport-to-v3-2-test Mark PR with this label to backport to v3-2-test branch label May 23, 2026
@jason810496 jason810496 force-pushed the ci/prek-guard-api-exception-import branch from 73d4eca to 59e987f Compare May 23, 2026 05:55
Comment thread .pre-commit-config.yaml
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd propose to keep the check general where it is but making the prek entries into the specific subfolders. e.g. the core things into /airflow-core/.pre-commit-config.yaml

Comment thread .pre-commit-config.yaml
^airflow-core/tests/unit/api_fastapi/.*\.py$|
^providers/amazon/(src|tests)/.*\.py$|
^providers/common/ai/(src|tests)/.*\.py$|
^providers/edge3/(src|tests)/.*\.py$|
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That check willr aise false-positives. Edge containes both client and server sides.

All code in providers/edge3/src/airflow/providers/edge3/worker_api and providers/edge3/src/airflow/providers/edge3/plugins is the API/Server side but in providers/edge3/src/airflow/providers/edge3/cli is is the client (where the FastAPI import will be wrong!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants