fix: add eventVersion check for map tasks (#484)

flyteorg · May 27, 2022 · b70d14b · b70d14b
1 parent 747b100
commit b70d14b
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 6 deletions.
diff --git a/packages/zapp/console/src/components/Executions/ExecutionDetails/ExecutionWorkflowGraph.tsx b/packages/zapp/console/src/components/Executions/ExecutionDetails/ExecutionWorkflowGraph.tsx
@@ -7,6 +7,7 @@ import { keyBy } from 'lodash';
 import { TaskExecutionPhase } from 'models/Execution/enums';
 import { ExternalResource, LogsByPhase, NodeExecution } from 'models/Execution/types';
 import { endNodeId, startNodeId } from 'models/Node/constants';
+import { isMapTaskV1 } from 'models/Task/utils';
 import { Workflow, WorkflowId } from 'models/Workflow/types';
 import * as React from 'react';
 import { useEffect, useMemo, useState } from 'react';
@@ -32,6 +33,16 @@ export const ExecutionWorkflowGraph: React.FC<ExecutionWorkflowGraphProps> = ({
     const taskExecutions = useTaskExecutions(nodeExecution.id);
     useTaskExecutionsRefresher(nodeExecution, taskExecutions);
 
+    const useNewMapTaskView = taskExecutions.value.every((taskExecution) => {
+      const {
+        closure: { taskType, metadata, eventVersion = 0 },
+      } = taskExecution;
+      return isMapTaskV1(
+        eventVersion,
+        metadata?.externalResources?.length ?? 0,
+        taskType ?? undefined,
+      );
+    });
     const externalResources: ExternalResource[] = taskExecutions.value
       .map((taskExecution) => taskExecution.closure.metadata?.externalResources)
       .flat()
@@ -41,7 +52,7 @@ export const ExecutionWorkflowGraph: React.FC<ExecutionWorkflowGraphProps> = ({
 
     return {
       ...nodeExecution,
-      ...(logsByPhase.size > 0 && { logsByPhase }),
+      ...(useNewMapTaskView && logsByPhase.size > 0 && { logsByPhase }),
     };
   });
 

diff --git a/packages/zapp/console/src/components/Executions/TaskExecutionsList/TaskExecutionsList.tsx b/packages/zapp/console/src/components/Executions/TaskExecutionsList/TaskExecutionsList.tsx
@@ -4,7 +4,7 @@ import { noExecutionsFoundString } from 'common/constants';
 import { NonIdealState } from 'components/common/NonIdealState';
 import { WaitForData } from 'components/common/WaitForData';
 import { MapTaskExecution, NodeExecution, TaskExecution } from 'models/Execution/types';
-import { isMapTaskType } from 'models/Task/utils';
+import { isMapTaskV1 } from 'models/Task/utils';
 import { TaskExecutionPhase } from 'models/Execution/enums';
 import { useTaskExecutions, useTaskExecutionsRefresher } from '../useTaskExecutions';
 import { MapTaskExecutionsListItem } from './MapTaskExecutionListItem';
@@ -42,9 +42,14 @@ export const TaskExecutionsListContent: React.FC<{
   return (
     <>
       {taskExecutions.map((taskExecution) => {
-        const taskType = taskExecution.closure.taskType ?? undefined;
-        const useNewMapTaskView =
-          isMapTaskType(taskType) && taskExecution.closure.metadata?.externalResources;
+        const {
+          closure: { taskType, metadata, eventVersion = 0 },
+        } = taskExecution;
+        const useNewMapTaskView = isMapTaskV1(
+          eventVersion,
+          metadata?.externalResources?.length ?? 0,
+          taskType ?? undefined,
+        );
         return useNewMapTaskView ? (
           <MapTaskExecutionsListItem
             key={getUniqueTaskExecutionName(taskExecution)}

diff --git a/packages/zapp/console/src/components/WorkflowGraph/test/WorkflowGraph.test.tsx b/packages/zapp/console/src/components/WorkflowGraph/test/WorkflowGraph.test.tsx
@@ -28,6 +28,7 @@ describe('WorkflowGraph', () => {
             onPhaseSelectionChanged={jest.fn}
             workflow={workflow}
             nodeExecutionsById={nodeExecutionsById}
+            isDetailsTabClosed={true}
           />
         </QueryClientProvider>,
       );

diff --git a/packages/zapp/console/src/models/Task/task.test.ts b/packages/zapp/console/src/models/Task/task.test.ts
@@ -1,5 +1,5 @@
 import { TaskType } from './constants';
-import { isMapTaskType } from './utils';
+import { isMapTaskType, isMapTaskV1 } from './utils';
 
 describe('models/Task', () => {
   it('isMapTaskType propely identifies mapped tasks', async () => {
@@ -14,4 +14,31 @@ describe('models/Task', () => {
     // when regular task is provided - to be false
     expect(isMapTaskType(TaskType.PYTHON)).toBeFalsy();
   });
+
+  it('isMapTaskV1 propely identifies mapped task events above version 1', () => {
+    const eventVersionZero = 0;
+    const eventVersionN = 3;
+    const resourcesLengthZero = 0;
+    const resourcesLengthN = 5;
+    const mapTaskTypes = [TaskType.ARRAY, TaskType.ARRAY_AWS, TaskType.ARRAY_K8S];
+    const nonMapTaskType = TaskType.PYTHON;
+
+    describe('FALSE cases:', () => {
+      // when no task is provided
+      expect(isMapTaskV1(eventVersionN, resourcesLengthN)).toBeFalsy();
+      // when regular task is provided
+      expect(isMapTaskV1(eventVersionN, resourcesLengthN, nonMapTaskType)).toBeFalsy();
+      // when eventVersion < 1
+      expect(isMapTaskV1(eventVersionZero, resourcesLengthN, mapTaskTypes[0])).toBeFalsy();
+      // when externalResources array has length of 0
+      expect(isMapTaskV1(eventVersionN, resourcesLengthZero, mapTaskTypes[0])).toBeFalsy();
+    });
+
+    describe('TRUE cases:', () => {
+      // when mapped task is provided AND eventVersion >= 1 AND externalResources array length > 0
+      for (const task of mapTaskTypes) {
+        expect(isMapTaskV1(eventVersionN, resourcesLengthN, task)).toBeTruthy();
+      }
+    });
+  });
 });
diff --git a/packages/zapp/console/src/models/Task/utils.ts b/packages/zapp/console/src/models/Task/utils.ts
@@ -24,3 +24,12 @@ export function isMapTaskType(taskType?: string): boolean {
     taskType === TaskType.ARRAY_K8S
   );
 }
+
+/** Returns true if tasks schema is treated as a map task AND eventVersion >= 1 AND externalResources array length > 0 */
+export function isMapTaskV1(
+  eventVersion: number,
+  externalResourcesLength: number,
+  taskType?: string,
+): boolean {
+  return isMapTaskType(taskType) && eventVersion >= 1 && externalResourcesLength > 0;
+}