You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "bbovenzi (via GitHub)" <gi...@apache.org> on 2023/06/08 21:14:28 UTC
[GitHub] [airflow] bbovenzi opened a new pull request, #31806: New gantt tab
bbovenzi opened a new pull request, #31806:
URL: https://github.com/apache/airflow/pull/31806
New Gantt Chart in grid new tabs
Closes: [#19940](https://github.com/apache/airflow/issues/19940)
Sync scrolling with grid:
Support task groups and sync with grid:
Sync task/run selection with grid:
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bbovenzi commented on pull request #31806: New gantt tab
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on PR #31806:
URL: https://github.com/apache/airflow/pull/31806#issuecomment-1647490534
> Looks great! Tested locally and seems nice. 🎉
>
> I just have issues scrolling the gantt. (scroll bars are not working for me), also I have vertical scroll when there is plenty of space vertically to render the Gantt chart. (scrollbar are unresponsive, both with the scroll wheel and by click/drag on it, tested on chrome and firefox on ubuntu)
Figured out the scroll focus issue on the gantt chart itself. I also change the `checkScrollPosition` from `onSelect` to a timeout, so if the scroll of the grid/gantt are not synced, it should rectify itself.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] marclamberti commented on pull request #31806: New gantt tab
Posted by "marclamberti (via GitHub)" <gi...@apache.org>.
marclamberti commented on PR #31806:
URL: https://github.com/apache/airflow/pull/31806#issuecomment-1621318684
my eyes 🤩🤩🤩🤩🤩
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a diff in pull request #31806: New gantt tab
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #31806:
URL: https://github.com/apache/airflow/pull/31806#discussion_r1275554577
##########
airflow/www/static/js/dag/details/gantt/GanttTooltip.tsx:
##########
@@ -0,0 +1,82 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React from "react";
+import { Box, Text } from "@chakra-ui/react";
+import { getDuration, formatDuration } from "src/datetime_utils";
+import Time from "src/components/Time";
+import type { Task, TaskInstance } from "src/types";
+
+interface Props {
+ instance: TaskInstance;
+ task: Task;
+}
+
+const GanttTooltip = ({ task, instance }: Props) => {
+ const hasQueuedDttm = !!instance?.queuedDttm;
+ const isGroup = !!task.children;
+ const isMappedOrGroupSummary = isGroup || task.isMapped;
+
+ // Calculate durations in ms
+ const taskDuration = getDuration(instance?.startDate, instance?.endDate);
+ const queuedDuration = hasQueuedDttm
+ ? getDuration(instance?.queuedDttm, instance?.startDate)
+ : 0;
+ return (
+ <Box>
+ <Text>
+ Task{isGroup ? " Group" : ""}: {task.label}
+ </Text>
+ <br />
+ {hasQueuedDttm && (
+ <Text>
+ {isMappedOrGroupSummary && "Total "}Queued Duration:{" "}
+ {formatDuration(queuedDuration)}
+ </Text>
+ )}
+ <Text>
+ {isMappedOrGroupSummary && "Total "}Run Duration:{" "}
+ {formatDuration(taskDuration)}
+ </Text>
+ <br />
+ {hasQueuedDttm && (
+ <Text>
+ {isMappedOrGroupSummary && "Earliest "}Queued At:{" "}
+ <Time dateTime={instance?.queuedDttm} />
+ </Text>
+ )}
+ <Text>
+ {isMappedOrGroupSummary && "Earliest "}Start:{" "}
+ <Time dateTime={instance?.startDate} />
+ </Text>
+ <Text>
+ {instance?.endDate ? (
+ <>
+ {isMappedOrGroupSummary && "Latest "}End:{" "}
+ <Time dateTime={instance?.endDate} />
+ </>
+ ) : (
+ "Ongoing"
Review Comment:
We should consider having nothing instead. This makes for a weird combo for queued tasks:
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on pull request #31806: New gantt tab
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #31806:
URL: https://github.com/apache/airflow/pull/31806#issuecomment-1584940138
this looks really good!
What happens when task retry and it will show the timeline only for last attempt right?
I wonder if it's doable to allow users choose to see all retries (as requested in https://github.com/apache/airflow/issues/17487 )
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bbovenzi commented on pull request #31806: New gantt tab
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on PR #31806:
URL: https://github.com/apache/airflow/pull/31806#issuecomment-1643037822
Ready for review although I'm still working on improving the scroll behavior.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #31806: New gantt tab
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #31806:
URL: https://github.com/apache/airflow/pull/31806#issuecomment-1583355783
Whoa. I don't have skills to review it but the screenshots looks like #protm
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #31806: New gantt tab
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #31806:
URL: https://github.com/apache/airflow/pull/31806#issuecomment-1652398371
> In this case I think we could even remove the original Gantt chart entirely
+1
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on a diff in pull request #31806: New gantt tab
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #31806:
URL: https://github.com/apache/airflow/pull/31806#discussion_r1271417526
##########
airflow/www/static/js/dag/details/gantt/Chart.tsx:
##########
@@ -0,0 +1,190 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React from "react";
+import { Box, Text, Tooltip, Flex } from "@chakra-ui/react";
+import useSelection from "src/dag/useSelection";
+import { getDuration, formatDuration } from "src/datetime_utils";
+import { SimpleStatus } from "src/dag/StatusBox";
+import { useContainerRef } from "src/context/containerRef";
+import { hoverDelay } from "src/utils";
+import Time from "src/components/Time";
+import type { DagRun, Task } from "src/types";
+
+interface Props {
+ ganttWidth?: number;
+ openGroupIds: string[];
+ dagRun: DagRun;
+ tasks: Task[];
+ checkScrollPosition: () => void;
+}
+
+const Chart = ({
+ ganttWidth = 500,
+ openGroupIds,
+ tasks,
+ dagRun,
+ checkScrollPosition,
+}: Props) => {
+ const {
+ selected: { runId, taskId },
+ onSelect,
+ } = useSelection();
+ const containerRef = useContainerRef();
+
+ const runDuration = getDuration(dagRun?.startDate, dagRun?.endDate);
+
+ return (
+ <div>
+ {tasks?.map((task) => {
+ const instance = task.instances.find((ti) => ti.runId === runId);
+ const isSelected = taskId === instance?.taskId;
+ const hasQueuedDttm = !!instance?.queuedDttm;
+ const isOpen = openGroupIds.includes(task.label || "");
+ const isGroup = !!task.children;
+
+ // Calculate durations in ms
+ const taskDuration = getDuration(
+ instance?.startDate,
+ instance?.endDate
+ );
+ const queuedDuration = hasQueuedDttm
+ ? getDuration(instance?.queuedDttm, instance?.startDate)
+ : 0;
+ const taskStartOffset = getDuration(
+ dagRun.startDate,
+ instance?.queuedDttm || instance?.startDate
+ );
+
+ // Percent of each duration vs the overall dag run
+ const taskDurationPercent = taskDuration / runDuration;
+ const taskStartOffsetPercent = taskStartOffset / runDuration;
+ const queuedDurationPercent = queuedDuration / runDuration;
+
+ // Calculate the pixel width of the queued and task bars and the position in the graph
+ // Min width should be 5px
+ let width = ganttWidth * taskDurationPercent;
+ if (width < 5) width = 5;
+ let queuedWidth = hasQueuedDttm
+ ? ganttWidth * queuedDurationPercent
+ : 0;
+ if (hasQueuedDttm && queuedWidth < 5) queuedWidth = 5;
+ const offsetMargin = taskStartOffsetPercent * ganttWidth;
+
+ return (
+ <div key={`gantt-${task.id}`}>
+ <Box
+ py="4px"
+ borderBottomWidth={1}
+ borderBottomColor={isGroup && isOpen ? "gray.400" : "gray.200"}
+ bg={isSelected ? "blue.100" : "inherit"}
+ >
+ {instance ? (
+ <Tooltip
+ label={
+ <Box>
+ <Text>
+ Task{isGroup ? " Group" : ""}: {task.label}
Review Comment:
Can we extract a component from this. I find it hard to read to have all this code inlined in the jsx.
##########
airflow/www/static/js/dag/details/index.tsx:
##########
@@ -262,6 +295,16 @@ const Details = ({ openGroupIds, onToggleGroups, hoveredTaskState }: Props) => {
hoveredTaskState={hoveredTaskState}
/>
</TabPanel>
+ {run && (
+ <TabPanel p={0} height="100%">
+ <Gantt
+ openGroupIds={openGroupIds}
+ // hoveredTaskState={hoveredTaskState}
Review Comment:
We can remove if unnecessary, tasks are not hovering in the Gantt tab, maybe 'nice to have' but not necessary IMHO
##########
airflow/www/static/js/dag/details/index.tsx:
##########
@@ -134,6 +153,12 @@ const Details = ({ openGroupIds, onToggleGroups, hoveredTaskState }: Props) => {
[setSearchParams, searchParams, showLogs, showMappedTasks, taskId]
);
+ useEffect(() => {
+ if ((!taskId || isGroup) && tabIndex > 3) {
+ onChangeTab(1);
+ }
+ }, [runId, taskId, tabIndex, isGroup, onChangeTab]);
Review Comment:
Is `runId` necessary here ?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on pull request #31806: New gantt tab
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #31806:
URL: https://github.com/apache/airflow/pull/31806#issuecomment-1652069108
> In this case I think we could even remove the original Gantt chart entirely
+1
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bbovenzi commented on a diff in pull request #31806: New gantt tab
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #31806:
URL: https://github.com/apache/airflow/pull/31806#discussion_r1271930553
##########
airflow/www/static/js/dag/details/index.tsx:
##########
@@ -134,6 +153,12 @@ const Details = ({ openGroupIds, onToggleGroups, hoveredTaskState }: Props) => {
[setSearchParams, searchParams, showLogs, showMappedTasks, taskId]
);
+ useEffect(() => {
+ if ((!taskId || isGroup) && tabIndex > 3) {
+ onChangeTab(1);
+ }
+ }, [runId, taskId, tabIndex, isGroup, onChangeTab]);
Review Comment:
Updated. There was an existing bug with resetting the tabs.
##########
airflow/www/static/js/dag/details/index.tsx:
##########
@@ -262,6 +295,16 @@ const Details = ({ openGroupIds, onToggleGroups, hoveredTaskState }: Props) => {
hoveredTaskState={hoveredTaskState}
/>
</TabPanel>
+ {run && (
+ <TabPanel p={0} height="100%">
+ <Gantt
+ openGroupIds={openGroupIds}
+ // hoveredTaskState={hoveredTaskState}
Review Comment:
Removed
##########
airflow/www/static/js/dag/details/gantt/Chart.tsx:
##########
@@ -0,0 +1,190 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React from "react";
+import { Box, Text, Tooltip, Flex } from "@chakra-ui/react";
+import useSelection from "src/dag/useSelection";
+import { getDuration, formatDuration } from "src/datetime_utils";
+import { SimpleStatus } from "src/dag/StatusBox";
+import { useContainerRef } from "src/context/containerRef";
+import { hoverDelay } from "src/utils";
+import Time from "src/components/Time";
+import type { DagRun, Task } from "src/types";
+
+interface Props {
+ ganttWidth?: number;
+ openGroupIds: string[];
+ dagRun: DagRun;
+ tasks: Task[];
+ checkScrollPosition: () => void;
+}
+
+const Chart = ({
+ ganttWidth = 500,
+ openGroupIds,
+ tasks,
+ dagRun,
+ checkScrollPosition,
+}: Props) => {
+ const {
+ selected: { runId, taskId },
+ onSelect,
+ } = useSelection();
+ const containerRef = useContainerRef();
+
+ const runDuration = getDuration(dagRun?.startDate, dagRun?.endDate);
+
+ return (
+ <div>
+ {tasks?.map((task) => {
+ const instance = task.instances.find((ti) => ti.runId === runId);
+ const isSelected = taskId === instance?.taskId;
+ const hasQueuedDttm = !!instance?.queuedDttm;
+ const isOpen = openGroupIds.includes(task.label || "");
+ const isGroup = !!task.children;
+
+ // Calculate durations in ms
+ const taskDuration = getDuration(
+ instance?.startDate,
+ instance?.endDate
+ );
+ const queuedDuration = hasQueuedDttm
+ ? getDuration(instance?.queuedDttm, instance?.startDate)
+ : 0;
+ const taskStartOffset = getDuration(
+ dagRun.startDate,
+ instance?.queuedDttm || instance?.startDate
+ );
+
+ // Percent of each duration vs the overall dag run
+ const taskDurationPercent = taskDuration / runDuration;
+ const taskStartOffsetPercent = taskStartOffset / runDuration;
+ const queuedDurationPercent = queuedDuration / runDuration;
+
+ // Calculate the pixel width of the queued and task bars and the position in the graph
+ // Min width should be 5px
+ let width = ganttWidth * taskDurationPercent;
+ if (width < 5) width = 5;
+ let queuedWidth = hasQueuedDttm
+ ? ganttWidth * queuedDurationPercent
+ : 0;
+ if (hasQueuedDttm && queuedWidth < 5) queuedWidth = 5;
+ const offsetMargin = taskStartOffsetPercent * ganttWidth;
+
+ return (
+ <div key={`gantt-${task.id}`}>
+ <Box
+ py="4px"
+ borderBottomWidth={1}
+ borderBottomColor={isGroup && isOpen ? "gray.400" : "gray.200"}
+ bg={isSelected ? "blue.100" : "inherit"}
+ >
+ {instance ? (
+ <Tooltip
+ label={
+ <Box>
+ <Text>
+ Task{isGroup ? " Group" : ""}: {task.label}
Review Comment:
Cleaned up!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bbovenzi merged pull request #31806: New gantt tab
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi merged PR #31806:
URL: https://github.com/apache/airflow/pull/31806
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jens-scheffler-bosch commented on pull request #31806: New gantt tab
Posted by "jens-scheffler-bosch (via GitHub)" <gi...@apache.org>.
jens-scheffler-bosch commented on PR #31806:
URL: https://github.com/apache/airflow/pull/31806#issuecomment-1636881615
WOW and only 440 LoC! This smells like PR of the month is coming :-D
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org