-
Notifications
You must be signed in to change notification settings - Fork 17.3k
Add date range filter to Gantt view #64387
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a2bcbe9
a71aa5b
28846e9
e46373c
58250f4
c57cbca
df15b67
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,10 @@ | |
|
|
||
| from __future__ import annotations | ||
|
|
||
| from fastapi import Depends, HTTPException, status | ||
| from datetime import datetime | ||
| from typing import Annotated | ||
|
|
||
| from fastapi import Depends, HTTPException, Query, status | ||
| from sqlalchemy import or_, select, union_all | ||
|
|
||
| from airflow.api_fastapi.auth.managers.models.resource_details import DagAccessEntity | ||
|
|
@@ -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", | ||
| ) | ||
|
Comment on lines
68
to
+73
|
||
|
|
||
| # Exclude mapped tasks (use grid summaries) and UP_FOR_RETRY (already in history) | ||
| current_tis = select( | ||
| TaskInstance.task_id.label("task_id"), | ||
|
|
@@ -74,6 +85,16 @@ def get_gantt_data( | |
| TaskInstance.run_id == run_id, | ||
| TaskInstance.map_index == -1, | ||
| or_(TaskInstance.state != TaskInstanceState.UP_FOR_RETRY, TaskInstance.state.is_(None)), | ||
| *( | ||
| [or_(TaskInstance.end_date >= start_date_range, TaskInstance.end_date.is_(None))] | ||
| if start_date_range is not None | ||
| else [] | ||
| ), | ||
| *( | ||
| [or_(TaskInstance.start_date <= end_date_range, TaskInstance.start_date.is_(None))] | ||
| if end_date_range is not None | ||
| else [] | ||
| ), | ||
| ) | ||
|
|
||
| history_tis = select( | ||
|
|
@@ -87,6 +108,21 @@ def get_gantt_data( | |
| TaskInstanceHistory.dag_id == dag_id, | ||
| TaskInstanceHistory.run_id == run_id, | ||
| TaskInstanceHistory.map_index == -1, | ||
| *( | ||
| [or_(TaskInstanceHistory.end_date >= start_date_range, TaskInstanceHistory.end_date.is_(None))] | ||
| if start_date_range is not None | ||
| else [] | ||
| ), | ||
| *( | ||
| [ | ||
| or_( | ||
| TaskInstanceHistory.start_date <= end_date_range, | ||
| TaskInstanceHistory.start_date.is_(None), | ||
| ) | ||
| ] | ||
| if end_date_range is not None | ||
| else [] | ||
| ), | ||
| ) | ||
|
|
||
| combined = union_all(current_tis, history_tis).subquery() | ||
|
|
@@ -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", | ||
| ) | ||
|
|
||
| task_instances = [ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ | |
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
| import { Box, useToken } from "@chakra-ui/react"; | ||
| import { Box, Field, Input, useToken } from "@chakra-ui/react"; | ||
| import { | ||
| Chart as ChartJS, | ||
| CategoryScale, | ||
|
|
@@ -33,7 +33,7 @@ import { | |
| import "chart.js/auto"; | ||
| import "chartjs-adapter-dayjs-4/dist/chartjs-adapter-dayjs-4.esm"; | ||
| import annotationPlugin from "chartjs-plugin-annotation"; | ||
| import { useDeferredValue } from "react"; | ||
| import { useDeferredValue, useState } from "react"; | ||
| import { Bar } from "react-chartjs-2"; | ||
| import { useTranslation } from "react-i18next"; | ||
| import { useParams, useNavigate, useLocation, useSearchParams } from "react-router-dom"; | ||
|
|
@@ -71,8 +71,6 @@ ChartJS.register( | |
| type Props = { | ||
| readonly dagRunState?: DagRunState | undefined; | ||
| readonly limit: number; | ||
| readonly runAfterGte?: string | undefined; | ||
| readonly runAfterLte?: string | undefined; | ||
| readonly runType?: DagRunType | undefined; | ||
| readonly triggeringUser?: string | undefined; | ||
| }; | ||
|
|
@@ -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(); | ||
|
Comment on lines
71
to
83
|
||
| const [filterStartDate, setFilterStartDate] = useState(""); | ||
| const [filterEndDate, setFilterEndDate] = useState(""); | ||
| const [dateError, setDateError] = useState(""); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need state management for the error. It can just be a variable:
|
||
|
|
||
| const [searchParams] = useSearchParams(); | ||
| const { openGroupIds } = useOpenGroups(); | ||
| const deferredOpenGroupIds = useDeferredValue(openGroupIds); | ||
|
|
@@ -116,8 +118,6 @@ export const Gantt = ({ dagRunState, limit, runAfterGte, runAfterLte, runType, t | |
| const { data: gridRuns, isLoading: runsLoading } = useGridRuns({ | ||
| dagRunState, | ||
| limit, | ||
| runAfterGte, | ||
| runAfterLte, | ||
| runType, | ||
| triggeringUser, | ||
| }); | ||
|
|
@@ -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, | ||
|
Comment on lines
+152
to
+153
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The dates should include the full time. Most dags run in less than a day, so if the most precise you can be is day, then this filter is entirely useless. |
||
| }, | ||
|
Comment on lines
+146
to
+154
|
||
| undefined, | ||
| { | ||
| enabled: Boolean(dagId) && Boolean(runId) && Boolean(selectedRun), | ||
|
|
@@ -212,7 +218,7 @@ export const Gantt = ({ dagRunState, limit, runAfterGte, runAfterLte, runType, t | |
| translate, | ||
| }); | ||
|
|
||
| if (runId === "" || (!isLoading && !selectedRun)) { | ||
| if (runId === "") { | ||
| return undefined; | ||
| } | ||
|
|
||
|
|
@@ -228,21 +234,74 @@ export const Gantt = ({ dagRunState, limit, runAfterGte, runAfterLte, runType, t | |
| }; | ||
|
|
||
| return ( | ||
| <Box | ||
| height={`${fixedHeight}px`} | ||
| minW="250px" | ||
| ml={-2} | ||
| mt={`${GRID_BODY_OFFSET_PX}px`} | ||
| onMouseLeave={handleChartMouseLeave} | ||
| w="100%" | ||
| > | ||
| <Bar | ||
| data={chartData} | ||
| options={chartOptions} | ||
| style={{ | ||
| paddingTop: flatNodes.length === 1 ? 15 : 1.5, | ||
| }} | ||
| /> | ||
| </Box> | ||
| <> | ||
| {/* Date range inputs — values are sent to backend as query params, no client-side filtering */} | ||
| <Box alignItems="flex-start" display="flex" gap="4" mb="4"> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Making this a box in the gantt chart messes up all of the positioning of the element. |
||
| <Field.Root invalid={Boolean(dateError)} maxW="200px"> | ||
| <Field.Label color="fg.muted" fontSize="xs"> | ||
| {translate("startDate")} | ||
| </Field.Label> | ||
| <Input | ||
| fontSize="sm" | ||
| fontWeight="medium" | ||
| onChange={(e) => { | ||
| const value = e.target.value; | ||
|
|
||
| if (value && filterEndDate && value > filterEndDate) { | ||
| setDateError(translate("startDateAfterEndDate")); | ||
| return; | ||
| } | ||
| setDateError(""); | ||
| setFilterStartDate(value); | ||
| }} | ||
| placeholder="YYYY-MM-DD" | ||
| size="sm" | ||
| type="date" | ||
| value={filterStartDate} | ||
| /> | ||
| </Field.Root> | ||
|
|
||
| <Field.Root invalid={Boolean(dateError)} maxW="200px"> | ||
| <Field.Label color="fg.muted" fontSize="xs"> | ||
| {translate("endDate")} | ||
| </Field.Label> | ||
| <Input | ||
| fontSize="sm" | ||
| fontWeight="medium" | ||
| onChange={(e) => { | ||
| const value = e.target.value; | ||
|
|
||
| if (value && filterStartDate && value < filterStartDate) { | ||
| setDateError(translate("endDateBeforeStartDate")); | ||
| return; | ||
| } | ||
| setDateError(""); | ||
| setFilterEndDate(value); | ||
| }} | ||
|
Smitaambiger marked this conversation as resolved.
|
||
| placeholder="YYYY-MM-DD" | ||
| size="sm" | ||
| type="date" | ||
| value={filterEndDate} | ||
| /> | ||
| {dateError ? <Field.ErrorText fontSize="xs">{dateError}</Field.ErrorText> : undefined} | ||
| </Field.Root> | ||
|
Comment on lines
+240
to
+287
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already have a date range filter component. Let's use it. |
||
| </Box> | ||
| <Box | ||
| height={`${fixedHeight}px`} | ||
| minW="250px" | ||
| ml={-2} | ||
| mt={`${GRID_BODY_OFFSET_PX}px`} | ||
| onMouseLeave={handleChartMouseLeave} | ||
| w="100%" | ||
| > | ||
| <Bar | ||
| data={chartData} | ||
| options={chartOptions} | ||
| style={{ | ||
| paddingTop: flatNodes.length === 1 ? 15 : 1.5, | ||
| }} | ||
| /> | ||
| </Box> | ||
| </> | ||
| ); | ||
| }; | ||
|
Smitaambiger marked this conversation as resolved.
|
Uh oh!
There was an error while loading. Please reload this page.