[UI] Visualization only loads on completed nodes (#3082)

* Visualization only loads on succeeded nodees * Also allow failed nodes to visualize
kubeflow · Feb 14, 2020 · b444f56 · b444f56
1 parent e10e119
commit b444f56
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 52 deletions.
diff --git a/frontend/src/pages/RunDetails.tsx b/frontend/src/pages/RunDetails.tsx
@@ -42,7 +42,11 @@ import { Page } from './Page';
 import { RoutePage, RouteParams } from '../components/Router';
 import { ToolbarProps } from '../components/Toolbar';
 import { ViewerConfig, PlotType } from '../components/viewers/Viewer';
-import { Workflow } from '../../third_party/argo-ui/argo_template';
+import {
+  Workflow,
+  NodeStatus,
+  NodePhase as ArgoNodePhase,
+} from '../../third_party/argo-ui/argo_template';
 import { classes, stylesheet } from 'typestyle';
 import { commonCss, padding, color, fonts, fontsize } from '../Css';
 import { componentMap } from '../components/viewers/ViewerContainer';
@@ -285,8 +289,14 @@ class RunDetails extends Page<RunDetailsProps, RunDetailsState> {
                                   this.state.selectedNodeDetails &&
                                   this.state.workflow && (
                                     <ArtifactsTabContent
-                                      workflow={this.state.workflow}
-                                      selectedNodeDetails={this.state.selectedNodeDetails}
+                                      nodeId={this.state.selectedNodeDetails.id}
+                                      nodeStatus={
+                                        this.state.workflow && this.state.workflow.status
+                                          ? this.state.workflow.status.nodes[
+                                              this.state.selectedNodeDetails.id
+                                            ]
+                                          : undefined
+                                      }
                                       visualizationCreatorConfig={visualizationCreatorConfig}
                                       generatedVisualizations={this.state.generatedVisualizations.filter(
                                         visualization =>
@@ -882,30 +892,27 @@ const Progress: React.FC<{
   );
 };
 
+const COMPLETED_NODE_PHASES: ArgoNodePhase[] = ['Succeeded', 'Failed', 'Error'];
+
 // TODO: add unit tests for this.
 /**
  * Artifacts tab content component, it handles loading progress state of
  * artifacts and visualize progress as a circular progress icon.
  */
 const ArtifactsTabContent: React.FC<{
   visualizationCreatorConfig: VisualizationCreatorConfig;
-  selectedNodeDetails: SelectedNodeDetails;
+  nodeId: string;
+  nodeStatus?: NodeStatus;
   generatedVisualizations: GeneratedVisualization[];
-  workflow: Workflow;
   onError: (error: Error) => void;
-}> = ({
-  visualizationCreatorConfig,
-  generatedVisualizations,
-  selectedNodeDetails,
-  workflow,
-  onError,
-}) => {
+}> = ({ visualizationCreatorConfig, generatedVisualizations, nodeId, nodeStatus, onError }) => {
   const [loaded, setLoaded] = React.useState(false);
   // Progress component expects onLoad function identity to stay the same
   const onLoad = React.useCallback(() => setLoaded(true), [setLoaded]);
 
   const [progress, setProgress] = React.useState(0);
   const [viewerConfigs, setViewerConfigs] = React.useState<ViewerConfig[]>([]);
+  const nodeCompleted: boolean = !!nodeStatus && COMPLETED_NODE_PHASES.includes(nodeStatus.phase);
 
   React.useEffect(() => {
     let aborted = false;
@@ -917,14 +924,12 @@ const ArtifactsTabContent: React.FC<{
       setProgress(0);
       setViewerConfigs([]);
 
-      if (!workflow.status || !workflow.status.nodes) {
+      if (!nodeStatus || !nodeCompleted) {
         setProgress(100); // Loaded will be set by Progress onComplete
-        return;
+        return; // Abort, because there is no data.
       }
       // Load runtime outputs from the selected Node
-      const outputPaths = WorkflowParser.loadNodeOutputPaths(
-        workflow.status.nodes[selectedNodeDetails.id],
-      );
+      const outputPaths = WorkflowParser.loadNodeOutputPaths(nodeStatus);
       const reportProgress = (reportedProgress: number) => {
         if (!aborted) {
           setProgress(reportedProgress);
@@ -937,10 +942,9 @@ const ArtifactsTabContent: React.FC<{
 
       // Load the viewer configurations from the output paths
       const builtConfigs = (await Promise.all([
-        OutputArtifactLoader.buildTFXArtifactViewerConfig(
-          selectedNodeDetails.id,
-          reportProgress,
-        ).catch(reportErrorAndReturnEmpty),
+        OutputArtifactLoader.buildTFXArtifactViewerConfig(nodeId, reportProgress).catch(
+          reportErrorAndReturnEmpty,
+        ),
         ...outputPaths.map(path =>
           OutputArtifactLoader.load(path).catch(reportErrorAndReturnEmpty),
         ),
@@ -959,7 +963,12 @@ const ArtifactsTabContent: React.FC<{
       aborted = true;
     };
     return abort;
-  }, [workflow, selectedNodeDetails.id, onError]);
+    // Workaround:
+    // Watches nodeStatus.phase in completed status instead of nodeStatus,
+    // because nodeStatus data won't further change after completed, but
+    // nodeStatus object instance will keep changing after new requests to get
+    // workflow status.
+  }, [nodeId, nodeCompleted, onError]);
 
   return (
     <div className={commonCss.page}>

diff --git a/frontend/src/pages/__snapshots__/RunDetails.test.tsx.snap b/frontend/src/pages/__snapshots__/RunDetails.test.tsx.snap
@@ -1391,13 +1391,13 @@ exports[`RunDetails opens side panel when graph node is clicked 1`] = `
               >
                 <ArtifactsTabContent
                   generatedVisualizations={Array []}
-                  onError={[Function]}
-                  selectedNodeDetails={
+                  nodeId="node1"
+                  nodeStatus={
                     Object {
                       "id": "node1",
-                      "phaseMessage": undefined,
                     }
                   }
+                  onError={[Function]}
                   visualizationCreatorConfig={
                     Object {
                       "allowCustomVisualizations": false,
@@ -1406,17 +1406,6 @@ exports[`RunDetails opens side panel when graph node is clicked 1`] = `
                       "type": "visualization-creator",
                     }
                   }
-                  workflow={
-                    Object {
-                      "status": Object {
-                        "nodes": Object {
-                          "node1": Object {
-                            "id": "node1",
-                          },
-                        },
-                      },
-                    }
-                  }
                 />
               </div>
             </div>
@@ -1643,13 +1632,15 @@ exports[`RunDetails shows clicked node message in side panel 1`] = `
               >
                 <ArtifactsTabContent
                   generatedVisualizations={Array []}
-                  onError={[Function]}
-                  selectedNodeDetails={
+                  nodeId="node1"
+                  nodeStatus={
                     Object {
                       "id": "node1",
-                      "phaseMessage": "This step is in Succeeded state with this message: some test message",
+                      "message": "some test message",
+                      "phase": "Succeeded",
                     }
                   }
+                  onError={[Function]}
                   visualizationCreatorConfig={
                     Object {
                       "allowCustomVisualizations": false,
@@ -1658,19 +1649,6 @@ exports[`RunDetails shows clicked node message in side panel 1`] = `
                       "type": "visualization-creator",
                     }
                   }
-                  workflow={
-                    Object {
-                      "status": Object {
-                        "nodes": Object {
-                          "node1": Object {
-                            "id": "node1",
-                            "message": "some test message",
-                            "phase": "Succeeded",
-                          },
-                        },
-                      },
-                    }
-                  }
                 />
               </div>
             </div>