kartik00052 commented on PR #73805:
URL: https://github.com/apache/airflow/pull/73805#issuecomment-5857701766
I went looking for a fix for #73803 and landed on the same three branches,
so this is
not a new observation — just a different source for the number. The
`AssetSchedule.tsx`
part of this PR looks right to me.
One suggestion on where the count comes from: the value you are adding as
`scheduling_asset_count` is, in the non-partitioned case, already in the
payload.
`redact_asset_expression` blanks the identifying fields of an unreadable
asset but
deliberately keeps its slot in the tree, "so the shape of the schedule is
still
honest" (`api_fastapi/common/asset_expression.py:52`). So counting `asset`
leaves of
`asset_expression` yields the number of assets the Dag is scheduled on, for
every
caller, with no extra round trip:
```ts
const scheduleTotal = countAssetLeaves(nextRun?.asset_expression) ??
nextRunEvents.length;
```
That is equal to the pre-#72862 `nextRunEvents.length` for two reasons worth
checking against your reading of the query:
- Both derive from `DagScheduleAssetReference`, so they enumerate the same
set.
- `alias` and `asset_ref` leaves are not assets and never produce `events`
rows, so
counting only `asset` leaves keeps the two numbers equal rather than
drifting.
And the fallback covers the one case where the expression is not usable:
`MaybeAssetExpression` degrades an unrecognized legacy shape to `None`
rather than
failing validation (`datamodels/common.py:182`), so the UI sees `null`, the
leaf count
returns `null`, and `events.length` keeps today's behavior.
The trade-off against your version:
- No second query. `next_run_assets` runs once per Dag card, and this
endpoint is
already the heaviest on the Dags-list render — your note puts
`assert_queries_count`
at 5 → 6, and the existing comment at `assets.py:174` shows the query
count on this
route is already something the codebase deliberately guards.
- No `datamodels`/`_private_ui.yaml`/`openapi-gen` churn and no backend
tests; the
change is four files in the UI.
- It does not depend on #72862 being merged first to be correct, which also
means it
can be reviewed as a standalone fix.
What it does not cover, same as yours: the partitioned label total still sums
`required_count` over the readable events only, so that stays a real gap.
And if you
prefer the server to be the authority on this number, your field is the more
defensible long-term shape — the UI is then not inferring semantics from a
redacted
tree.
Happy to share the patch and the tests if you want to try it; I have no
attachment
to this approach either way.
---
Drafted-by: Claude Code Sonnet; reviewed by @kartik00052 before posting
--
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]
