airflow code-review.instructions.md

Airflow Code Review Instructions

Use these rules when reviewing pull requests to the Apache Airflow repository.

Architecture Boundaries

• Scheduler must never run user code. It only processes serialized Dags. Flag any scheduler-path code that deserializes or executes Dag/task code.
• Flag any task execution code that accesses the metadata DB directly instead of through the Execution API (/execution endpoints).
• Flag any code in Dag Processor or Triggerer that breaks process isolation — these components run user code in separate processes from the Scheduler and API Server, but note that they potentially have direct metadata database access and potentially bypass JWT authentication via in-process Execution API transport. This is an intentional design choice documented in the security model, not a security vulnerability.
• Flag any provider importing core internals like SUPERVISOR_COMMS or task-runner plumbing. Providers interact through the public SDK and execution API only.

Database and Query Correctness

• Flag any SQLAlchemy relationship access inside a loop without joinedload() or selectinload() — this is an N+1 query.
• Flag any query on run_id without dag_id. run_id is only unique per Dag. Queries that filter, group, partition, or join on run_id alone will silently collide across Dags.
• Flag any session.commit() call in airflow-core code that receives a session parameter. Session lifecycle is managed by the caller, not the callee.
• Flag any session parameter that is not keyword-only (*, session) in airflow-core.
• Flag any database-specific SQL (e.g., LATERAL joins, PostgreSQL-only functions, MySQL-only syntax) without cross-DB handling. SQL must work on PostgreSQL, MySQL, and SQLite.

Code Quality Rules

• Flag any assert in non-test code. assert is stripped in optimized Python (python -O), making it a silent no-op in production.
• Flag any time.time() used for measuring durations. Use time.monotonic() instead — time.time() is affected by system clock adjustments.
• Flag any from or import statement inside a function or method body. Imports must be at the top of the file. The only valid exceptions are: (1) circular import avoidance, (2) lazy loading for worker isolation, (3) TYPE_CHECKING blocks. If the import is inside a function, ask the author to justify why it cannot be at module level.
• 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 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 files) is not a new usage — do not flag it, but confirm the diff removes the identical raise elsewhere and leaves it otherwise unchanged. Do not suggest changing specific exceptions back to AirflowException.