Improve query validation, including for streaming#67212
Merged
Conversation
bbovenzi
requested review from
choo121600,
guan404ming,
pierrejeambrun,
ryanahamilton,
shubhamraj-git and
vatsrahul1001
as code owners
bbovenzi
added
the
backport-to-v3-2-test
pierrejeambrun
added a commit
to astronomer/airflow
that referenced
this pull request
May 21, 2026
Bulk delete only invalidated the top-level dag-runs and task-instances lists. The grid view, the dag-details / latest-run-info queries, and the mapped task instance list also depend on the deleted runs and stayed stale until a manual refresh. Mirror the invalidation set ``useBulkClearTaskInstances`` uses (apache#67212), minus the per-attempt keys (logs / extra links / try details) — those don't change when a Dag Run is deleted; the orphaned cache entries are simply never queried again. The affected ``dag_id`` set is captured in ``bulkAction`` from the request body and read in ``onSuccess``, same lifecycle as ``useBulkClearTaskInstances``'s ``byDagRun`` grouping.
pierrejeambrun
added a commit
to astronomer/airflow
that referenced
this pull request
May 21, 2026
Bulk delete only invalidated the top-level dag-runs and task-instances lists. The grid view, the dag-details / latest-run-info queries, the mapped task instance list, and per-attempt TI caches (log / extra links / try details) all depend on the deleted runs and stayed stale until a manual refresh — including when the user navigates back to a TI try detail via the browser back button after deleting its run, where the cached log would have been served instead of the now-404. Mirrors the invalidation set ``useBulkClearTaskInstances`` uses (introduced in apache#67212). The affected ``dag_id`` set is captured in ``bulkAction`` from the request body and read in ``onSuccess``, same lifecycle as ``useBulkClearTaskInstances``'s ``byDagRun`` grouping.
pierrejeambrun
added a commit
to astronomer/airflow
that referenced
this pull request
May 21, 2026
Bulk delete only invalidated the top-level dag-runs and task-instances lists. Two more cache sets stay stale otherwise: - Per-attempt TI caches (log / extra links / try details), keyed by TI identity. When the user navigates back to a TI try detail via the browser back button after deleting its run, react-query serves the cached log instead of letting the request hit and return 404. - The grid view query set for each affected Dag — the grid renders one bar per Dag Run, so bulk delete literally removes bars. The per-attempt set mirrors the addition ``useBulkTaskInstances`` already gained in apache#67212. The grid invalidation is specific to this hook because deleting Dag Runs (unlike deleting TIs) changes what the grid bars themselves represent. The affected ``dag_id`` set is captured in ``bulkAction`` from the request body and read in ``onSuccess``, same lifecycle as ``useBulkClearTaskInstances``'s ``byDagRun`` grouping.
pierrejeambrun
added a commit
to astronomer/airflow
that referenced
this pull request
May 21, 2026
Bulk delete only invalidated the top-level dag-runs and task-instances lists. Two more cache sets stay stale otherwise: - Per-attempt TI caches (log / extra links / try details), keyed by TI identity. When the user navigates back to a TI try detail via the browser back button after deleting its run, react-query serves the cached log instead of letting the request hit and return 404. - The grid view query set for each affected Dag — the grid renders one bar per Dag Run, so bulk delete literally removes bars. The per-attempt set mirrors the addition ``useBulkTaskInstances`` already gained in apache#67212. The grid invalidation is specific to this hook because deleting Dag Runs (unlike deleting TIs) changes what the grid bars themselves represent. The affected ``dag_id`` set is captured in ``bulkAction`` from the request body and read in ``onSuccess``, same lifecycle as ``useBulkClearTaskInstances``'s ``byDagRun`` grouping.
pierrejeambrun
added a commit
to astronomer/airflow
that referenced
this pull request
May 21, 2026
Bulk delete only invalidated the top-level dag-runs and task-instances lists. Two more cache sets stay stale otherwise: - Per-attempt TI caches (log / extra links / try details), keyed by TI identity. When the user navigates back to a TI try detail via the browser back button after deleting its run, react-query serves the cached log instead of letting the request hit and return 404. - The grid view query set for each affected Dag — the grid renders one bar per Dag Run, so bulk delete literally removes bars. The per-attempt set mirrors the addition ``useBulkTaskInstances`` already gained in apache#67212. The grid invalidation is specific to this hook because deleting Dag Runs (unlike deleting TIs) changes what the grid bars themselves represent. The affected ``dag_id`` set is captured in ``bulkAction`` from the request body and read in ``onSuccess``, same lifecycle as ``useBulkClearTaskInstances``'s ``byDagRun`` grouping.
pierrejeambrun
added a commit
to astronomer/airflow
that referenced
this pull request
May 21, 2026
Bulk delete only invalidated the top-level dag-runs and task-instances lists. Two more cache sets stay stale otherwise: - Per-attempt TI caches (log / extra links / try details), keyed by TI identity. When the user navigates back to a TI try detail via the browser back button after deleting its run, react-query serves the cached log instead of letting the request hit and return 404. - The grid view query set for each affected Dag — the grid renders one bar per Dag Run, so bulk delete literally removes bars. The per-attempt set mirrors the addition ``useBulkTaskInstances`` already gained in apache#67212. The grid invalidation is specific to this hook because deleting Dag Runs (unlike deleting TIs) changes what the grid bars themselves represent. The affected ``dag_id`` set is captured in ``bulkAction`` from the request body and read in ``onSuccess``, same lifecycle as ``useBulkClearTaskInstances``'s ``byDagRun`` grouping.
pierrejeambrun
added a commit
that referenced
this pull request
May 21, 2026
* Add bulk delete endpoint for Dag Runs
Restores feature parity with Airflow 2.x where DagRunModelView exposed
collective Delete on the Dag Runs list view. Adds:
- PATCH /dags/{dag_id}/dagRuns — bulk endpoint structured like the
existing bulk task-instances endpoint. Only ``delete`` is supported in
this PR; ``create`` and ``update`` are wired to return 405 in the
BulkResponse so future PRs can fill them in without changing the
route surface.
- BulkDagRunService with deletable-state enforcement (matches the
single-run delete: only QUEUED / SUCCESS / FAILED can be deleted),
per-Dag authorization caching for the wildcard path
/dags/~/dagRuns, and ``action_on_non_existence: fail | skip``
semantics.
- Row selection + a Delete bulk action on the runs list page,
mirroring how Task Instances does it.
Bulk Mark-as and Bulk Clear are intentionally out of scope and will
follow in separate PRs.
The grid view stays single-select; multi-select on the grid was not
available in 2.x either, and the runs list page is the natural target
for bulk operations on a filtered set (e.g. state=failed).
closes: #52439
* Address self-review on bulk delete Dag Runs
- Switch BulkDagRunService._fetch_dag_runs to tuple_(dag_id, run_id).in_()
to avoid a Cartesian over-fetch when /dags/~/dagRuns is called with
pairs spanning multiple Dags. Matches BulkTaskInstanceService.
- Narrow _check_dag_authorization's method type to Literal["DELETE"];
this PR only does delete, no point in exposing PUT/POST/GET on the
signature.
- Add a wildcard test that exercises the per-Dag authorization path
(limited user accepted for one Dag, rejected with 403 in the
BulkResponse for a team-restricted Dag).
* Apply Brent's review feedback from the closed twin PR
Pulls in the substantive UI feedback @bbovenzi left on #66554 so this
PR lands without re-litigating the same comments. The closed PR was
broader (clear / mark / delete) but the structural feedback applies
verbatim to delete-only:
- Move all DagRuns-related files into ``pages/DagRuns/``, mirroring
``pages/TaskInstances/``. Adds a small ``index.ts`` re-export so
``router.tsx`` keeps the same import path.
- Rename ``useBulkDagRuns`` to ``useBulkDeleteDagRuns`` so the hook
name matches the single button it serves (one-hook-one-button
symmetry — when we add bulk update/clear we'll add sibling hooks).
- Stop hand-rolling pending/error state with ``useState<unknown>``;
read ``error`` straight off ``useMutation``'s return.
- Surface ALL per-entity errors from ``BulkResponse.delete.errors``
instead of just the first; render each as its own ``Alert`` row.
- Only invalidate the dag-runs / task-instances queries when at least
one entry actually succeeded — a 200 with all-errors should not
churn the table.
- Keep the dialog open when per-entity errors come back so the user
can read what failed; ``reset()`` clears them on close.
* Address inline review: mirror task-instance bulk delete patterns
Backend:
- Inline ``_resolve_dag_id``, ``_result_key`` and ``_fetch_dag_runs`` —
each had exactly one caller. Memory rule added so future PRs avoid
premature helpers.
- Drop the deletable-state restriction. Bulk task-instance delete has no
such restriction; bulk Dag-run delete shouldn't either.
- Emit one 404 error per missing entity when ``action_on_non_existence``
is ``fail`` instead of collating them into a single error, and stop
early-returning so matched runs still get deleted. The invariant
``len(success) + len(errors) == len(requested entities)`` now holds.
- Distinguish the two ways a wildcard can leak into ``dag_id`` (path
vs body) in the 400 message, mirroring
``BulkTaskInstanceService._categorize_entities``.
UI:
- Mirror ``useBulkTaskInstances`` exactly: bring back ``useState`` for
the error, the shared ``handleActionResult`` helper, single-error
surfacing via ``ErrorAlert``, and the ``bulkAction(requestBody)``
shape so the consumer constructs the full ``BulkBody``. Brent's prior
review of the closed twin PR pushed past this pattern, but until TI
is updated we want both hooks symmetrical — a follow-up can improve
both at once.
- Inline the affected-runs column array into ``BulkDeleteDagRunsButton``
and delete the standalone ``bulkDagRunsColumns.tsx`` file (single
caller).
* Move bulk Dag Run authorization to a dedicated route dependency
Adds ``requires_access_dag_run_bulk`` in ``core_api/security.py``
following the ``requires_access_connection_bulk`` pattern. The
dependency reads the parsed ``BulkBody[BulkDAGRunBody]``, resolves
each entity's ``dag_id`` (body wins, falling back to the path),
collects team mappings per Dag, and uses ``batch_is_authorized_dag``
to enforce auth before the route handler runs.
The route now declares only this dependency plus ``action_logging``;
the per-entity auth check is no longer duplicated inside
``BulkDagRunService``. Unauthorized requests fail with a single
route-level 403 instead of returning 200 with per-entity 403s in the
``BulkResponse``, matching how connections / pools / variables behave.
* Mirror single-run DELETE state restriction on bulk delete
Single ``DELETE /dags/{dag_id}/dagRuns/{dag_run_id}`` rejects RUNNING
runs with 409; bulk delete now does the same — a RUNNING entity yields
a per-entity 409 in ``BulkResponse.errors`` and the matched non-running
entities still get deleted.
Also renames ``DAGRunPatchStates`` to ``DagRunMutableStates`` since it
now gates both PATCH (mark-as) and DELETE — same set of states (QUEUED,
SUCCESS, FAILED), broader meaning. Propagated through the route handlers,
the bulk service, and the UI components that import the generated type.
* Invalidate dependent queries after bulk delete Dag Run
Bulk delete only invalidated the top-level dag-runs and task-instances
lists. Two more cache sets stay stale otherwise:
- Per-attempt TI caches (log / extra links / try details), keyed by
TI identity. When the user navigates back to a TI try detail via
the browser back button after deleting its run, react-query serves
the cached log instead of letting the request hit and return 404.
- The grid view query set for each affected Dag — the grid renders
one bar per Dag Run, so bulk delete literally removes bars.
The per-attempt set mirrors the addition ``useBulkTaskInstances``
already gained in #67212. The grid invalidation is specific to this
hook because deleting Dag Runs (unlike deleting TIs) changes what the
grid bars themselves represent.
The affected ``dag_id`` set is captured in ``bulkAction`` from the
request body and read in ``onSuccess``, same lifecycle as
``useBulkClearTaskInstances``'s ``byDagRun`` grouping.
* Restore resolve_run_on_latest_version import dropped during rebase
Rebasing the branch onto the latest ``apache/main`` with the
``-X theirs`` strategy used the incoming side for every conflict in
``routes/public/dag_run.py``. That dropped the
``resolve_run_on_latest_version`` import that #63884 added — the
function call at line 336 was preserved but the import line wasn't,
so the module failed to load and every test in ``test_dag_run.py``
crashed at collection (Static checks, mypy, and all DB-core test
matrices on CI).
This commit only adds the missing import back; no other change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Was generative AI tooling used to co-author this PR?
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.