fix: cache icon fro map task (#519)

* fix: cache icon fro map task * fix: icon and text center Signed-off-by: eugenejahn <[email protected]>
flyteorg · Jun 28, 2022 · e933f21 · e933f21
1 parent 54c8ca1
commit e933f21
Show file tree

Hide file tree

Showing 16 changed files with 296 additions and 109 deletions.
diff --git a/packages/composites/ui-atoms/src/Icons/MapCacheIcon/index.tsx b/packages/composites/ui-atoms/src/Icons/MapCacheIcon/index.tsx
@@ -0,0 +1,24 @@
+import * as React from 'react';
+import SvgIcon, { SvgIconProps } from '@material-ui/core/SvgIcon';
+
+export const MapCacheIcon = React.forwardRef<SVGSVGElement, SvgIconProps>((props, ref) => {
+  return (
+    <SvgIcon {...props} ref={ref} viewBox="0 0 17 17">
+      <g clipPath="url(#clip0_6712_89419)">
+        <path
+          d="M5.68615 2.99228L6.91566 7.15285L4.89438 6.0537C4.31136 7.12586 4.17812 8.3857 4.52399 9.55609C4.86986 10.7265 5.45103 11.4847 6.52318 12.0677C7.19695 12.4341 8.24054 12.5388 8.96464 12.5397L9.41341 14.0583C8.25879 14.1378 6.70623 14.0476 5.68964 13.4944C4.2601 12.717 3.51417 11.5513 3.05301 9.99079C2.59185 8.43027 2.76949 6.75048 3.54686 5.32094L1.52558 4.22179L5.68615 2.99228Z"
+          fill="#666666"
+        />
+        <path
+          d="M9.90614 2.76737C11.152 3.02647 12.2756 3.69438 13.0985 4.66504C13.9214 5.6357 14.3964 6.85346 14.4481 8.12494C14.4998 9.39642 14.1252 10.6487 13.3839 11.683C12.6425 12.7173 11.5768 13.4742 10.3561 13.8336L8.74374 8.35689L9.90614 2.76737Z"
+          fill="#DDDDE5"
+        />
+      </g>
+      <defs>
+        <clipPath id="clip0_6712_89419">
+          <rect width="16" height="16" fill="white" transform="translate(0.406982 0.356445)" />
+        </clipPath>
+      </defs>
+    </SvgIcon>
+  );
+});
diff --git a/packages/composites/ui-atoms/src/Icons/index.tsx b/packages/composites/ui-atoms/src/Icons/index.tsx
@@ -1,4 +1,5 @@
 export { FlyteLogo } from './FlyteLogo';
 export { InfoIcon } from './InfoIcon';
 export { RerunIcon } from './RerunIcon';
+export { MapCacheIcon } from './MapCacheIcon';
 export { MuiLaunchPlanIcon } from './MuiLaunchPlanIcon';
diff --git a/...p/console/src/components/Executions/ExecutionDetails/NodeExecutionDetailsPanelContent.tsx b/...p/console/src/components/Executions/ExecutionDetails/NodeExecutionDetailsPanelContent.tsx
@@ -364,7 +364,7 @@ export const NodeExecutionDetailsPanelContent: React.FC<NodeExecutionDetailsProp
   if (nodeExecution) {
     detailsContent = (
       <>
-        <NodeExecutionCacheStatus taskNodeMetadata={nodeExecution.closure.taskNodeMetadata} />
+        <NodeExecutionCacheStatus execution={nodeExecution} />
         <ExecutionTypeDetails details={details} execution={nodeExecution} />
       </>
     );

diff --git a/...pp/console/src/components/Executions/ExecutionDetails/Timeline/TimelineChart/chartData.ts b/...pp/console/src/components/Executions/ExecutionDetails/Timeline/TimelineChart/chartData.ts
@@ -1,6 +1,7 @@
 import { timestampToDate } from 'common/utils';
 import { CatalogCacheStatus, NodeExecutionPhase } from 'models/Execution/enums';
 import { dNode } from 'models/Graph/types';
+import { isMapTaskType } from 'models/Task/utils';
 import { BarItemData } from './utils';
 
 const WEEK_DURATION_SEC = 7 * 24 * 3600;
@@ -10,6 +11,7 @@ const EMPTY_BAR_ITEM: BarItemData = {
   startOffsetSec: 0,
   durationSec: 0,
   isFromCache: false,
+  isMapTaskCache: false,
 };
 
 export const getChartDurationData = (
@@ -20,7 +22,8 @@ export const getChartDurationData = (
 
   let totalDurationSec = 0;
   const initialStartTime = startedAt.getTime();
-  const result: BarItemData[] = nodes.map(({ execution }) => {
+
+  const result: BarItemData[] = nodes.map(({ execution, value }) => {
     if (!execution) {
       return EMPTY_BAR_ITEM;
     }
@@ -29,6 +32,9 @@ export const getChartDurationData = (
     const isFromCache =
       execution.closure.taskNodeMetadata?.cacheStatus === CatalogCacheStatus.CACHE_HIT;
 
+    const isMapTaskCache =
+      isMapTaskType(value?.template?.type) && value?.template?.metadata?.cacheSerializable;
+
     // Offset values
     let startOffset = 0;
     const startedAt = execution.closure.startedAt;
@@ -67,7 +73,7 @@ export const getChartDurationData = (
 
     const startOffsetSec = Math.trunc(startOffset / 1000);
     totalDurationSec = Math.max(totalDurationSec, startOffsetSec + durationSec);
-    return { phase, startOffsetSec, durationSec, isFromCache };
+    return { phase, startOffsetSec, durationSec, isFromCache, isMapTaskCache };
   });
 
   // Do we want to get initialStartTime from different place, to avoid recalculating it.

diff --git a/...s/zapp/console/src/components/Executions/ExecutionDetails/Timeline/TimelineChart/utils.ts b/...s/zapp/console/src/components/Executions/ExecutionDetails/Timeline/TimelineChart/utils.ts
@@ -1,6 +1,7 @@
 import { getNodeExecutionPhaseConstants } from 'components/Executions/utils';
 import { primaryTextColor } from 'components/Theme/constants';
 import { NodeExecutionPhase } from 'models/Execution/enums';
+import t from 'components/Executions/strings';
 
 export const CASHED_GREEN = 'rgba(74,227,174,0.25)'; // statusColors.SUCCESS (Mint20) with 25% opacity
 export const TRANSPARENT = 'rgba(0, 0, 0, 0)';
@@ -10,6 +11,7 @@ export interface BarItemData {
   startOffsetSec: number;
   durationSec: number;
   isFromCache: boolean;
+  isMapTaskCache: boolean;
 }
 
 interface ChartDataInput {
@@ -60,11 +62,24 @@ export const generateChartData = (data: BarItemData[]): ChartDataInput => {
     // don't show Label if there is now duration yet.
     const labelString = element.durationSec > 0 ? durationString : '';
 
+    const generateTooltipLabelText = (element: BarItemData): string[] => {
+      if (element.isMapTaskCache) return [tooltipString, t('mapCacheMessage')];
+      if (element.isFromCache) return [tooltipString, t('readFromCache')];
+
+      return [tooltipString];
+    };
+
+    const generateBarLabelText = (element: BarItemData): string => {
+      if (element.isMapTaskCache) return '\u229A ' + t('mapCacheMessage');
+      if (element.isFromCache) return '\u229A ' + t('fromCache');
+      return labelString;
+    };
+
     durations.push(element.durationSec);
     startOffset.push(element.startOffsetSec);
     offsetColor.push(element.isFromCache ? CASHED_GREEN : TRANSPARENT);
-    tooltipLabel.push(element.isFromCache ? [tooltipString, 'Read from cache'] : [tooltipString]);
-    barLabel.push(element.isFromCache ? '\u229A From cache' : labelString);
+    tooltipLabel.push(generateTooltipLabelText(element));
+    barLabel.push(generateBarLabelText(element));
     barColor.push(phaseConstant.badgeColor);
   });
 

diff --git a/packages/zapp/console/src/components/Executions/ExecutionDetails/test/TimelineChart.test.tsx b/packages/zapp/console/src/components/Executions/ExecutionDetails/test/TimelineChart.test.tsx
@@ -28,12 +28,21 @@ describe('ExecutionDetails > Timeline > BarChart', () => {
 
   it('generateChartData properly generates map of data for ChartBars', () => {
     const chartData = generateChartData(mockbarItems);
-    expect(chartData.durations).toEqual([15, 11, 23, 0]);
-    expect(chartData.startOffset).toEqual([0, 5, 17, 39]);
-    expect(chartData.offsetColor).toEqual([TRANSPARENT, CASHED_GREEN, TRANSPARENT, TRANSPARENT]);
+    expect(chartData.durations).toEqual([15, 11, 23, 0, 11]);
+    expect(chartData.startOffset).toEqual([0, 5, 17, 39, 5]);
+    expect(chartData.offsetColor).toEqual([
+      TRANSPARENT,
+      CASHED_GREEN,
+      TRANSPARENT,
+      TRANSPARENT,
+      TRANSPARENT,
+    ]);
     // labels looks as expected
     expect(chartData.barLabel[0]).toEqual(formatSecondsToHmsFormat(mockbarItems[0].durationSec));
     expect(chartData.barLabel[1]).toEqual('\u229A From cache');
     expect(chartData.barLabel[3]).toEqual('');
+    expect(chartData.barLabel[4]).toEqual(
+      "\u229A Check the detail panel for each task's cache status.",
+    );
   });
 });
diff --git a/...p/console/src/components/Executions/ExecutionDetails/test/__mocks__/NodeExecution.mock.ts b/...p/console/src/components/Executions/ExecutionDetails/test/__mocks__/NodeExecution.mock.ts
@@ -60,10 +60,41 @@ const getMockNodeExecution = (
 };
 
 export const mockbarItems = [
-  { phase: NodeExecutionPhase.FAILED, startOffsetSec: 0, durationSec: 15, isFromCache: false },
-  { phase: NodeExecutionPhase.SUCCEEDED, startOffsetSec: 5, durationSec: 11, isFromCache: true },
-  { phase: NodeExecutionPhase.RUNNING, startOffsetSec: 17, durationSec: 23, isFromCache: false },
-  { phase: NodeExecutionPhase.QUEUED, startOffsetSec: 39, durationSec: 0, isFromCache: false },
+  {
+    phase: NodeExecutionPhase.FAILED,
+    startOffsetSec: 0,
+    durationSec: 15,
+    isFromCache: false,
+    isMapTaskCache: false,
+  },
+  {
+    phase: NodeExecutionPhase.SUCCEEDED,
+    startOffsetSec: 5,
+    durationSec: 11,
+    isFromCache: true,
+    isMapTaskCache: false,
+  },
+  {
+    phase: NodeExecutionPhase.RUNNING,
+    startOffsetSec: 17,
+    durationSec: 23,
+    isFromCache: false,
+    isMapTaskCache: false,
+  },
+  {
+    phase: NodeExecutionPhase.QUEUED,
+    startOffsetSec: 39,
+    durationSec: 0,
+    isFromCache: false,
+    isMapTaskCache: false,
+  },
+  {
+    phase: NodeExecutionPhase.SUCCEEDED,
+    startOffsetSec: 5,
+    durationSec: 11,
+    isFromCache: false,
+    isMapTaskCache: true,
+  },
 ];
 
 export const getMockExecutionsForBarChart = (startTimeSec: number) => {
@@ -73,5 +104,6 @@ export const getMockExecutionsForBarChart = (startTimeSec: number) => {
     getMockNodeExecution(start, NodeExecutionPhase.SUCCEEDED, 5, 11, CatalogCacheStatus.CACHE_HIT),
     getMockNodeExecution(start, NodeExecutionPhase.RUNNING, 17, 23, CatalogCacheStatus.CACHE_MISS),
     getMockNodeExecution(start, NodeExecutionPhase.QUEUED, 39, 0),
+    getMockNodeExecution(start, NodeExecutionPhase.SUCCEEDED, 5, 11, CatalogCacheStatus.MAP_CACHE),
   ];
 };
diff --git a/packages/zapp/console/src/components/Executions/NodeExecutionCacheStatus.tsx b/packages/zapp/console/src/components/Executions/NodeExecutionCacheStatus.tsx
@@ -7,9 +7,13 @@ import classnames from 'classnames';
 import { assertNever } from 'common/utils';
 import { PublishedWithChangesOutlined } from 'components/common/PublishedWithChanges';
 import { useCommonStyles } from 'components/common/styles';
+import { NodeExecutionDetails } from 'components/Executions/types';
+import { useNodeExecutionContext } from 'components/Executions/contextProvider/NodeExecutionDetails';
 import { CatalogCacheStatus } from 'models/Execution/enums';
-import { TaskNodeMetadata } from 'models/Execution/types';
+import { NodeExecution, TaskExecutionIdentifier } from 'models/Execution/types';
+import { MapCacheIcon } from '@flyteconsole/ui-atoms';
 import * as React from 'react';
+import { isMapTaskType } from 'models/Task/utils';
 import { Link as RouterLink } from 'react-router-dom';
 import { Routes } from 'routes/routes';
 import {
@@ -50,6 +54,9 @@ export const NodeExecutionCacheStatusIcon: React.FC<
     case CatalogCacheStatus.CACHE_PUT_FAILURE: {
       return <ErrorOutlined {...props} ref={ref} />;
     }
+    case CatalogCacheStatus.MAP_CACHE: {
+      return <MapCacheIcon {...props} ref={ref} />;
+    }
     default: {
       assertNever(status);
       return null;
@@ -58,7 +65,7 @@ export const NodeExecutionCacheStatusIcon: React.FC<
 });
 
 export interface NodeExecutionCacheStatusProps {
-  taskNodeMetadata?: TaskNodeMetadata;
+  execution: NodeExecution;
   /** `normal` will render an icon with description message beside it
    *  `iconOnly` will render just the icon with the description as a tooltip
    */
@@ -68,20 +75,78 @@ export interface NodeExecutionCacheStatusProps {
  * the cache status with a descriptive message. For `Core.CacheCatalogStatus.CACHE_HIT`,
  * it will also attempt to render a link to the source `WorkflowExecution` (normal
  * variant only).
+ *
+ * For Map Tasks, we will check the NodeExecutionDetail for the cache status instead. Since map tasks
+ * cotains multiple tasks, the logic of the cache status is different.
  */
 export const NodeExecutionCacheStatus: React.FC<NodeExecutionCacheStatusProps> = ({
-  taskNodeMetadata,
+  execution,
+  variant = 'normal',
+}) => {
+  const taskNodeMetadata = execution.closure?.taskNodeMetadata;
+  const detailsContext = useNodeExecutionContext();
+  const [nodeDetails, setNodeDetails] = React.useState<NodeExecutionDetails | undefined>();
+
+  React.useEffect(() => {
+    let isCurrent = true;
+    detailsContext.getNodeExecutionDetails(execution).then((res) => {
+      if (isCurrent) {
+        setNodeDetails(res);
+      }
+    });
+    return () => {
+      isCurrent = false;
+    };
+  });
+
+  if (isMapTaskType(nodeDetails?.taskTemplate?.type)) {
+    if (nodeDetails?.taskTemplate?.metadata?.cacheSerializable) {
+      return <CacheStatus cacheStatus={CatalogCacheStatus.MAP_CACHE} variant={variant} />;
+    }
+  }
+
+  // cachestatus can be 0
+  if (taskNodeMetadata?.cacheStatus == null) {
+    return null;
+  }
+
+  const sourceTaskExecution = taskNodeMetadata.catalogKey?.sourceTaskExecution;
+
+  return (
+    <CacheStatus
+      cacheStatus={taskNodeMetadata.cacheStatus}
+      sourceTaskExecution={sourceTaskExecution}
+      variant={variant}
+    />
+  );
+};
+
+export interface CacheStatusProps {
+  cacheStatus: CatalogCacheStatus | null | undefined;
+  /** `normal` will render an icon with description message beside it
+   *  `iconOnly` will render just the icon with the description as a tooltip
+   */
+  variant?: 'normal' | 'iconOnly';
+  sourceTaskExecution?: TaskExecutionIdentifier;
+  iconStyles?: React.CSSProperties;
+}
+
+export const CacheStatus: React.FC<CacheStatusProps> = ({
+  cacheStatus,
+  sourceTaskExecution,
   variant = 'normal',
+  iconStyles,
 }) => {
   const commonStyles = useCommonStyles();
   const styles = useStyles();
-  if (taskNodeMetadata == null || taskNodeMetadata.cacheStatus == null) {
+
+  if (cacheStatus == null) {
     return null;
   }
 
-  const message = cacheStatusMessages[taskNodeMetadata.cacheStatus] || unknownCacheStatusString;
+  const message = cacheStatusMessages[cacheStatus] || unknownCacheStatusString;
 
-  const sourceExecutionId = taskNodeMetadata.catalogKey?.sourceTaskExecution;
+  const sourceExecutionId = sourceTaskExecution;
   const sourceExecutionLink = sourceExecutionId ? (
     <RouterLink
       className={classnames(commonStyles.primaryLink, styles.sourceExecutionLink)}
@@ -95,18 +160,19 @@ export const NodeExecutionCacheStatus: React.FC<NodeExecutionCacheStatusProps> =
     <Tooltip title={message} aria-label="cache status">
       <NodeExecutionCacheStatusIcon
         className={classnames(
-          commonStyles.iconLeft,
+          // commonStyles.iconLeft,
           commonStyles.iconRight,
           commonStyles.iconSecondary,
         )}
-        status={taskNodeMetadata.cacheStatus}
+        style={iconStyles}
+        status={cacheStatus}
       />
     </Tooltip>
   ) : (
     <div>
       <Typography className={styles.cacheStatus} variant="subtitle1" color="textSecondary">
         <NodeExecutionCacheStatusIcon
-          status={taskNodeMetadata.cacheStatus}
+          status={cacheStatus}
           className={classnames(commonStyles.iconSecondary, commonStyles.iconLeft)}
         />
         {message}

diff --git a/packages/zapp/console/src/components/Executions/Tables/nodeExecutionColumns.tsx b/packages/zapp/console/src/components/Executions/Tables/nodeExecutionColumns.tsx
@@ -139,16 +139,13 @@ export function generateColumns(
       label: 'type',
     },
     {
-      cellRenderer: ({
-        execution: {
-          closure: { phase = NodeExecutionPhase.UNDEFINED, taskNodeMetadata },
-        },
-      }) => (
+      cellRenderer: ({ execution }) => (
         <>
-          <ExecutionStatusBadge phase={phase} type="node" />
-          {hasCacheStatus(taskNodeMetadata) ? (
-            <NodeExecutionCacheStatus taskNodeMetadata={taskNodeMetadata} variant="iconOnly" />
-          ) : null}
+          <ExecutionStatusBadge
+            phase={execution.closure?.phase ?? NodeExecutionPhase.UNDEFINED}
+            type="node"
+          />
+          <NodeExecutionCacheStatus execution={execution} variant="iconOnly" />
         </>
       ),
       className: styles.columnStatus,

diff --git a/packages/zapp/console/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx b/packages/zapp/console/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx
@@ -243,6 +243,8 @@ describe('NodeExecutionsTable', () => {
         CatalogCacheStatus.CACHE_LOOKUP_FAILURE,
         CatalogCacheStatus.CACHE_POPULATED,
         CatalogCacheStatus.CACHE_PUT_FAILURE,
+        CatalogCacheStatus.CACHE_MISS,
+        CatalogCacheStatus.CACHE_DISABLED,
       ].forEach((cacheStatusValue) =>
         it(`renders correct icon for ${CatalogCacheStatus[cacheStatusValue]}`, async () => {
           taskNodeMetadata.cacheStatus = cacheStatusValue;
@@ -254,19 +256,6 @@ describe('NodeExecutionsTable', () => {
           );
         }),
       );
-
-      [CatalogCacheStatus.CACHE_DISABLED, CatalogCacheStatus.CACHE_MISS].forEach(
-        (cacheStatusValue) =>
-          it(`renders no icon for ${CatalogCacheStatus[cacheStatusValue]}`, async () => {
-            taskNodeMetadata.cacheStatus = cacheStatusValue;
-            updateNodeExecutions([cachedNodeExecution]);
-            const { getByText, queryByTitle } = renderTable();
-            await waitFor(() => {
-              getByText(cachedNodeExecution.id.nodeId);
-            });
-            expect(queryByTitle(cacheStatusMessages[cacheStatusValue])).toBeNull();
-          }),
-      );
     });
   });