jscheffl commented on code in PR #43367: URL: https://github.com/apache/airflow/pull/43367#discussion_r1815699746
########## airflow/ui/src/components/DataTable/TriggerDAG.ts: ########## @@ -0,0 +1,30 @@ +/*! + * 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. + */ + +type DagParams = { + configJson: string; Review Comment: That should be the params dict, not a string? ########## airflow/ui/src/components/DataTable/TriggerDAG.ts: ########## @@ -0,0 +1,30 @@ +/*! + * 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. + */ + +type DagParams = { + configJson: string; + dagId: string; + logicalDate: string; + runId?: string; Review Comment: Was passed as empty string, not `null` when not filling the field. So the `?` might not be needed. ########## airflow/ui/src/components/DataTable/TriggerDAGForm.tsx: ########## @@ -0,0 +1,188 @@ +/*! + * 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 { + FormControl, + FormLabel, + Input, + VStack, + ModalCloseButton, + Button, + ModalFooter, + Box, + Text, + Spacer, + HStack, + Collapse, +} from "@chakra-ui/react"; +import { autocompletion } from "@codemirror/autocomplete"; +import { json } from "@codemirror/lang-json"; +import { oneDark } from "@codemirror/theme-one-dark"; +import CodeMirror, { lineNumbers } from "@uiw/react-codemirror"; +import React, { useState, useEffect } from "react"; + +type DagParams = { + configJson: string; + dagId: string; + logicalDate: string; + runId?: string; +}; + +type TriggerDAGFormProps = { + dagParams: DagParams; + onClose: () => void; + onTrigger: () => void; + setDagParams: React.Dispatch<React.SetStateAction<DagParams>>; +}; + +const TriggerDAGForm: React.FC<TriggerDAGFormProps> = ({ + dagParams, + onTrigger, + setDagParams, +}) => { + const [showDetails, setShowDetails] = useState(false); + + useEffect(() => { + try { + const prettyJson = JSON.stringify( + JSON.parse(dagParams.configJson), + undefined, + 2, + ); + + setDagParams((prev) => ({ ...prev, configJson: prettyJson })); + } catch { + // Invalid JSON handling + } + }, [dagParams.configJson, setDagParams]); + + const handleChange = ( + ele: React.ChangeEvent<HTMLInputElement | HTMLTextAreaElement>, + ) => { + const { name, value } = ele.target; + + setDagParams((prev) => ({ ...prev, [name]: value })); + }; + + const handleReset = () => { + setDagParams({ + configJson: "{}", + dagId: dagParams.dagId, + logicalDate: "", + runId: "", + }); + }; + + const handleJsonChange = (value: string) => { + setDagParams((prev) => ({ ...prev, configJson: value })); + }; + + const isValidJson = () => { + try { + JSON.parse(dagParams.configJson); + + return true; + } catch { + return false; + } + }; + + return ( + <> + <ModalCloseButton /> + + <VStack align="stretch" p={5} spacing={2}> + <Button + onClick={() => setShowDetails(!showDetails)} + variant="outline" + width="full" + > + {showDetails ? "Hide Advance Options" : "Show Advance Options"} + </Button> + + <Collapse in={showDetails}> + <VStack align="stretch" spacing={3}> + <FormControl> + <FormLabel size="sm">Logical date</FormLabel> + <Input Review Comment: I am not sure and am not really an expert but @bbovenzi and @pierrejeambrun proposed to me to use react-hook-form as toolkit for the trigger form. Don't know if this is proposed instead of a plain form. ########## airflow/ui/src/components/DataTable/TriggerDAGModal.tsx: ########## @@ -0,0 +1,98 @@ +/*! + * 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 { + Modal, + ModalOverlay, + ModalContent, + useDisclosure, + Box, + Text, + Heading, + VStack, +} from "@chakra-ui/react"; +import React, { useState } from "react"; +import { FiPlay } from "react-icons/fi"; + +import { TriggerDAG } from "./TriggerDAG"; +import TriggerDAGForm from "./TriggerDAGForm"; + +type DagParams = { + configJson: string; + dagId: string; + logicalDate: string; + runId?: string; +}; + +type TriggerDAGModalProps = { + dagDisplayName: string; + dagId: string; +}; + +const TriggerDAGModal: React.FC<TriggerDAGModalProps> = ({ + dagDisplayName, + dagId, +}) => { + const { isOpen, onClose, onOpen } = useDisclosure(); + const [dagParams, setDagParams] = useState<DagParams>({ + configJson: "{}", + dagId, + logicalDate: "", + runId: "", + }); + + const handleTrigger = () => { + TriggerDAG(dagParams); + onClose(); + }; + + return ( + <Box> + <Box alignSelf="center" cursor="pointer" onClick={onOpen}> + <FiPlay /> + </Box> + + <Modal isOpen={isOpen} onClose={onClose} size="xl"> + <ModalOverlay /> + <ModalContent> + <VStack align="start" p={5} spacing={2}> + <Heading size="md">Trigger DAG</Heading> + + <Box> + <Heading mb={1} size="sm"> + {dagDisplayName} + </Heading> + <Text color="gray.500" fontSize="xs"> + DAG ID: {dagId} + </Text> Review Comment: I think displaying the DGA ID only makes sense if the display name is different than the DAG ID. Otherwise good idea to have this as well! ########## airflow/ui/src/components/DataTable/TriggerDAGForm.tsx: ########## @@ -0,0 +1,188 @@ +/*! + * 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 { + FormControl, + FormLabel, + Input, + VStack, + ModalCloseButton, + Button, + ModalFooter, + Box, + Text, + Spacer, + HStack, + Collapse, +} from "@chakra-ui/react"; +import { autocompletion } from "@codemirror/autocomplete"; +import { json } from "@codemirror/lang-json"; +import { oneDark } from "@codemirror/theme-one-dark"; +import CodeMirror, { lineNumbers } from "@uiw/react-codemirror"; +import React, { useState, useEffect } from "react"; + +type DagParams = { + configJson: string; + dagId: string; + logicalDate: string; + runId?: string; +}; + +type TriggerDAGFormProps = { + dagParams: DagParams; + onClose: () => void; + onTrigger: () => void; + setDagParams: React.Dispatch<React.SetStateAction<DagParams>>; +}; + +const TriggerDAGForm: React.FC<TriggerDAGFormProps> = ({ + dagParams, + onTrigger, + setDagParams, +}) => { + const [showDetails, setShowDetails] = useState(false); + + useEffect(() => { + try { + const prettyJson = JSON.stringify( + JSON.parse(dagParams.configJson), + undefined, + 2, + ); + + setDagParams((prev) => ({ ...prev, configJson: prettyJson })); + } catch { + // Invalid JSON handling + } + }, [dagParams.configJson, setDagParams]); + + const handleChange = ( + ele: React.ChangeEvent<HTMLInputElement | HTMLTextAreaElement>, + ) => { + const { name, value } = ele.target; + + setDagParams((prev) => ({ ...prev, [name]: value })); + }; + + const handleReset = () => { + setDagParams({ + configJson: "{}", Review Comment: If reset: Should it not be reset to the default params defined by the DAG? ########## airflow/ui/src/components/DataTable/TriggerDAGForm.tsx: ########## @@ -0,0 +1,188 @@ +/*! + * 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 { + FormControl, + FormLabel, + Input, + VStack, + ModalCloseButton, + Button, + ModalFooter, + Box, + Text, + Spacer, + HStack, + Collapse, +} from "@chakra-ui/react"; +import { autocompletion } from "@codemirror/autocomplete"; +import { json } from "@codemirror/lang-json"; +import { oneDark } from "@codemirror/theme-one-dark"; +import CodeMirror, { lineNumbers } from "@uiw/react-codemirror"; +import React, { useState, useEffect } from "react"; + +type DagParams = { + configJson: string; + dagId: string; + logicalDate: string; + runId?: string; +}; + +type TriggerDAGFormProps = { + dagParams: DagParams; + onClose: () => void; + onTrigger: () => void; + setDagParams: React.Dispatch<React.SetStateAction<DagParams>>; +}; + +const TriggerDAGForm: React.FC<TriggerDAGFormProps> = ({ + dagParams, + onTrigger, + setDagParams, +}) => { + const [showDetails, setShowDetails] = useState(false); + + useEffect(() => { + try { + const prettyJson = JSON.stringify( + JSON.parse(dagParams.configJson), + undefined, + 2, + ); + + setDagParams((prev) => ({ ...prev, configJson: prettyJson })); + } catch { + // Invalid JSON handling + } + }, [dagParams.configJson, setDagParams]); + + const handleChange = ( + ele: React.ChangeEvent<HTMLInputElement | HTMLTextAreaElement>, + ) => { + const { name, value } = ele.target; + + setDagParams((prev) => ({ ...prev, [name]: value })); + }; + + const handleReset = () => { + setDagParams({ + configJson: "{}", + dagId: dagParams.dagId, + logicalDate: "", + runId: "", + }); + }; + + const handleJsonChange = (value: string) => { + setDagParams((prev) => ({ ...prev, configJson: value })); + }; + + const isValidJson = () => { + try { + JSON.parse(dagParams.configJson); + + return true; + } catch { + return false; + } + }; + + return ( + <> + <ModalCloseButton /> + + <VStack align="stretch" p={5} spacing={2}> + <Button + onClick={() => setShowDetails(!showDetails)} + variant="outline" + width="full" + > + {showDetails ? "Hide Advance Options" : "Show Advance Options"} + </Button> + + <Collapse in={showDetails}> + <VStack align="stretch" spacing={3}> + <FormControl> + <FormLabel size="sm">Logical date</FormLabel> + <Input + name="logicalDate" + onChange={handleChange} + placeholder="yyyy-mm-ddThh:mm" + size="sm" + type="datetime-local" + value={dagParams.logicalDate} + /> + </FormControl> + + <FormControl> + <FormLabel size="sm">Run ID (Optional)</FormLabel> + <Input + name="runId" + onChange={handleChange} + placeholder="Autogenerated if left empty" Review Comment: Previous UI used: ```suggestion placeholder="Run id, optional - will be generated if not provided" ``` ########## airflow/ui/src/components/DataTable/TriggerDAGForm.tsx: ########## @@ -0,0 +1,188 @@ +/*! + * 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 { + FormControl, + FormLabel, + Input, + VStack, + ModalCloseButton, + Button, + ModalFooter, + Box, + Text, + Spacer, + HStack, + Collapse, +} from "@chakra-ui/react"; +import { autocompletion } from "@codemirror/autocomplete"; +import { json } from "@codemirror/lang-json"; +import { oneDark } from "@codemirror/theme-one-dark"; +import CodeMirror, { lineNumbers } from "@uiw/react-codemirror"; +import React, { useState, useEffect } from "react"; + +type DagParams = { + configJson: string; + dagId: string; + logicalDate: string; + runId?: string; +}; + +type TriggerDAGFormProps = { + dagParams: DagParams; + onClose: () => void; + onTrigger: () => void; + setDagParams: React.Dispatch<React.SetStateAction<DagParams>>; +}; + +const TriggerDAGForm: React.FC<TriggerDAGFormProps> = ({ + dagParams, + onTrigger, + setDagParams, +}) => { + const [showDetails, setShowDetails] = useState(false); + + useEffect(() => { + try { + const prettyJson = JSON.stringify( + JSON.parse(dagParams.configJson), + undefined, + 2, + ); + + setDagParams((prev) => ({ ...prev, configJson: prettyJson })); + } catch { + // Invalid JSON handling + } + }, [dagParams.configJson, setDagParams]); + + const handleChange = ( + ele: React.ChangeEvent<HTMLInputElement | HTMLTextAreaElement>, + ) => { + const { name, value } = ele.target; + + setDagParams((prev) => ({ ...prev, [name]: value })); + }; + + const handleReset = () => { + setDagParams({ + configJson: "{}", + dagId: dagParams.dagId, + logicalDate: "", + runId: "", + }); + }; + + const handleJsonChange = (value: string) => { + setDagParams((prev) => ({ ...prev, configJson: value })); + }; + + const isValidJson = () => { + try { + JSON.parse(dagParams.configJson); + + return true; + } catch { + return false; + } + }; + + return ( + <> + <ModalCloseButton /> + + <VStack align="stretch" p={5} spacing={2}> + <Button + onClick={() => setShowDetails(!showDetails)} + variant="outline" + width="full" + > + {showDetails ? "Hide Advance Options" : "Show Advance Options"} + </Button> + + <Collapse in={showDetails}> + <VStack align="stretch" spacing={3}> + <FormControl> + <FormLabel size="sm">Logical date</FormLabel> + <Input + name="logicalDate" + onChange={handleChange} + placeholder="yyyy-mm-ddThh:mm" + size="sm" + type="datetime-local" + value={dagParams.logicalDate} + /> + </FormControl> + + <FormControl> + <FormLabel size="sm">Run ID (Optional)</FormLabel> + <Input + name="runId" + onChange={handleChange} + placeholder="Autogenerated if left empty" + size="sm" + value={dagParams.runId} + /> + </FormControl> + + <FormControl> + <FormLabel size="sm">Configuration JSON</FormLabel> + <CodeMirror + extensions={[json(), autocompletion(), lineNumbers()]} + height="200px" + onChange={handleJsonChange} + style={{ + border: "1px solid #CBD5E0", + borderRadius: "8px", + outline: "none", + padding: "2px", + }} + theme={oneDark} + value={dagParams.configJson} + /> + {isValidJson() ? undefined : ( + <Box color="red.500" mt={2}> + <Text fontSize="sm">Invalid JSON format</Text> + </Box> + )} + </FormControl> + </VStack> + </Collapse> + </VStack> + + <ModalFooter> + <HStack w="full"> + <Button colorScheme="red" onClick={handleReset}> + Reset Review Comment: I'd propose to have a "Cancel" button, not a Reset. ########## airflow/ui/src/components/DataTable/TriggerDAGModal.tsx: ########## @@ -0,0 +1,98 @@ +/*! + * 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 { + Modal, + ModalOverlay, + ModalContent, + useDisclosure, + Box, + Text, + Heading, + VStack, +} from "@chakra-ui/react"; +import React, { useState } from "react"; +import { FiPlay } from "react-icons/fi"; + +import { TriggerDAG } from "./TriggerDAG"; +import TriggerDAGForm from "./TriggerDAGForm"; + +type DagParams = { + configJson: string; + dagId: string; + logicalDate: string; + runId?: string; +}; + +type TriggerDAGModalProps = { + dagDisplayName: string; + dagId: string; +}; + +const TriggerDAGModal: React.FC<TriggerDAGModalProps> = ({ + dagDisplayName, + dagId, +}) => { + const { isOpen, onClose, onOpen } = useDisclosure(); + const [dagParams, setDagParams] = useState<DagParams>({ + configJson: "{}", + dagId, + logicalDate: "", + runId: "", + }); + + const handleTrigger = () => { + TriggerDAG(dagParams); + onClose(); + }; + + return ( + <Box> + <Box alignSelf="center" cursor="pointer" onClick={onOpen}> + <FiPlay /> + </Box> + + <Modal isOpen={isOpen} onClose={onClose} size="xl"> + <ModalOverlay /> + <ModalContent> + <VStack align="start" p={5} spacing={2}> + <Heading size="md">Trigger DAG</Heading> + + <Box> + <Heading mb={1} size="sm"> + {dagDisplayName} + </Heading> + <Text color="gray.500" fontSize="xs"> + DAG ID: {dagId} + </Text> + </Box> + </VStack> + + <TriggerDAGForm + dagParams={dagParams} + onClose={onClose} Review Comment: Can you reset the form content when the modal is opened? Else if you trigger once and open again, the values from previous trigger are still there. ########## airflow/ui/package.json: ########## @@ -18,10 +18,14 @@ "dependencies": { "@chakra-ui/anatomy": "^2.2.2", "@chakra-ui/react": "^2.8.2", + "@codemirror/lang-json": "^6.0.1", + "@codemirror/theme-one-dark": "^6.0.0", Review Comment: In light mode the codemirror was always showing dark mode. THat disturbed a bit. I'd propose to ajust the coloring like the rest of the UI. Looks like this in light mode:  ...compared to this in ark mode...  ########## airflow/ui/src/components/DataTable/TriggerDAGForm.tsx: ########## @@ -0,0 +1,188 @@ +/*! + * 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 { + FormControl, + FormLabel, + Input, + VStack, + ModalCloseButton, + Button, + ModalFooter, + Box, + Text, + Spacer, + HStack, + Collapse, +} from "@chakra-ui/react"; +import { autocompletion } from "@codemirror/autocomplete"; +import { json } from "@codemirror/lang-json"; +import { oneDark } from "@codemirror/theme-one-dark"; +import CodeMirror, { lineNumbers } from "@uiw/react-codemirror"; +import React, { useState, useEffect } from "react"; + +type DagParams = { + configJson: string; + dagId: string; + logicalDate: string; + runId?: string; +}; + +type TriggerDAGFormProps = { + dagParams: DagParams; + onClose: () => void; + onTrigger: () => void; + setDagParams: React.Dispatch<React.SetStateAction<DagParams>>; +}; + +const TriggerDAGForm: React.FC<TriggerDAGFormProps> = ({ + dagParams, + onTrigger, + setDagParams, +}) => { + const [showDetails, setShowDetails] = useState(false); + + useEffect(() => { + try { + const prettyJson = JSON.stringify( + JSON.parse(dagParams.configJson), + undefined, + 2, + ); + + setDagParams((prev) => ({ ...prev, configJson: prettyJson })); + } catch { + // Invalid JSON handling + } + }, [dagParams.configJson, setDagParams]); + + const handleChange = ( + ele: React.ChangeEvent<HTMLInputElement | HTMLTextAreaElement>, + ) => { + const { name, value } = ele.target; + + setDagParams((prev) => ({ ...prev, [name]: value })); + }; + + const handleReset = () => { + setDagParams({ + configJson: "{}", + dagId: dagParams.dagId, + logicalDate: "", + runId: "", + }); + }; + + const handleJsonChange = (value: string) => { + setDagParams((prev) => ({ ...prev, configJson: value })); + }; + + const isValidJson = () => { + try { + JSON.parse(dagParams.configJson); + + return true; + } catch { + return false; + } + }; + + return ( + <> + <ModalCloseButton /> + + <VStack align="stretch" p={5} spacing={2}> + <Button + onClick={() => setShowDetails(!showDetails)} + variant="outline" + width="full" + > + {showDetails ? "Hide Advance Options" : "Show Advance Options"} + </Button> + + <Collapse in={showDetails}> + <VStack align="stretch" spacing={3}> + <FormControl> + <FormLabel size="sm">Logical date</FormLabel> + <Input + name="logicalDate" + onChange={handleChange} + placeholder="yyyy-mm-ddThh:mm" + size="sm" + type="datetime-local" + value={dagParams.logicalDate} + /> + </FormControl> + + <FormControl> + <FormLabel size="sm">Run ID (Optional)</FormLabel> Review Comment: Previous UI used: ```suggestion <FormLabel size="sm">Run ID</FormLabel> ``` ########## airflow/ui/src/components/DataTable/TriggerDAGForm.tsx: ########## @@ -0,0 +1,188 @@ +/*! + * 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 { + FormControl, + FormLabel, + Input, + VStack, + ModalCloseButton, + Button, + ModalFooter, + Box, + Text, + Spacer, + HStack, + Collapse, +} from "@chakra-ui/react"; +import { autocompletion } from "@codemirror/autocomplete"; +import { json } from "@codemirror/lang-json"; +import { oneDark } from "@codemirror/theme-one-dark"; +import CodeMirror, { lineNumbers } from "@uiw/react-codemirror"; +import React, { useState, useEffect } from "react"; + +type DagParams = { + configJson: string; + dagId: string; + logicalDate: string; + runId?: string; +}; + +type TriggerDAGFormProps = { + dagParams: DagParams; + onClose: () => void; + onTrigger: () => void; + setDagParams: React.Dispatch<React.SetStateAction<DagParams>>; +}; + +const TriggerDAGForm: React.FC<TriggerDAGFormProps> = ({ + dagParams, + onTrigger, + setDagParams, +}) => { + const [showDetails, setShowDetails] = useState(false); + + useEffect(() => { + try { + const prettyJson = JSON.stringify( + JSON.parse(dagParams.configJson), Review Comment: I tried with example_params_ui_tutorial but the pre-filled dict was `{}` - seems the defaults did not come down as default. ########## airflow/ui/src/pages/DagsList/DagsList.tsx: ########## @@ -102,6 +103,17 @@ const columns: Array<ColumnDef<DAGResponse>> = [ enableSorting: false, header: () => "Tags", }, + { + accessorKey: "trigger", + cell: ({ row }) => ( + <TriggerDAGModal + dagDisplayName={row.original.dag_display_name} + dagId={row.original.dag_id} Review Comment: As at a later point also the DAG params are needed - does it make sense to pass the full `row.original` object, not just the two strings? ########## airflow/ui/src/components/DataTable/TriggerDAGForm.tsx: ########## @@ -0,0 +1,188 @@ +/*! + * 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 { + FormControl, + FormLabel, + Input, + VStack, + ModalCloseButton, + Button, + ModalFooter, + Box, + Text, + Spacer, + HStack, + Collapse, +} from "@chakra-ui/react"; +import { autocompletion } from "@codemirror/autocomplete"; +import { json } from "@codemirror/lang-json"; +import { oneDark } from "@codemirror/theme-one-dark"; +import CodeMirror, { lineNumbers } from "@uiw/react-codemirror"; +import React, { useState, useEffect } from "react"; + +type DagParams = { + configJson: string; + dagId: string; + logicalDate: string; + runId?: string; +}; + +type TriggerDAGFormProps = { + dagParams: DagParams; + onClose: () => void; + onTrigger: () => void; + setDagParams: React.Dispatch<React.SetStateAction<DagParams>>; +}; + +const TriggerDAGForm: React.FC<TriggerDAGFormProps> = ({ + dagParams, + onTrigger, + setDagParams, +}) => { + const [showDetails, setShowDetails] = useState(false); + + useEffect(() => { + try { + const prettyJson = JSON.stringify( + JSON.parse(dagParams.configJson), + undefined, + 2, + ); + + setDagParams((prev) => ({ ...prev, configJson: prettyJson })); + } catch { + // Invalid JSON handling + } + }, [dagParams.configJson, setDagParams]); + + const handleChange = ( + ele: React.ChangeEvent<HTMLInputElement | HTMLTextAreaElement>, + ) => { + const { name, value } = ele.target; + + setDagParams((prev) => ({ ...prev, [name]: value })); + }; + + const handleReset = () => { + setDagParams({ + configJson: "{}", + dagId: dagParams.dagId, + logicalDate: "", + runId: "", + }); + }; + + const handleJsonChange = (value: string) => { + setDagParams((prev) => ({ ...prev, configJson: value })); + }; + + const isValidJson = () => { + try { + JSON.parse(dagParams.configJson); + + return true; + } catch { + return false; + } + }; + + return ( + <> + <ModalCloseButton /> + + <VStack align="stretch" p={5} spacing={2}> + <Button + onClick={() => setShowDetails(!showDetails)} + variant="outline" + width="full" + > + {showDetails ? "Hide Advance Options" : "Show Advance Options"} + </Button> + + <Collapse in={showDetails}> + <VStack align="stretch" spacing={3}> + <FormControl> + <FormLabel size="sm">Logical date</FormLabel> + <Input + name="logicalDate" + onChange={handleChange} + placeholder="yyyy-mm-ddThh:mm" + size="sm" + type="datetime-local" + value={dagParams.logicalDate} + /> + </FormControl> + + <FormControl> + <FormLabel size="sm">Run ID (Optional)</FormLabel> + <Input + name="runId" + onChange={handleChange} + placeholder="Autogenerated if left empty" + size="sm" + value={dagParams.runId} + /> + </FormControl> + + <FormControl> + <FormLabel size="sm">Configuration JSON</FormLabel> + <CodeMirror + extensions={[json(), autocompletion(), lineNumbers()]} + height="200px" + onChange={handleJsonChange} Review Comment: While I was typing JSON text the cursor jumped around and then then JSON was malformed. I assume the native auto-formatting of CodeMirror is better than this. -- 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]
