This is an automated email from the ASF dual-hosted git repository.
shahar1 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new c5bbd4b087a Instruct agents to reduce direct AirflowException usage
(#68656)
c5bbd4b087a is described below
commit c5bbd4b087a7691f650076e03fdce503124aa628
Author: Shahar Epstein <[email protected]>
AuthorDate: Wed Jun 17 11:30:57 2026 +0300
Instruct agents to reduce direct AirflowException usage (#68656)
Make explicit on both the authoring and review sides that direct
AirflowException raises are being reduced, not added. New direct usages
are disallowed (already enforced by the check-no-new-airflow-exceptions
prek hook), and the reviewer guidance clarifies that the only acceptable
movement of an AirflowException line is relocating an already-existing
one verbatim during a refactor, which is not a new usage.
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
---
.github/instructions/code-review.instructions.md | 2 +-
AGENTS.md | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/.github/instructions/code-review.instructions.md
b/.github/instructions/code-review.instructions.md
index 08eec81085d..da2674c4c1a 100644
--- a/.github/instructions/code-review.instructions.md
+++ b/.github/instructions/code-review.instructions.md
@@ -32,7 +32,7 @@ Use these rules when reviewing pull requests to the Apache
Airflow repository.
- **Flag any `@lru_cache(maxsize=None)`.** This creates an unbounded cache —
every unique argument set is cached forever. Note: `@lru_cache()` without
arguments defaults to `maxsize=128` and is fine.
- **Flag any heavy import** (e.g., `kubernetes.client`) in multi-process code
paths that is not behind a `TYPE_CHECKING` guard.
- **Flag any file, connection, or session opened without a context manager or
`try/finally`.**
-- **Flag any new `raise AirflowException` usage.** The community has stopped
adding new ones (enforced by the `check-no-new-airflow-exceptions` prek hook) —
prefer Python's standard exceptions (`ValueError`, `TypeError`, `OSError`), or
a dedicated class in the appropriate `exceptions.py`. **Do not suggest changing
specific exceptions back to `AirflowException`.**
+- **Flag any new `raise AirflowException` usage.** The community is reducing
direct `AirflowException` usage, not increasing it; new ones are not allowed
(enforced by the `check-no-new-airflow-exceptions` prek hook) — prefer Python's
standard exceptions (`ValueError`, `TypeError`, `OSError`), or a dedicated
class in the appropriate `exceptions.py`. **The one exception is a pure
relocation: an already-existing `AirflowException` moved verbatim during a
refactor (e.g. code moved between fi [...]
## Testing Requirements
diff --git a/AGENTS.md b/AGENTS.md
index 4f8b4a37d44..f0eb03813f4 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -139,7 +139,7 @@ reported as such are described in "What is NOT considered a
security vulnerabili
- In `airflow-core`, functions with a `session` parameter must not call
`session.commit()`. Use keyword-only `session` parameters.
- Imports at top of file. Valid exceptions: circular imports, lazy loading for
worker isolation, `TYPE_CHECKING` blocks.
- Guard heavy type-only imports (e.g., `kubernetes.client`) with
`TYPE_CHECKING` in multi-process code paths.
-- Define dedicated exception classes or use existing exceptions such as
`ValueError` instead of raising the broad `AirflowException` directly. Each
error case should have a specific exception type that conveys what went wrong.
+- Define dedicated exception classes or use existing exceptions such as
`ValueError` instead of raising the broad `AirflowException` directly. Each
error case should have a specific exception type that conveys what went wrong.
**Never add new direct `raise AirflowException(...)` usages — the community is
actively reducing them, not adding more, and the
`check-no-new-airflow-exceptions` prek hook enforces this across
`airflow-core`, `airflow-ctl`, `task-sdk`, `providers`, and `shared`.** [...]
- Translate domain-layer exceptions to `HTTPException` at FastAPI route
boundaries. In `airflow-core/src/airflow/core_api/` route handlers, catch
errors raised by domain code (e.g., `ValueError` from
`airflow.state.metastore.MetastoreStateBackend` for a missing row or invalid
input) and re-raise as `HTTPException` with the right status (`404` for
not-found, `400` for invalid input). Otherwise they propagate as `500 Internal
Server Error`, leaking internals and misleading clients.
- Bulk `DELETE`/`UPDATE` in the scheduler loop or any synchronous interval
task (e.g. `call_regular_interval` callbacks) must be batched with `LIMIT` and
committed between batches — never issue a single unbounded bulk write against a
user-driven table. Unbounded bulk writes hold row locks for the entire
transaction (blocking concurrent writers) and stall the scheduler main loop.
Filter columns used by the cleanup must be indexed. Follow the batching pattern
in `airflow-core/src/airflow/u [...]
- Name functions and methods with action verbs: `get_`, `extract_`, `find_`,
`compute_`, `build_`, etc. Avoid noun-only names like `_serialize_keys` or
`_base_names` — they read as attributes, not callables. Predicates (`is_`,
`has_`) are the one exception.
