# Dag Review

> Review an Airflow DAG before it is merged: idempotency, retries and timeouts, failure alerting, heavy imports at module level, configuration through Variables, secrets in Connections, and concurrency settings. Use when asked to "review this DAG", "check the pipeline", "is this ready to merge", "why does the DAG fail", "add a new DAG", "optimise this DAG". Not for research notebooks.

- Skill: `viderstar/dag-review` (Agent Skill)
- Install (CLI): `npx skillmds@latest add viderstar/dag-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/viderstar/dag-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: DevOps & Infra
- Author: ViderStar (https://skillmd.com/u/viderstar)
- Updated: 2026-09-22
- Page: https://skillmd.com/skills/viderstar/dag-review

---


# 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.