bbovenzi commented on code in PR #44604: URL: https://github.com/apache/airflow/pull/44604#discussion_r1869704569
########## airflow/ui/src/pages/DagsList/Dag/Tasks/TaskCard.tsx: ########## @@ -0,0 +1,84 @@ +/*! + * 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 { + Heading, + VStack, + HStack, + Box, + SimpleGrid, + Text, +} from "@chakra-ui/react"; + +import type { + TaskResponse, + TaskInstanceResponse, +} from "openapi/requests/types.gen"; +import { Status } from "src/components/ui"; + +import { TaskRecentRuns } from "./TaskRecentRuns.tsx"; + +type Props = { + readonly task: TaskResponse; + readonly taskInstances: Array<TaskInstanceResponse>; +}; + +export const TaskCard = ({ task, taskInstances }: Props) => ( + <Box + borderColor="border.emphasized" + borderRadius={8} + borderWidth={1} + overflow="hidden" + > + <Text bg="bg.info" color="fg.info" p={2}> + {task.task_display_name ?? task.task_id} + {task.is_mapped ? "[]" : undefined} + </Text> + <SimpleGrid columns={4} gap={4} height={20} px={3} py={2}> + <VStack align="flex-start" gap={1}> + <Heading color="fg.muted" fontSize="xs"> + Operator + </Heading> + <Text fontSize="sm">{task.operator_name}</Text> + </VStack> + <VStack align="flex-start" gap={1}> + <Heading color="fg.muted" fontSize="xs"> + Trigger Rule + </Heading> + <Text fontSize="sm">{task.trigger_rule}</Text> + </VStack> + <VStack align="flex-start" gap={1}> + <Heading color="fg.muted" fontSize="xs"> + Last Run Review Comment: ```suggestion Last Instance ``` ########## airflow/ui/src/pages/DagsList/Dag/Tasks/TaskCard.tsx: ########## @@ -0,0 +1,84 @@ +/*! + * 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 { + Heading, + VStack, + HStack, + Box, + SimpleGrid, + Text, +} from "@chakra-ui/react"; + +import type { + TaskResponse, + TaskInstanceResponse, +} from "openapi/requests/types.gen"; +import { Status } from "src/components/ui"; + +import { TaskRecentRuns } from "./TaskRecentRuns.tsx"; + +type Props = { + readonly task: TaskResponse; + readonly taskInstances: Array<TaskInstanceResponse>; +}; + +export const TaskCard = ({ task, taskInstances }: Props) => ( + <Box + borderColor="border.emphasized" + borderRadius={8} + borderWidth={1} + overflow="hidden" + > + <Text bg="bg.info" color="fg.info" p={2}> + {task.task_display_name ?? task.task_id} + {task.is_mapped ? "[]" : undefined} + </Text> + <SimpleGrid columns={4} gap={4} height={20} px={3} py={2}> + <VStack align="flex-start" gap={1}> + <Heading color="fg.muted" fontSize="xs"> + Operator + </Heading> + <Text fontSize="sm">{task.operator_name}</Text> + </VStack> + <VStack align="flex-start" gap={1}> + <Heading color="fg.muted" fontSize="xs"> + Trigger Rule + </Heading> + <Text fontSize="sm">{task.trigger_rule}</Text> + </VStack> + <VStack align="flex-start" gap={1}> + <Heading color="fg.muted" fontSize="xs"> + Last Run + </Heading> + {taskInstances[0] ? ( + <HStack fontSize="sm"> + <Text> {taskInstances[0].logical_date} </Text> + {taskInstances[0].state === null ? undefined : ( + <Status state={taskInstances[0].state}> + {taskInstances[0].state} + </Status> + )} + </HStack> Review Comment: Let's wrap this in a tooltip like we do with the latest dag run? Include the dag run id and state, and the TI start/end/try_number ########## airflow/ui/src/pages/DagsList/Dag/Tasks/TaskCard.tsx: ########## @@ -0,0 +1,84 @@ +/*! + * 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 { + Heading, + VStack, + HStack, + Box, + SimpleGrid, + Text, +} from "@chakra-ui/react"; + +import type { + TaskResponse, + TaskInstanceResponse, +} from "openapi/requests/types.gen"; +import { Status } from "src/components/ui"; + +import { TaskRecentRuns } from "./TaskRecentRuns.tsx"; + +type Props = { + readonly task: TaskResponse; + readonly taskInstances: Array<TaskInstanceResponse>; +}; + +export const TaskCard = ({ task, taskInstances }: Props) => ( + <Box + borderColor="border.emphasized" + borderRadius={8} + borderWidth={1} + overflow="hidden" + > + <Text bg="bg.info" color="fg.info" p={2}> Review Comment: ```suggestion <Text color="fg.info" fontWeight="bold" p={2}> ``` ########## airflow/ui/src/pages/DagsList/Dag/Tasks/TaskRecentRuns.tsx: ########## @@ -0,0 +1,97 @@ +/*! + * 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 { Box, Text } from "@chakra-ui/react"; +import { Flex } from "@chakra-ui/react"; +import dayjs from "dayjs"; +import duration from "dayjs/plugin/duration"; + +import type { TaskInstanceResponse } from "openapi/requests/types.gen"; +import Time from "src/components/Time"; +import { Tooltip } from "src/components/ui"; +import { stateColor } from "src/utils/stateColor"; + +dayjs.extend(duration); + +const BAR_HEIGHT = 60; + +export const TaskRecentRuns = ({ + taskInstances, +}: { + readonly taskInstances: Array<TaskInstanceResponse>; +}) => { + if (!taskInstances.length) { + return undefined; + } + + const taskInstancesWithDuration = taskInstances.map((taskInstance) => ({ + ...taskInstance, + duration: + dayjs + .duration(dayjs(taskInstance.end_date).diff(taskInstance.start_date)) + .asSeconds() || 0, + })); + + const max = Math.max.apply( + undefined, + taskInstancesWithDuration.map((taskInstance) => taskInstance.duration), + ); + + return ( + <Flex alignItems="flex-end" flexDirection="row-reverse"> + {taskInstancesWithDuration.map((taskInstance) => + taskInstance.state === null ? undefined : ( + <Tooltip + content={ + <Box> + <Text>State: {taskInstance.state}</Text> + <Text> + Start Date: <Time datetime={taskInstance.start_date} /> + </Text> + <Text> + End Date: <Time datetime={taskInstance.end_date} /> + </Text> + <Text>Try Number: {taskInstance.try_number}</Text> Review Comment: Let's only show try_number if it's greater than 1 ########## airflow/ui/src/pages/DagsList/Dag/Tasks/TaskRecentRuns.tsx: ########## @@ -0,0 +1,97 @@ +/*! + * 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 { Box, Text } from "@chakra-ui/react"; +import { Flex } from "@chakra-ui/react"; +import dayjs from "dayjs"; +import duration from "dayjs/plugin/duration"; + +import type { TaskInstanceResponse } from "openapi/requests/types.gen"; +import Time from "src/components/Time"; +import { Tooltip } from "src/components/ui"; +import { stateColor } from "src/utils/stateColor"; + +dayjs.extend(duration); + +const BAR_HEIGHT = 60; + +export const TaskRecentRuns = ({ + taskInstances, +}: { + readonly taskInstances: Array<TaskInstanceResponse>; +}) => { + if (!taskInstances.length) { + return undefined; + } + + const taskInstancesWithDuration = taskInstances.map((taskInstance) => ({ + ...taskInstance, + duration: + dayjs + .duration(dayjs(taskInstance.end_date).diff(taskInstance.start_date)) + .asSeconds() || 0, + })); + + const max = Math.max.apply( + undefined, + taskInstancesWithDuration.map((taskInstance) => taskInstance.duration), + ); + + return ( + <Flex alignItems="flex-end" flexDirection="row-reverse"> + {taskInstancesWithDuration.map((taskInstance) => + taskInstance.state === null ? undefined : ( + <Tooltip + content={ + <Box> + <Text>State: {taskInstance.state}</Text> Review Comment: Let's include the dag run id and state too. We could probably make this tooltip into its own `TaskInstanceTooltip` and use it for the "Latest Instance" card field too. ########## airflow/ui/src/pages/DagsList/Dag/Tasks/Tasks.tsx: ########## @@ -0,0 +1,113 @@ +/*! + * 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 { Heading, Skeleton, Box } from "@chakra-ui/react"; +import { useParams } from "react-router-dom"; + +import { + useTaskServiceGetTasks, + useTaskInstanceServiceGetTaskInstances, + useDagsServiceRecentDagRuns, +} from "openapi/queries"; +import type { + TaskResponse, + TaskInstanceResponse, +} from "openapi/requests/types.gen"; +import { DataTable } from "src/components/DataTable"; +import type { CardDef } from "src/components/DataTable/types"; +import { ErrorAlert } from "src/components/ErrorAlert"; + +import { TaskCard } from "./TaskCard"; + +const cardDef = ( + taskInstances?: Array<TaskInstanceResponse>, +): CardDef<TaskResponse> => ({ + card: ({ row }) => ( + <TaskCard + task={row} + taskInstances={ + taskInstances + ? taskInstances.filter( + (instance: TaskInstanceResponse) => + instance.task_id === row.task_id, + ) + : [] + } + /> + ), + meta: { + customSkeleton: <Skeleton height="120px" width="100%" />, + }, +}); + +export const Tasks = () => { + const { dagId } = useParams(); + const { + data, + error: TasksError, + isFetching, + isLoading, + } = useTaskServiceGetTasks({ + dagId: dagId ?? "", + }); + + // TODO: Replace dagIdPattern with dagId once supported for better matching + const { data: runsData } = useDagsServiceRecentDagRuns( + { dagIdPattern: dagId ?? "", dagRunsLimit: 14 }, + undefined, + { + enabled: Boolean(dagId), + }, + ); + + const runs = + runsData?.dags.find((dagWithRuns) => dagWithRuns.dag_id === dagId) + ?.latest_dag_runs ?? []; + + // TODO: Revisit this endpoint since only 100 task instances are returned and + // only duration is calculated with other attributes unused. + const { data: TaskInstancesResponse } = + useTaskInstanceServiceGetTaskInstances( + { + dagId: dagId ?? "", + dagRunId: "~", + logicalDateGte: runs.at(-1)?.logical_date ?? "", + }, + undefined, + { enabled: Boolean(runs[0]?.dag_run_id) }, + ); + + return ( + <Box> + <ErrorAlert error={TasksError} /> + <Heading my={1} size="md"> + {data ? data.total_entries : 0} Tasks Review Comment: Let's use `pluralize()` here ########## airflow/ui/src/pages/DagsList/Dag/Tasks/TaskCard.tsx: ########## @@ -0,0 +1,84 @@ +/*! + * 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 { + Heading, + VStack, + HStack, + Box, + SimpleGrid, + Text, +} from "@chakra-ui/react"; + +import type { + TaskResponse, + TaskInstanceResponse, +} from "openapi/requests/types.gen"; +import { Status } from "src/components/ui"; + +import { TaskRecentRuns } from "./TaskRecentRuns.tsx"; + +type Props = { + readonly task: TaskResponse; + readonly taskInstances: Array<TaskInstanceResponse>; +}; + +export const TaskCard = ({ task, taskInstances }: Props) => ( + <Box + borderColor="border.emphasized" + borderRadius={8} + borderWidth={1} + overflow="hidden" + > + <Text bg="bg.info" color="fg.info" p={2}> + {task.task_display_name ?? task.task_id} + {task.is_mapped ? "[]" : undefined} + </Text> + <SimpleGrid columns={4} gap={4} height={20} px={3} py={2}> + <VStack align="flex-start" gap={1}> + <Heading color="fg.muted" fontSize="xs"> + Operator + </Heading> + <Text fontSize="sm">{task.operator_name}</Text> + </VStack> + <VStack align="flex-start" gap={1}> + <Heading color="fg.muted" fontSize="xs"> + Trigger Rule + </Heading> + <Text fontSize="sm">{task.trigger_rule}</Text> + </VStack> + <VStack align="flex-start" gap={1}> + <Heading color="fg.muted" fontSize="xs"> + Last Run + </Heading> + {taskInstances[0] ? ( + <HStack fontSize="sm"> + <Text> {taskInstances[0].logical_date} </Text> Review Comment: This should be inside of a `<Time>` component. And I think we want to use start_date here. ########## airflow/ui/src/pages/DagsList/Dag/Tasks/Tasks.tsx: ########## @@ -0,0 +1,113 @@ +/*! + * 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 { Heading, Skeleton, Box } from "@chakra-ui/react"; +import { useParams } from "react-router-dom"; + +import { + useTaskServiceGetTasks, + useTaskInstanceServiceGetTaskInstances, + useDagsServiceRecentDagRuns, +} from "openapi/queries"; +import type { + TaskResponse, + TaskInstanceResponse, +} from "openapi/requests/types.gen"; +import { DataTable } from "src/components/DataTable"; +import type { CardDef } from "src/components/DataTable/types"; +import { ErrorAlert } from "src/components/ErrorAlert"; + +import { TaskCard } from "./TaskCard"; + +const cardDef = ( + taskInstances?: Array<TaskInstanceResponse>, +): CardDef<TaskResponse> => ({ + card: ({ row }) => ( + <TaskCard + task={row} + taskInstances={ + taskInstances + ? taskInstances.filter( + (instance: TaskInstanceResponse) => + instance.task_id === row.task_id, + ) + : [] + } + /> + ), + meta: { + customSkeleton: <Skeleton height="120px" width="100%" />, + }, +}); + +export const Tasks = () => { + const { dagId } = useParams(); + const { + data, + error: TasksError, + isFetching, + isLoading, + } = useTaskServiceGetTasks({ + dagId: dagId ?? "", + }); + + // TODO: Replace dagIdPattern with dagId once supported for better matching + const { data: runsData } = useDagsServiceRecentDagRuns( + { dagIdPattern: dagId ?? "", dagRunsLimit: 14 }, + undefined, + { + enabled: Boolean(dagId), + }, + ); + + const runs = + runsData?.dags.find((dagWithRuns) => dagWithRuns.dag_id === dagId) + ?.latest_dag_runs ?? []; + + // TODO: Revisit this endpoint since only 100 task instances are returned and + // only duration is calculated with other attributes unused. Review Comment: Yes good point. We'll need a custom ui endpoint in the API to properly power this. At the bare minimum the public endpoint should have a `task_display_name_pattern` param. ########## airflow/ui/src/router.tsx: ########## @@ -26,8 +26,10 @@ import { Overview } from "src/pages/DagsList/Dag/Overview"; import { Runs } from "src/pages/DagsList/Dag/Runs"; import { Run } from "src/pages/DagsList/Run"; import { Dashboard } from "src/pages/Dashboard"; -import { ErrorPage } from "src/pages/Error"; -import { Events } from "src/pages/Events"; + +import { Tasks } from "./pages/DagsList/Dag/Tasks"; +import { ErrorPage } from "./pages/Error"; +import { Events } from "./pages/Events"; Review Comment: ```suggestion import { Tasks } from "src/pages/DagsList/Dag/Tasks"; import { ErrorPage } from "src/pages/Error"; import { Events } from "src/pages/Events"; ``` I might open a PR to add more aliases so that we can just call `pages/Events` -- 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]
