ViderStar Reviewing an Airflow DAG
Layering
dags/<name>.py should describe the DAG and nothing else: schedule, tasks, dependencies.
Orchestration, transformation and writing belong in modules the DAG imports. Business logic
inside the DAG file is the first thing to send back — it cannot be tested and it is
re-parsed by the scheduler forever.
Checklist
Idempotency. Re-running the same execution date must not double the data. Check that
the write replaces a partition, or goes through a deduplicating engine with a proper key,
rather than being a plain INSERT. This is the most common and the most expensive defect:
it surfaces weeks later, when the numbers have already diverged and nobody knows since when.
Heavy imports inside the task function, not at module level. The scheduler parses every
DAG file continuously; a top-level import torch slows down the whole instance, not just
this DAG.
Retries and a timeout. retries in default_args, and dagrun_timeout set. A DAG with
no timeout can hang indefinitely holding a slot.
An alert on failure. on_failure_callback wired to wherever the team actually looks. A
pipeline that fails silently is discovered by someone noticing a stale number.
Configuration through Variable.get('name', default), not constants in the code, with a
default sensible enough that a fresh environment comes up without manual setup.
Secrets only in Connections. Never in the code, never in a variables dump, never as the
default of a Variable.get. Fetch them by connection id at runtime.
Queries. Filter on the partitioning key, no SELECT *, batch inserts rather than
frequent small mutations. Keep SQL in files, not in string literals inside the pipeline.
max_active_runs and concurrency set deliberately. Heavy pipelines running twice in
parallel usually means two runs fighting over memory and over the same destination table.
Dependency and image versions bumped when the requirements file or the Dockerfile changed — otherwise the image is not rebuilt and the environment quietly runs old code.
Model artefacts
If the DAG pulls a model artefact from object storage, check the caching. A cache that
keys on a file name and skips the download when the file is non-empty means overwriting
latest.pt never reaches live workers. The correct rollout is to upload a versioned key
and switch a Variable to point at it, not to overwrite a mutable name.
What a review should output
Findings ordered by cost, each with the file and line and what to change. Distinguish "this will corrupt data" from "this is untidy" — a review that lists both at the same volume gets skimmed.