Skip to content

Conversation

@bbovenzi
Copy link
Contributor

@bbovenzi bbovenzi commented Dec 4, 2024

Make a shared DetailsLayout component to help render the pages for a Dag, Dag Run, and Task Instance. The layout handles errors, loading, tabs and the visualizations modal. Also updated the visualizations modal to include space for the Grid and Gantt charts

Screenshot 2024-12-04 at 4 00 01 PM Screenshot 2024-12-04 at 4 00 09 PM Screenshot 2024-12-04 at 4 00 16 PM ---

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
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.

@boring-cyborg boring-cyborg bot added the area:UI Related to UI/UX. For Frontend Developers. label Dec 4, 2024
@bbovenzi bbovenzi mentioned this pull request Dec 4, 2024
@bbovenzi bbovenzi changed the title Add basic Dag Run Details page Fill out Dag Run and Task Instance Details pages with Grid and Gantt buttons. Dec 4, 2024
@tirkarthi
Copy link
Contributor

The components and pages can later have clipboard button in relevant places to copy dag_id, run_id, task_id etc. which was present in the old UI and was handy.

https://www.chakra-ui.com/docs/components/clipboard

@tirkarthi
Copy link
Contributor

One way to handle mapped task instances might be to pass map_index as a query parameter in the URL and then handle it. Below is a rough approach but this could be discussed in a separate issue since this will involve URL changes.

diff --git a/airflow/ui/src/pages/Events/Events.tsx b/airflow/ui/src/pages/Events/Events.tsx
index 60663aefec..d311471252 100644
--- a/airflow/ui/src/pages/Events/Events.tsx
+++ b/airflow/ui/src/pages/Events/Events.tsx
@@ -18,7 +18,7 @@
  */
 import { Box } from "@chakra-ui/react";
 import type { ColumnDef } from "@tanstack/react-table";
-import { useParams } from "react-router-dom";
+import { useSearchParams, useParams } from "react-router-dom";
 
 import { useEventLogServiceGetEventLogs } from "openapi/queries";
 import type { EventLogResponse } from "openapi/requests/types.gen";
@@ -113,6 +113,8 @@ const eventsColumn = (
 
 export const Events = () => {
   const { dagId, runId, taskId } = useParams();
+  const [searchParams, setSearchParams] = useSearchParams();
+  const mapIndex = searchParams.get("map_index");
   const { setTableURLState, tableURLState } = useTableURLState({
     sorting: [{ desc: true, id: "when" }],
   });
@@ -132,6 +134,8 @@ export const Events = () => {
     offset: pagination.pageIndex * pagination.pageSize,
     orderBy,
     runId,
+    taskId,
+    mapIndex,
   });
 
   return (
diff --git a/airflow/ui/src/pages/Run/TaskInstances.tsx b/airflow/ui/src/pages/Run/TaskInstances.tsx
index e96ff55d46..4e4127cdfc 100644
--- a/airflow/ui/src/pages/Run/TaskInstances.tsx
+++ b/airflow/ui/src/pages/Run/TaskInstances.tsx
@@ -35,7 +35,7 @@ const columns: Array<ColumnDef<TaskInstanceResponse>> = [
     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}`}
+          to={`/dags/${original.dag_id}/runs/${original.dag_run_id}/tasks/${original.task_id}?map_index=${original.map_index}`}
         >
           {original.task_display_name}
         </RouterLink>
diff --git a/airflow/ui/src/pages/TaskInstance/TaskInstance.tsx b/airflow/ui/src/pages/TaskInstance/TaskInstance.tsx
index b9e41de93d..d0f5ca9930 100644
--- a/airflow/ui/src/pages/TaskInstance/TaskInstance.tsx
+++ b/airflow/ui/src/pages/TaskInstance/TaskInstance.tsx
@@ -17,37 +17,51 @@
  * under the License.
  */
 import { LiaSlashSolid } from "react-icons/lia";
-import { useParams, Link as RouterLink } from "react-router-dom";
+import {
+  useParams,
+  useSearchParams,
+  Link as RouterLink,
+} from "react-router-dom";
 
 import {
   useDagServiceGetDagDetails,
   useTaskInstanceServiceGetTaskInstance,
+  useTaskInstanceServiceGetMappedTaskInstance,
 } from "openapi/queries";
 import { Breadcrumb } from "src/components/ui";
 import { DetailsLayout } from "src/layouts/Details/DetailsLayout";
 
 import { Header } from "./Header";
 
-const tabs = [
-  { label: "Logs", value: "" },
-  { label: "Events", value: "events" },
-  { label: "XCom", value: "xcom" },
-  { label: "Code", value: "code" },
-  { label: "Details", value: "details" },
-];
-
 export const TaskInstance = () => {
   const { dagId = "", runId = "", taskId = "" } = useParams();
+  const [searchParams, setSearchParams] = useSearchParams();
+  const mapIndex = searchParams.get("map_index");
+
+  const tabs = [
+    { label: "Logs", value: "" },
+    { label: "Events", value: `events?map_index=${mapIndex}` },
+    { label: "XCom", value: `xcom?map_index=${mapIndex}` },
+    { label: "Code", value: `code?map_index=${mapIndex}` },
+    { label: "Details", value: `details?map_index=${mapIndex}` },
+  ];
 
   const {
     data: taskInstance,
     error,
     isLoading,
-  } = useTaskInstanceServiceGetTaskInstance({
-    dagId,
-    dagRunId: runId,
-    taskId,
-  });
+  } = Boolean(mapIndex) && mapIndex > -1
+    ? useTaskInstanceServiceGetMappedTaskInstance({
+        dagId,
+        dagRunId: runId,
+        taskId,
+        mapIndex,
+      })
+    : useTaskInstanceServiceGetTaskInstance({
+        dagId,
+        dagRunId: runId,
+        taskId,
+      });
 
   const {
     data: dag,

@bbovenzi
Copy link
Contributor Author

bbovenzi commented Dec 6, 2024

One way to handle mapped task instances might be to pass map_index as a query parameter in the URL and then handle it. Below is a rough approach but this could be discussed in a separate issue since this will involve URL changes.

Updated everything to accept map_index=X search param or to at least forward the param around. Also, I realized we can just use useTaskInstanceServiceGetMappedTaskInstance and pass -1 for regular task instances.

Copy link
Contributor

@shubhamraj-git shubhamraj-git left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes! This looks perfect overall. Just one minor suggestion.

Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working as expected. Code looks good!

@bbovenzi bbovenzi merged commit e122b20 into apache:main Dec 10, 2024
40 checks passed
@bbovenzi bbovenzi deleted the dag-run-details branch December 10, 2024 17:53
got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
…buttons. (apache#44656)

* Rebasing

* Handle mapped instances

* Handle null in Status component
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:UI Related to UI/UX. For Frontend Developers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants