ashb opened a new pull request, #74931:
URL: https://github.com/apache/airflow/pull/74931
* Filter task instances by loop and iteration
Regions are how the server locates an execution; they are not something a
user should
have to know to narrow a table. The loop filter is expressed the way the Dag
was
written -- the loop's task group id and the pass number -- and the server
resolves that
to the regions behind it.
Loop and iteration are one control rather than two. An iteration index means
nothing
until you say which loop counts it, so the two could previously be set
independently
into combinations that matched nothing: a pass without a loop, or a pass the
loop never
reached. Both now sit behind a single pill whose options come from the
loop's own
summary, so only passes that exist are offered, and removing the pill clears
both
params. This follows the Run state pill, which already spreads one value
over several
URL params.
The same options render as a strip above the table, because a loop's shape
-- how many
passes it took and how they went -- is what you came to the page for, and a
menu you
have to open is a poor place for it. Each chip is also the control that
narrows to it,
and clicking the active one clears it. Past ten passes the chips collapse
into the rich
dropdown, the threshold task tries already settled on.
A looped group no longer has a tab of its own. Its page is its task
instances, the same
as any other group, with the strip and filter carrying the loop vocabulary.
The strip
is scoped to the loop the group belongs to rather than every loop in the
Dag; group ids
are dotted paths, so a group nested inside a loop reports the loop enclosing
it.
Grid links now carry the newest region coordinate. A task inside a loop has
one row per
iteration, so an unscoped link was ambiguous and the API rejected it with "a
loop
producer requires an explicit scope".
isRegional answers whether a task instance carries a region; it is not the
same question
as whether a loop produced it, and a mapped expansion is regional without
being in a
loop. Cross-run scope and the mapped-tasks tab ask isLoopTaskInstance
instead, backed by
the in_loop the schema already reports.
Affected-task selection goes back to being keyed by coordinate. Keying it by
row id
meant an exclusion the user ticked was lost when a retry came back under a
new id -- the
case its own test describes.
The filter registry no longer fetches a Dag structure for every filter bar
in the app;
the page that knows which Dag is in view supplies the loops.
Drops the translation keys the rewrite orphaned.
* Select a loop iteration from the breadcrumb
A pass is part of where you are rather than a filter you applied -- the same
task id means a
different run of it in every pass -- so it becomes its own breadcrumb level,
built on the
switcher the Dag and run levels already use. A task group can be read whole
and keeps an
"all" entry; a task instance is one row and must name a pass. Picking one
moves the region
coordinate as well as the filter, so the page shows that pass rather than
merely narrowing
to it, and a pass carried over from a different loop is ignored instead of
being read as a
position in this one.
The task instances list loses its iteration strip. The loop pill already
offers every loop
in scope with its passes, so the strip was a second way to say the same
thing.
The loop filter now survives navigation. Coordinates pin one execution and
must not follow
you to another, but a pass is where you are in a loop, so it stays as you
move between the
loop's tasks; a page outside that loop ignores it. Task instances can be
ordered by
iteration, and carry a Loop column so a list spanning loops says which one
each row is from.
The gantt outlines the pinned pass, over the whole group or the single row
depending on
which is being read, and clicking a bar selects the pass it was drawn for.
Graph links carry
the newest region coordinate for the same reason grid links do: a task
inside a loop has one
row per pass, so an unscoped link is ambiguous and the API rejects it.
Checkbox options move to the left of a dialog footer, away from Cancel and
Confirm. This
replaces a margin hack in the clear dialog and gives the other dialogs the
same shape.
Also fixes clearing a task instance, which had been failing silently: the
dialog closed
itself as soon as the request was sent, and unmounting took its mutation
observer with it,
so neither the error toast nor the query invalidation ever ran. Closing is
left to the
mutation's success path.
* Address review: drop the dead execution changes and the broken sort
The UI no longer calls getExecution, so the route keeps the shape it had on
main rather
than gaining order_by, in_loop and region nesting that nothing reaches. Its
translations
go with it.
Ordering by iteration is withdrawn. A row's own region index is its map slot
once a mapped
task runs inside a loop, so it does not order by what the column shows, and
the enclosing
pass is an ancestry walk rather than a column. Sorting on region_index while
the cursor
reported the pass would page wrongly, which is worse than the raise it
replaced.
Clearing is fixed on main by #74725: the confirmation dialog was dismissed
twice over by
the layer stack, not by its mutation observer being torn down. This drops
the change that
guessed otherwise, which also reopened the main dialog mid-clear on the
gated path.
A link that moves the coordinate now clears the pass it leaves behind, so
the breadcrumb
and the Gantt cannot name a pass the page is not on. The loop filter is sent
only with a
run, since the endpoint rejects it without one. The grid structure is not
requested
without a Dag, and the task instance stream is only materialised for a Dag
with loops.
Also removes two imports of a module this branch deleted, which were failing
static checks.
* Address review: structure-based loop scope, polling, and an exact region
filter
Loop membership is read from the Dag structure rather than the task id. A
loop declared
with prefix_group_id=False does not name its members after itself, so
matching on the
prefix lost them the breadcrumb, the filter and the Gantt highlight.
region_id goes back to naming one region exactly, and stops rejecting the
cross-run form.
The logical scope of a pass -- the loop's own region plus the expansions
nested in it --
is what loop_id and iteration are for, so the two filters keep one meaning
each. The
coverage for that pair comes back alongside the region case, including the
two guards.
The loop options follow the run the way the loop summary does, so a running
loop's passes
and their states no longer sit stale behind a five minute staleTime.
Also fixes the task instance stream being drained before the summaries were
built: without
loops it is walked once, and the pass that looks for the newest iteration
has nothing to
find, so it is skipped rather than consuming it.
* Take a row's loop iteration from the server, and count every pass
The breadcrumb worked out which pass a page was on by reading region_index
off the url.
For a mapped expansion that index is the map slot rather than the pass, so
it searched the
pass list for a slot number, found nothing, and dropped the iteration level
entirely --
leaving a mapped task inside a loop with no way to see or change its pass. A
row already
knows its own pass: loop_iteration is resolved server side and holds for an
expansion and
for a pass a clear forked into a successor region alike. Only a group, which
is not one
row, still reads the url.
Map index and iteration are separate levels and now stay that way, rather
than one url
field being asked to mean both.
State counts cover every pass. A looped task's cell stands for the work the
loop did and
the UI says how many iterations that covers, so an earlier pass put back to
work by a
clear is visible instead of hidden behind a finished later one. The
coordinate still comes
from the newest pass, since a link has to address a row.
* Resolve a loop pass to the row it became, instead of guessing a coordinate
Switching iteration on a task instance page wrote the loop's region and the
pass number as
if they addressed a row. They do not: a mapped expansion lives in its own
region, and a
clear forks later passes into a successor one. The pair named no row, and
asking for a
looped task instance that way is rejected rather than answered empty, so
navigating
between a loop's tasks could fail outright.
The switcher now writes only what it knows -- the loop and the pass. The
page resolves
that to a coordinate through the list endpoint, which already takes the pair
and returns
each row's own region, and writes the answer back into the url. Everything
downstream
keeps reading the url unchanged, and the resolved link is the one worth
sharing.
The lookup sits on the page rather than in the shared coordinate hook, which
nineteen
components use: a query there would have every one of them make a request.
The page holds its children while the lookup is in flight. The request has
to be withheld
rather than made and handled, since there is no scope to make it with.
* Open the Gantt highlight where one run's segments meet
An outline rings all four sides, so a run drawn as a queued stretch followed
by its state
showed the highlight down the seam between them -- an edge inside the thing
being
highlighted rather than its boundary. The sides are drawn individually now,
and the two
that already know they are touching a neighbour are left off.
* Address review: group membership, paging, and the loop a summary is asked
for
The Gantt group highlight decided membership from the task id, so a group
declared with
prefix_group_id=False lost it. It reads the structure now, the way the loop
scope already
does.
Switching pass restarts paging. A page of one pass says nothing about where
the next one
begins, so carrying its cursor across left the table on rows from the pass
before.
A task instance page no longer offers its own row's region as the loop to
summarise. Only
a group page's region names an invocation: a row's region is an expansion,
or a pass a
clear forked, and asking for a run summary by one of those is rejected --
which left the
header falling back on a count of itself.
* Highlight only a pass the reader chose in the Gantt
The highlight fell back to the region index when no iteration was named.
That index is the
map slot on a mapped expansion, so it rang bars of whatever pass happened to
share the
number -- and resolving a pass to a row now writes a region index into the
url far more
often, which made the fallback fire where it used to be rare.
* Keep earlier attempts on the Gantt chart after a clear
Every attempt is its own row and only the newest stays live, and ORM selects
read live rows
unless they ask otherwise. The chart did not, so clearing a task took its
earlier tries off
the chart entirely -- including the attempt that the pending-retry filter
just above
deliberately drops the live row in favour of, which could never have been
there to find.
* Name the try in the Gantt tooltip
The try sat inside a branch keyed on start_date, but the chart reports
min_start_date, so
it never reached a bar's tooltip at all. It is read off the shape a caller
passes now,
rather than from one of the branches below it.
Shown on a first try too. Clearing a loop pass regenerates the later ones as
fresh
executions that start again at try 1, so two neighbouring bars can both be a
first try and
the number is what tells them apart.
* Drop the iteration label from the mapped task instances header
The counts beside it are the task's across the whole loop, so naming one
pass above them
said that pass had every expansion of every pass. This page lists the
expansions of a
mapped task, which is an aggregate view, and the pass it belongs to is
already named in
the breadcrumb directly above. Narrowing to a single pass is what the task
instances
table and its loop filter are for.
* Regenerate the UI client and reformat after the review changes
The spec changed when the execution route went back to its shape on main and
the iteration
sort was withdrawn, but the generated client still carried both, so the UI
lint hook
regenerated it and failed on the difference.
Two Python files were left unformatted by the same round of edits.
* Fix the type errors the UI static check compiles for
The checked-in tsconfig does not typecheck src; the hook builds its own
project extending
tsconfig.app.json, which turns on noUnusedLocals among other things, so
running tsc
against the root config said clean while the hook did not.
Two unused locals left behind by earlier edits, a key typed as a plain
string where its
neighbours are cast, and a tooltip reading try_number off a union whose grid
summary
member does not carry one -- lifting it out of the start_date branch lost
the narrowing
that had been doing that work.
---------
(cherry picked from commit 42945d62b1f6ecbb8255b306c9becf7c2900dfef)
<!-- SPDX-License-Identifier: Apache-2.0
https://www.apache.org/licenses/LICENSE-2.0 -->
<!--
Thank you for contributing!
Please provide above a brief description of the changes made in this pull
request.
Write a good git commit message following this guide:
https://chris.beams.io/posts/git-commit/
Please make sure that your code changes are covered with tests.
And in case of new features or big changes remember to adjust the
documentation.
For user-facing UI changes, please attach before/after screenshots (or a
short
screen recording) so reviewers can assess the visual impact.
Feel free to ping (in general) for the review if you do not see reaction for
a few days
(72 Hours is the minimum reaction time you can expect from volunteers) - we
sometimes miss notifications.
In case of an existing issue, reference it using one of the following:
* closes: #ISSUE
* related: #ISSUE
-->
---
##### Was generative AI tooling used to co-author this PR?
<!--
If generative AI tooling has been used in the process of authoring this PR,
please
change below checkbox to `[X]` followed by the name of the tool, uncomment
the "Generated-by".
-->
- [ ] Yes (please specify the tool below)
<!--
Generated-by: [Tool Name] following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
-->
---
* Read the **[Pull Request
Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)**
for more information. Note: commit author/co-author name and email in commits
become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals))
is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License
Policy](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment:
`{pr_number}.significant.rst`, in
[airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
You can add this file in a follow-up commit after the PR is created so you
know the PR number.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
