Copilot commented on code in PR #64387:
URL: https://github.com/apache/airflow/pull/64387#discussion_r3066491302
##########
airflow-core/src/airflow/ui/src/layouts/Details/Gantt/Gantt.tsx:
##########
@@ -143,9 +143,15 @@ export const Gantt = ({ dagRunState, limit, runAfterGte,
runAfterLte, runType, t
const gridTiSummaries = summariesByRunId.get(runId);
const summariesLoading = Boolean(runId && selectedRun &&
!summariesByRunId.has(runId));
- // Single fetch for all Gantt data (individual task tries)
+ // startDate and endDate are sent to the backend as query parameters.
+ // The server filters the data — NOT the browser.
const { data: ganttData, isLoading: ganttLoading } =
useGanttServiceGetGanttData(
- { dagId, runId },
+ {
+ dagId,
+ runId,
+ startDate: filterStartDate ? `${filterStartDate}T00:00:00Z` : undefined,
+ endDate: filterEndDate ? `${filterEndDate}T23:59:59Z` : undefined,
+ },
Review Comment:
`useGanttServiceGetGanttData` is being called with `startDate`/`endDate`,
but the generated OpenAPI client currently only accepts `{ dagId, runId }` (see
`airflow-core/src/airflow/ui/openapi-gen/queries/queries.ts` and
`.../requests/services.gen.ts`). Either regenerate the OpenAPI UI client after
adding these query params to the API spec, or avoid passing unsupported params.
##########
airflow-core/package-lock.json:
##########
@@ -0,0 +1,6 @@
+{
+ "name": "airflow-core",
+ "lockfileVersion": 3,
+ "requires": true,
+ "packages": {}
+}
Review Comment:
`airflow-core/package-lock.json` is being added, but there’s no
`package.json` under `airflow-core/`, and the lockfile is essentially empty
(`"packages": {}`). This looks like an accidental artifact and can confuse Node
tooling/CI; please remove it (or add the corresponding `package.json` + explain
why this lockfile is needed).
```suggestion
```
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/gantt.py:
##########
@@ -59,8 +62,16 @@ def get_gantt_data(
dag_id: str,
run_id: str,
session: SessionDep,
+ start_date_range: Annotated[datetime | None, Query(alias="start_date")] =
None,
+ end_date_range: Annotated[datetime | None, Query(alias="end_date")] = None,
) -> GanttResponse:
"""Get all task instance tries for Gantt chart."""
+ if start_date_range and end_date_range and start_date_range >
end_date_range:
+ raise HTTPException(
+ status.HTTP_400_BAD_REQUEST,
+ "start_date cannot be greater than end_date",
+ )
Review Comment:
This endpoint now raises HTTP 400 for invalid `start_date`/`end_date`
ranges, but the route decorator’s
`responses=create_openapi_http_exception_doc(...)` still only documents 404.
Please update the documented responses accordingly so the OpenAPI spec/UI
client generation includes the 400 case.
##########
airflow-core/src/airflow/ui/src/layouts/Details/Gantt/Gantt.tsx:
##########
@@ -81,8 +79,12 @@ const CHART_PADDING = 36;
const CHART_ROW_HEIGHT = 20;
const MIN_BAR_WIDTH = 10;
-export const Gantt = ({ dagRunState, limit, runAfterGte, runAfterLte, runType,
triggeringUser }: Props) => {
+export const Gantt = ({ dagRunState, limit, runType, triggeringUser }: Props)
=> {
const { dagId = "", groupId: selectedGroupId, runId = "", taskId:
selectedTaskId } = useParams();
Review Comment:
`Gantt` Props no longer include `runAfterGte`/`runAfterLte`, but callers
still pass them (e.g. `DetailsLayout.tsx` around lines 201-208). This will fail
TypeScript compilation unless the call sites are updated or the props are kept.
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/gantt.py:
##########
@@ -97,7 +133,7 @@ def get_gantt_data(
if not results:
raise HTTPException(
status.HTTP_404_NOT_FOUND,
- f"No task instances for dag_id={dag_id} run_id={run_id}",
+ detail="DAG or DagRun not found",
)
Review Comment:
With the new date-range filters, an empty result set can mean “no tasks in
this range” for a valid DagRun, but the endpoint currently returns 404 when
`results` is empty. This will incorrectly error for valid runs when the
selected range excludes all tasks; consider validating DagRun existence
separately and returning 200 with an empty `task_instances` list for the
no-matches case.
--
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]
