wiasliaw commented on code in PR #73555:
URL: https://github.com/apache/airflow/pull/73555#discussion_r4120691044
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/gantt.py:
##########
@@ -95,14 +107,24 @@ def get_gantt_data(
combined = union_all(current_tis, history_tis).subquery()
query = select(combined).order_by(combined.c.task_id,
combined.c.try_number)
+ # Rebind the filters to the union subquery columns so they apply to both
TI and TIH rows.
Review Comment:
Fixed. Widened the annotation to `ColumnElement | InstrumentedAttribute`
rather than bare `ColumnElement`.
`InstrumentedAttribute` is not a `ColumnElement` subclass, so plain
`ColumnElement` would fail mypy at the existing direct-construction call sites
that pass `Mapped[...]` model attributes (e.g. `RangeFilter(...,
attribute=DagRun.run_after)` in `task_instances.py` and `dag_run.py`). The
union follows the existing precedent in `common/parameters/base.py`
(`BaseParam.attribute`). Both `# type: ignore[arg-type]` lines in `gantt.py`
are now gone.
---
Drafted-by: Claude Code (Fable 5); reviewed by @wiasliaw before posting
##########
airflow-core/src/airflow/ui/src/layouts/Details/PanelButtons.tsx:
##########
@@ -298,7 +298,7 @@ export const PanelButtons = ({
{dagView !== "graph" && (
<Flex justifyContent="space-between" mt={2}>
- <GridFilters />
+ <GridFilters showGanttDateFilters={dagView === "gantt"} />
Review Comment:
I looked into clearing them, but I think keeping the params matches the
existing behavior of this layout, and they are inert outside the Gantt view:
- Within the Details layout, only `Gantt.tsx` reads `start_date_gte/lte` and
`end_date_gte/lte`. The Grid view queries use `run_after_*`, so the leftover
params never filter Grid data invisibly.
- They also can't leak to other pages that read the same keys (DagRuns,
Jobs): `NavTabs.tsx` navigates with `to={{ pathname }}` only, which drops the
query string.
- The Graph task filters (`GRAPH_OPERATOR`, `GRAPH_TASK_STATE`, … in
`GraphTaskFilters.tsx`) already work exactly this way — pills hidden when you
switch to Grid, params kept, filters restored when you switch back. Clearing
only the Gantt date params would make the two filter sets behave differently.
- `DetailsLayout.tsx` also auto-switches gantt → grid when there is no
`runId`; clearing on view change would wipe the user's filters on that
programmatic switch, not just on an explicit toggle.
If we want view switches to reset view-specific filters, I'd rather do that
consistently for the `GRAPH_*` params too in a follow-up. Happy to change it
here if you feel strongly.
---
Drafted-by: Claude Code (Fable 5); reviewed by @wiasliaw 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]
