jscheffl commented on code in PR #45039:
URL: https://github.com/apache/airflow/pull/45039#discussion_r1890631109
##########
airflow/ui/src/components/ClearRun/ClearRunButton.tsx:
##########
Review Comment:
Should we name it to make clear it is not a "Run" but the whole "DagRun"? (I
was looking for the clear of task instance run but the button is for the DagRun
only) - I assume a task clear button will also be added as a follow-up PR
##########
airflow/ui/package.json:
##########
@@ -27,6 +28,8 @@
"@visx/group": "^3.12.0",
"@visx/shape": "^3.12.0",
"@xyflow/react": "^12.3.5",
+ "accordion": "^3.0.2",
Review Comment:
Interesting... why is accordion being added here? An accordion is already
using in the TriggerDAGForm - can we consolidate and not use two libs in two
areas?
##########
airflow/ui/src/components/ClearRun/ClearRunDialog.tsx:
##########
@@ -0,0 +1,140 @@
+/*!
+ * 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 { Flex, Heading, HStack, Spacer, VStack } from "@chakra-ui/react";
+import { FiPlay, FiXSquare } from "react-icons/fi";
+
+import type {
+ DAGRunClearBody,
+ TaskInstanceCollectionResponse,
+} from "openapi/requests/types.gen";
+import { Button, Dialog } from "src/components/ui";
+
+import SegmentedControl from "../ui/SegmentedControl";
+import ClearRunTasksAccordion from "./ClearRunTaskAccordion";
+
+type Props = {
+ readonly affectedTasks: TaskInstanceCollectionResponse;
+ readonly dagId: string;
+ readonly dagRunId: string;
+ readonly isPending: boolean;
+ readonly mutate: ({
+ dagId,
+ dagRunId,
+ requestBody,
+ }: {
+ dagId: string;
+ dagRunId: string;
+ requestBody: DAGRunClearBody;
+ }) => void;
+ readonly onClose: () => void;
+ readonly onlyFailed: boolean;
+ readonly open: boolean;
+ readonly setOnlyFailed: (value: boolean) => void;
+};
+
+const ClearRunDialog = ({
+ affectedTasks,
+ dagId,
+ dagRunId,
+ isPending,
+ mutate,
+ onClose,
+ onlyFailed,
+ open,
+ setOnlyFailed,
+}: Props) => {
+ const onChange = (value: string) => {
+ switch (value) {
+ case "existing_tasks":
+ setOnlyFailed(false);
+ mutate({
+ dagId,
+ dagRunId,
+ requestBody: { dry_run: true, only_failed: false },
+ });
+ break;
+ case "only_failed":
+ setOnlyFailed(true);
+ mutate({
+ dagId,
+ dagRunId,
+ requestBody: { dry_run: true, only_failed: true },
+ });
+ break;
+ default:
+ // TODO: Handle this `new_tasks` case
+ break;
+ }
+ };
+
+ return (
+ <Dialog.Root onOpenChange={onClose} open={open} size="xl">
+ <Dialog.Content backdrop>
+ <Dialog.Header>
+ <VStack align="start" gap={4}>
+ <Heading size="xl">Clear DagRun - {dagRunId} </Heading>
+ </VStack>
+ </Dialog.Header>
+
+ <Dialog.CloseTrigger />
+
+ <Dialog.Body width="full">
+ <Flex justifyContent="center">
+ <SegmentedControl
+ mb={3}
+ onValueChange={onChange}
+ options={[
+ { label: "Clear existing tasks", value: "existing_tasks" },
+ { label: "Clear only failed tasks", value: "only_failed" },
+ {
+ disabled: true,
+ label: "Queued up new tasks",
+ value: "new_tasks",
+ },
+ ]}
+ value={onlyFailed ? "only_failed" : "existing_tasks"}
+ />
+ </Flex>
+ <ClearRunTasksAccordion affectedTasks={affectedTasks} />
+ <HStack mt={3} w="full">
+ <Button colorPalette="gray" onClick={onClose}>
+ <FiXSquare /> Cancel
+ </Button>
+ <Spacer />
+ <Button
+ colorPalette="blue"
+ loading={isPending}
+ onClick={() => {
+ mutate({
+ dagId,
+ dagRunId,
+ requestBody: { dry_run: false, only_failed: onlyFailed },
+ });
+ }}
+ >
+ <FiPlay /> Confirm
Review Comment:
The play button is a bit mis-leading, can we use the same icon for the
confirmation like the button that opens the dialog? Otherwise it is confusing
as it smells like a new DAG run
##########
airflow/ui/src/components/ClearRun/ClearRunDialog.tsx:
##########
@@ -0,0 +1,140 @@
+/*!
+ * 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 { Flex, Heading, HStack, Spacer, VStack } from "@chakra-ui/react";
+import { FiPlay, FiXSquare } from "react-icons/fi";
+
+import type {
+ DAGRunClearBody,
+ TaskInstanceCollectionResponse,
+} from "openapi/requests/types.gen";
+import { Button, Dialog } from "src/components/ui";
+
+import SegmentedControl from "../ui/SegmentedControl";
+import ClearRunTasksAccordion from "./ClearRunTaskAccordion";
+
+type Props = {
+ readonly affectedTasks: TaskInstanceCollectionResponse;
+ readonly dagId: string;
+ readonly dagRunId: string;
+ readonly isPending: boolean;
+ readonly mutate: ({
+ dagId,
+ dagRunId,
+ requestBody,
+ }: {
+ dagId: string;
+ dagRunId: string;
+ requestBody: DAGRunClearBody;
+ }) => void;
+ readonly onClose: () => void;
+ readonly onlyFailed: boolean;
+ readonly open: boolean;
+ readonly setOnlyFailed: (value: boolean) => void;
+};
+
+const ClearRunDialog = ({
+ affectedTasks,
+ dagId,
+ dagRunId,
+ isPending,
+ mutate,
+ onClose,
+ onlyFailed,
+ open,
+ setOnlyFailed,
+}: Props) => {
+ const onChange = (value: string) => {
+ switch (value) {
+ case "existing_tasks":
+ setOnlyFailed(false);
+ mutate({
+ dagId,
+ dagRunId,
+ requestBody: { dry_run: true, only_failed: false },
+ });
+ break;
+ case "only_failed":
+ setOnlyFailed(true);
+ mutate({
+ dagId,
+ dagRunId,
+ requestBody: { dry_run: true, only_failed: true },
+ });
+ break;
+ default:
+ // TODO: Handle this `new_tasks` case
+ break;
+ }
+ };
+
+ return (
+ <Dialog.Root onOpenChange={onClose} open={open} size="xl">
+ <Dialog.Content backdrop>
+ <Dialog.Header>
+ <VStack align="start" gap={4}>
+ <Heading size="xl">Clear DagRun - {dagRunId} </Heading>
+ </VStack>
+ </Dialog.Header>
+
+ <Dialog.CloseTrigger />
+
+ <Dialog.Body width="full">
+ <Flex justifyContent="center">
+ <SegmentedControl
+ mb={3}
+ onValueChange={onChange}
+ options={[
+ { label: "Clear existing tasks", value: "existing_tasks" },
+ { label: "Clear only failed tasks", value: "only_failed" },
+ {
+ disabled: true,
+ label: "Queued up new tasks",
+ value: "new_tasks",
+ },
+ ]}
+ value={onlyFailed ? "only_failed" : "existing_tasks"}
+ />
+ </Flex>
+ <ClearRunTasksAccordion affectedTasks={affectedTasks} />
Review Comment:
From the visual it is not clear that this is an accordion. I was just
cloicking on it as accident and then it expanded. Can you make it similar to
"Advanced Options" in Trigger DAG Run?
##########
airflow/ui/src/components/ClearRun/ClearRunDialog.tsx:
##########
@@ -0,0 +1,140 @@
+/*!
+ * 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 { Flex, Heading, HStack, Spacer, VStack } from "@chakra-ui/react";
+import { FiPlay, FiXSquare } from "react-icons/fi";
+
+import type {
+ DAGRunClearBody,
+ TaskInstanceCollectionResponse,
+} from "openapi/requests/types.gen";
+import { Button, Dialog } from "src/components/ui";
+
+import SegmentedControl from "../ui/SegmentedControl";
+import ClearRunTasksAccordion from "./ClearRunTaskAccordion";
+
+type Props = {
+ readonly affectedTasks: TaskInstanceCollectionResponse;
+ readonly dagId: string;
+ readonly dagRunId: string;
+ readonly isPending: boolean;
+ readonly mutate: ({
+ dagId,
+ dagRunId,
+ requestBody,
+ }: {
+ dagId: string;
+ dagRunId: string;
+ requestBody: DAGRunClearBody;
+ }) => void;
+ readonly onClose: () => void;
+ readonly onlyFailed: boolean;
+ readonly open: boolean;
+ readonly setOnlyFailed: (value: boolean) => void;
+};
+
+const ClearRunDialog = ({
+ affectedTasks,
+ dagId,
+ dagRunId,
+ isPending,
+ mutate,
+ onClose,
+ onlyFailed,
+ open,
+ setOnlyFailed,
+}: Props) => {
+ const onChange = (value: string) => {
+ switch (value) {
+ case "existing_tasks":
+ setOnlyFailed(false);
+ mutate({
+ dagId,
+ dagRunId,
+ requestBody: { dry_run: true, only_failed: false },
+ });
+ break;
+ case "only_failed":
+ setOnlyFailed(true);
+ mutate({
+ dagId,
+ dagRunId,
+ requestBody: { dry_run: true, only_failed: true },
+ });
+ break;
+ default:
+ // TODO: Handle this `new_tasks` case
+ break;
+ }
+ };
+
+ return (
+ <Dialog.Root onOpenChange={onClose} open={open} size="xl">
+ <Dialog.Content backdrop>
+ <Dialog.Header>
+ <VStack align="start" gap={4}>
+ <Heading size="xl">Clear DagRun - {dagRunId} </Heading>
+ </VStack>
+ </Dialog.Header>
+
+ <Dialog.CloseTrigger />
+
+ <Dialog.Body width="full">
+ <Flex justifyContent="center">
+ <SegmentedControl
+ mb={3}
+ onValueChange={onChange}
+ options={[
+ { label: "Clear existing tasks", value: "existing_tasks" },
+ { label: "Clear only failed tasks", value: "only_failed" },
+ {
+ disabled: true,
+ label: "Queued up new tasks",
+ value: "new_tasks",
+ },
+ ]}
+ value={onlyFailed ? "only_failed" : "existing_tasks"}
+ />
+ </Flex>
+ <ClearRunTasksAccordion affectedTasks={affectedTasks} />
+ <HStack mt={3} w="full">
+ <Button colorPalette="gray" onClick={onClose}>
+ <FiXSquare /> Cancel
Review Comment:
On the Trigger DAG Run modal we have no cancel, you need to use the [x] on
the top right - we should also make this consistent from look and feel. I think
we don't need a cancel button :-D
##########
airflow/ui/src/components/ClearRun/ClearRunTaskAccordion.tsx:
##########
@@ -0,0 +1,107 @@
+/*!
+ * 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 { Link } from "@chakra-ui/react";
+import type { ColumnDef } from "@tanstack/react-table";
+import { Link as RouterLink } from "react-router-dom";
+
+import type {
+ TaskInstanceCollectionResponse,
+ TaskInstanceResponse,
+} from "openapi/requests/types.gen";
+import { DataTable } from "src/components/DataTable";
+import { Status } from "src/components/ui";
+
+import { Accordion } from "../ui";
+
+const columns: Array<ColumnDef<TaskInstanceResponse>> = [
+ {
+ accessorKey: "task_display_name",
+ cell: ({ row: { original } }) => (
+ <Link asChild color="fg.info" fontWeight="bold">
+ <RouterLink
+
to={`/dags/${original.dag_id}/runs/${original.dag_run_id}/tasks/${original.task_id}${original.map_index
> -1 ? `?map_index=${original.map_index}` : ""}`}
+ >
+ {original.task_display_name}
+ </RouterLink>
+ </Link>
+ ),
+ enableSorting: false,
+ header: "Task ID",
+ },
+ {
+ accessorKey: "state",
+ cell: ({
+ row: {
+ original: { state },
+ },
+ }) => <Status state={state}>{state}</Status>,
+ enableSorting: false,
+ header: () => "State",
+ },
+ {
+ accessorKey: "map_index",
+ enableSorting: false,
+ header: "Map Index",
+ },
+
+ {
+ accessorKey: "dag_run_id",
+ enableSorting: false,
+ header: "Run Id",
+ },
+];
+
+type Props = {
+ readonly affectedTasks?: TaskInstanceCollectionResponse;
+};
+
+// Table is in memory, pagination and sorting are disabled.
+// TODO: Make a front-end only unconnected table component with client side
ordering and pagination
+const ClearRunTasksAccordion = ({ affectedTasks }: Props) => (
+ <Accordion.Root collapsible>
+ <Accordion.Item key="tasks" value="tasks">
+ <Accordion.ItemTrigger>
+ <Text fontWeight="bold">
+ Affected Tasks: {affectedTasks?.total_entries ?? 0}
+ </Text>
+ </Accordion.ItemTrigger>
+ <Accordion.ItemContent>
+ <Box maxH="400px" overflowY="scroll">
+ <DataTable
Review Comment:
Interesting - if no items are found (DAG run was success and you select
failed tasks only) then it is displaying "no task instances found" but below
that text still the table headers are shown.
--
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]
