[Dashboard] Refactor dialogs to use parent component state instead of routes (#7129)

python/ray/dashboard/client/src/App.tsx

@@ -3,8 +3,6 @@ import React from "react";
 import { Provider } from "react-redux";
 import { BrowserRouter, Route } from "react-router-dom";
 import Dashboard from "./pages/dashboard/Dashboard";
-import Errors from "./pages/dashboard/dialogs/errors/Errors";
-import Logs from "./pages/dashboard/dialogs/logs/Logs";
 import { store } from "./store";
 
 class App extends React.Component {
@@ -13,9 +11,7 @@ class App extends React.Component {
       <Provider store={store}>
         <BrowserRouter>
           <CssBaseline />
-          <Dashboard />
-          <Route component={Logs} path="/logs/:hostname/:pid?" />
-          <Route component={Errors} path="/errors/:hostname/:pid?" />
+          <Route component={Dashboard} exact path="/" />
         </BrowserRouter>
       </Provider>
     );

python/ray/dashboard/client/src/api.ts

@@ -143,15 +143,21 @@ export interface ErrorsResponse {
   }>;
 }
 
-export const getErrors = (hostname: string, pid: string | undefined) =>
-  get<ErrorsResponse>("/api/errors", { hostname, pid: pid || "" });
+export const getErrors = (hostname: string, pid: number | null) =>
+  get<ErrorsResponse>("/api/errors", {
+    hostname,
+    pid: pid === null ? "" : pid
+  });
 
 export interface LogsResponse {
   [pid: string]: string[];
 }
 
-export const getLogs = (hostname: string, pid: string | undefined) =>
-  get<LogsResponse>("/api/logs", { hostname, pid: pid || "" });
+export const getLogs = (hostname: string, pid: number | null) =>
+  get<LogsResponse>("/api/logs", {
+    hostname,
+    pid: pid === null ? "" : pid
+  });
 
 export type LaunchProfilingResponse = string;
 

python/ray/dashboard/client/src/common/SpanButton.tsx

@@ -0,0 +1,26 @@
+import { Theme } from "@material-ui/core/styles/createMuiTheme";
+import createStyles from "@material-ui/core/styles/createStyles";
+import withStyles, { WithStyles } from "@material-ui/core/styles/withStyles";
+import React, { HTMLAttributes } from "react";
+
+const styles = (theme: Theme) =>
+  createStyles({
+    button: {
+      color: theme.palette.primary.main,
+      "&:hover": {
+        cursor: "pointer",
+        textDecoration: "underline"
+      }
+    }
+  });
+
+class SpanButton extends React.Component<
+  HTMLAttributes<HTMLSpanElement> & WithStyles<typeof styles>
+> {
+  render() {
+    const { classes, ...otherProps } = this.props;
+    return <span className={classes.button} {...otherProps} />;
+  }
+}
+
+export default withStyles(styles)(SpanButton);

python/ray/dashboard/client/src/pages/dashboard/Dashboard.tsx

@@ -43,6 +43,8 @@ class Dashboard extends React.Component<
     ReturnType<typeof mapStateToProps> &
     typeof mapDispatchToProps
 > {
+  timeoutId = 0;
+
   refreshNodeAndRayletInfo = async () => {
     try {
       const [nodeInfo, rayletInfo, tuneAvailability] = await Promise.all([
@@ -56,7 +58,7 @@ class Dashboard extends React.Component<
     } catch (error) {
       this.props.setError(error.toString());
     } finally {
-      setTimeout(this.refreshNodeAndRayletInfo, 1000);
+      this.timeoutId = window.setTimeout(this.refreshNodeAndRayletInfo, 1000);
     }
   };
 
@@ -64,6 +66,10 @@ class Dashboard extends React.Component<
     await this.refreshNodeAndRayletInfo();
   }
 
+  componentWillUnmount() {
+    clearTimeout(this.timeoutId);
+  }
+
   handleTabChange = (event: React.ChangeEvent<{}>, value: number) => {
     this.props.setTab(value);
   };

python/ray/dashboard/client/src/pages/dashboard/node-info/NodeInfo.tsx

@@ -10,6 +10,8 @@ import Typography from "@material-ui/core/Typography";
 import React from "react";
 import { connect } from "react-redux";
 import { StoreState } from "../../../store";
+import Errors from "./dialogs/errors/Errors";
+import Logs from "./dialogs/logs/Logs";
 import NodeRowGroup from "./NodeRowGroup";
 import TotalRow from "./TotalRow";
 
@@ -32,11 +34,38 @@ const mapStateToProps = (state: StoreState) => ({
   rayletInfo: state.dashboard.rayletInfo
 });
 
+interface State {
+  logDialog: { hostname: string; pid: number | null } | null;
+  errorDialog: { hostname: string; pid: number | null } | null;
+}
+
 class NodeInfo extends React.Component<
   WithStyles<typeof styles> & ReturnType<typeof mapStateToProps>
 > {
+  state: State = {
+    logDialog: null,
+    errorDialog: null
+  };
+
+  setLogDialog = (hostname: string, pid: number | null) => {
+    this.setState({ logDialog: { hostname, pid } });
+  };
+
+  clearLogDialog = () => {
+    this.setState({ logDialog: null });
+  };
+
+  setErrorDialog = (hostname: string, pid: number | null) => {
+    this.setState({ errorDialog: { hostname, pid } });
+  };
+
+  clearErrorDialog = () => {
+    this.setState({ errorDialog: null });
+  };
+
   render() {
     const { classes, nodeInfo, rayletInfo } = this.props;
+    const { logDialog, errorDialog } = this.state;
 
     if (nodeInfo === null || rayletInfo === null) {
       return <Typography color="textSecondary">Loading...</Typography>;
@@ -88,44 +117,62 @@ class NodeInfo extends React.Component<
     }
 
     return (
-      <Table className={classes.table}>
-        <TableHead>
-          <TableRow>
-            <TableCell className={classes.cell} />
-            <TableCell className={classes.cell}>Host</TableCell>
-            <TableCell className={classes.cell}>Workers</TableCell>
-            <TableCell className={classes.cell}>Uptime</TableCell>
-            <TableCell className={classes.cell}>CPU</TableCell>
-            <TableCell className={classes.cell}>RAM</TableCell>
-            <TableCell className={classes.cell}>Disk</TableCell>
-            <TableCell className={classes.cell}>Sent</TableCell>
-            <TableCell className={classes.cell}>Received</TableCell>
-            <TableCell className={classes.cell}>Logs</TableCell>
-            <TableCell className={classes.cell}>Errors</TableCell>
-          </TableRow>
-        </TableHead>
-        <TableBody>
-          {nodeInfo.clients.map(client => (
-            <NodeRowGroup
-              key={client.ip}
-              node={client}
-              raylet={
-                client.ip in rayletInfo.nodes
-                  ? rayletInfo.nodes[client.ip]
-                  : null
-              }
-              logCounts={logCounts[client.ip]}
-              errorCounts={errorCounts[client.ip]}
-              initialExpanded={nodeInfo.clients.length <= 4}
+      <React.Fragment>
+        <Table className={classes.table}>
+          <TableHead>
+            <TableRow>
+              <TableCell className={classes.cell} />
+              <TableCell className={classes.cell}>Host</TableCell>
+              <TableCell className={classes.cell}>Workers</TableCell>
+              <TableCell className={classes.cell}>Uptime</TableCell>
+              <TableCell className={classes.cell}>CPU</TableCell>
+              <TableCell className={classes.cell}>RAM</TableCell>
+              <TableCell className={classes.cell}>Disk</TableCell>
+              <TableCell className={classes.cell}>Sent</TableCell>
+              <TableCell className={classes.cell}>Received</TableCell>
+              <TableCell className={classes.cell}>Logs</TableCell>
+              <TableCell className={classes.cell}>Errors</TableCell>
+            </TableRow>
+          </TableHead>
+          <TableBody>
+            {nodeInfo.clients.map(client => (
+              <NodeRowGroup
+                key={client.ip}
+                node={client}
+                raylet={
+                  client.ip in rayletInfo.nodes
+                    ? rayletInfo.nodes[client.ip]
+                    : null
+                }
+                logCounts={logCounts[client.ip]}
+                errorCounts={errorCounts[client.ip]}
+                setLogDialog={this.setLogDialog}
+                setErrorDialog={this.setErrorDialog}
+                initialExpanded={nodeInfo.clients.length <= 1}
+              />
+            ))}
+            <TotalRow
+              nodes={nodeInfo.clients}
+              logCounts={logCounts}
+              errorCounts={errorCounts}
             />
-          ))}
-          <TotalRow
-            nodes={nodeInfo.clients}
-            logCounts={logCounts}
-            errorCounts={errorCounts}
+          </TableBody>
+        </Table>
+        {logDialog !== null && (
+          <Logs
+            clearLogDialog={this.clearLogDialog}
+            hostname={logDialog.hostname}
+            pid={logDialog.pid}
           />
-        </TableBody>
-      </Table>
+        )}
+        {errorDialog !== null && (
+          <Errors
+            clearErrorDialog={this.clearErrorDialog}
+            hostname={errorDialog.hostname}
+            pid={errorDialog.pid}
+          />
+        )}
+      </React.Fragment>
     );
   }
 }

python/ray/dashboard/client/src/pages/dashboard/node-info/NodeRowGroup.tsx

@@ -56,6 +56,8 @@ interface Props {
     perWorker: { [pid: string]: number };
     total: number;
   };
+  setLogDialog: (hostname: string, pid: number | null) => void;
+  setErrorDialog: (hostname: string, pid: number | null) => void;
   initialExpanded: boolean;
 }
 
@@ -78,7 +80,15 @@ class NodeRowGroup extends React.Component<
   };
 
   render() {
-    const { classes, node, raylet, logCounts, errorCounts } = this.props;
+    const {
+      classes,
+      node,
+      raylet,
+      logCounts,
+      errorCounts,
+      setLogDialog,
+      setErrorDialog
+    } = this.props;
     const { expanded } = this.state;
 
     const features = [
@@ -91,12 +101,12 @@ class NodeRowGroup extends React.Component<
       { NodeFeature: NodeSent, WorkerFeature: WorkerSent },
       { NodeFeature: NodeReceived, WorkerFeature: WorkerReceived },
       {
-        NodeFeature: makeNodeLogs(logCounts),
-        WorkerFeature: makeWorkerLogs(logCounts)
+        NodeFeature: makeNodeLogs(logCounts, setLogDialog),
+        WorkerFeature: makeWorkerLogs(logCounts, setLogDialog)
       },
       {
-        NodeFeature: makeNodeErrors(errorCounts),
-        WorkerFeature: makeWorkerErrors(errorCounts)
+        NodeFeature: makeNodeErrors(errorCounts, setErrorDialog),
+        WorkerFeature: makeWorkerErrors(errorCounts, setErrorDialog)
       }
     ];
 

python/ray/dashboard/client/src/pages/dashboard/node-info/dialogs/errors/Errors.tsx

@@ -4,10 +4,9 @@ import createStyles from "@material-ui/core/styles/createStyles";
 import withStyles, { WithStyles } from "@material-ui/core/styles/withStyles";
 import Typography from "@material-ui/core/Typography";
 import React from "react";
-import { RouteComponentProps } from "react-router";
-import { ErrorsResponse, getErrors } from "../../../../api";
-import DialogWithTitle from "../../../../common/DialogWithTitle";
-import NumberedLines from "../../../../common/NumberedLines";
+import { ErrorsResponse, getErrors } from "../../../../../api";
+import DialogWithTitle from "../../../../../common/DialogWithTitle";
+import NumberedLines from "../../../../../common/NumberedLines";
 
 const styles = (theme: Theme) =>
   createStyles({
@@ -30,29 +29,26 @@ const styles = (theme: Theme) =>
     }
   });
 
+interface Props {
+  clearErrorDialog: () => void;
+  hostname: string;
+  pid: number | null;
+}
+
 interface State {
   result: ErrorsResponse | null;
   error: string | null;
 }
 
-class Errors extends React.Component<
-  WithStyles<typeof styles> &
-    RouteComponentProps<{ hostname: string; pid: string | undefined }>,
-  State
-> {
+class Errors extends React.Component<Props & WithStyles<typeof styles>, State> {
   state: State = {
     result: null,
     error: null
   };
 
-  handleClose = () => {
-    this.props.history.push("/");
-  };
-
   async componentDidMount() {
     try {
-      const { match } = this.props;
-      const { hostname, pid } = match.params;
+      const { hostname, pid } = this.props;
       const result = await getErrors(hostname, pid);
       this.setState({ result, error: null });
     } catch (error) {
@@ -61,13 +57,11 @@ class Errors extends React.Component<
   }
 
   render() {
-    const { classes, match } = this.props;
+    const { classes, clearErrorDialog, hostname } = this.props;
     const { result, error } = this.state;
 
-    const { hostname } = match.params;
-
     return (
-      <DialogWithTitle handleClose={this.handleClose} title="Errors">
+      <DialogWithTitle handleClose={clearErrorDialog} title="Errors">
         {error !== null ? (
           <Typography color="error">{error}</Typography>
         ) : result === null ? (

python/ray/dashboard/client/src/pages/dashboard/node-info/dialogs/logs/Logs.tsx

@@ -4,10 +4,9 @@ import createStyles from "@material-ui/core/styles/createStyles";
 import withStyles, { WithStyles } from "@material-ui/core/styles/withStyles";
 import Typography from "@material-ui/core/Typography";
 import React from "react";
-import { RouteComponentProps } from "react-router";
-import { getLogs, LogsResponse } from "../../../../api";
-import DialogWithTitle from "../../../../common/DialogWithTitle";
-import NumberedLines from "../../../../common/NumberedLines";
+import { getLogs, LogsResponse } from "../../../../../api";
+import DialogWithTitle from "../../../../../common/DialogWithTitle";
+import NumberedLines from "../../../../../common/NumberedLines";
 
 const styles = (theme: Theme) =>
   createStyles({
@@ -25,29 +24,26 @@ const styles = (theme: Theme) =>
     }
   });
 
+interface Props {
+  clearLogDialog: () => void;
+  hostname: string;
+  pid: number | null;
+}
+
 interface State {
   result: LogsResponse | null;
   error: string | null;
 }
 
-class Logs extends React.Component<
-  WithStyles<typeof styles> &
-    RouteComponentProps<{ hostname: string; pid: string | undefined }>,
-  State
-> {
+class Logs extends React.Component<Props & WithStyles<typeof styles>, State> {
   state: State = {
     result: null,
     error: null
   };
 
-  handleClose = () => {
-    this.props.history.push("/");
-  };
-
   async componentDidMount() {
     try {
-      const { match } = this.props;
-      const { hostname, pid } = match.params;
+      const { hostname, pid } = this.props;
       const result = await getLogs(hostname, pid);
       this.setState({ result, error: null });
     } catch (error) {
@@ -56,13 +52,11 @@ class Logs extends React.Component<
   }
 
   render() {
-    const { classes, match } = this.props;
+    const { classes, clearLogDialog, hostname } = this.props;
     const { result, error } = this.state;
 
-    const { hostname } = match.params;
-
     return (
-      <DialogWithTitle handleClose={this.handleClose} title="Logs">
+      <DialogWithTitle handleClose={clearLogDialog} title="Logs">
         {error !== null ? (
           <Typography color="error">{error}</Typography>
         ) : result === null ? (

python/ray/dashboard/client/src/pages/dashboard/node-info/features/Errors.tsx

@@ -1,7 +1,6 @@
-import Link from "@material-ui/core/Link";
 import Typography from "@material-ui/core/Typography";
 import React from "react";
-import { Link as RouterLink } from "react-router-dom";
+import SpanButton from "../../../../common/SpanButton";
 import {
   ClusterFeatureComponent,
   NodeFeatureComponent,
@@ -34,30 +33,36 @@ export const makeClusterErrors = (errorCounts: {
   );
 };
 
-export const makeNodeErrors = (errorCounts: {
-  perWorker: { [pid: string]: number };
-  total: number;
-}): NodeFeatureComponent => ({ node }) =>
+export const makeNodeErrors = (
+  errorCounts: {
+    perWorker: { [pid: string]: number };
+    total: number;
+  },
+  setErrorDialog: (hostname: string, pid: number | null) => void
+): NodeFeatureComponent => ({ node }) =>
   errorCounts.total === 0 ? (
     <Typography color="textSecondary" component="span" variant="inherit">
       No errors
     </Typography>
   ) : (
-    <Link component={RouterLink} to={`/errors/${node.hostname}`}>
+    <SpanButton onClick={() => setErrorDialog(node.hostname, null)}>
       View all errors ({errorCounts.total.toLocaleString()})
-    </Link>
+    </SpanButton>
   );
 
-export const makeWorkerErrors = (errorCounts: {
-  perWorker: { [pid: string]: number };
-  total: number;
-}): WorkerFeatureComponent => ({ node, worker }) =>
+export const makeWorkerErrors = (
+  errorCounts: {
+    perWorker: { [pid: string]: number };
+    total: number;
+  },
+  setErrorDialog: (hostname: string, pid: number | null) => void
+): WorkerFeatureComponent => ({ node, worker }) =>
   errorCounts.perWorker[worker.pid] === 0 ? (
     <Typography color="textSecondary" component="span" variant="inherit">
       No errors
     </Typography>
   ) : (
-    <Link component={RouterLink} to={`/errors/${node.hostname}/${worker.pid}`}>
+    <SpanButton onClick={() => setErrorDialog(node.hostname, worker.pid)}>
       View errors ({errorCounts.perWorker[worker.pid].toLocaleString()})
-    </Link>
+    </SpanButton>
   );

python/ray/dashboard/client/src/pages/dashboard/node-info/features/Logs.tsx

@@ -1,7 +1,6 @@
-import Link from "@material-ui/core/Link";
 import Typography from "@material-ui/core/Typography";
 import React from "react";
-import { Link as RouterLink } from "react-router-dom";
+import SpanButton from "../../../../common/SpanButton";
 import {
   ClusterFeatureComponent,
   NodeFeatureComponent,
@@ -33,32 +32,38 @@ export const makeClusterLogs = (logCounts: {
   );
 };
 
-export const makeNodeLogs = (logCounts: {
-  perWorker: { [pid: string]: number };
-  total: number;
-}): NodeFeatureComponent => ({ node }) =>
+export const makeNodeLogs = (
+  logCounts: {
+    perWorker: { [pid: string]: number };
+    total: number;
+  },
+  setLogDialog: (hostname: string, pid: number | null) => void
+): NodeFeatureComponent => ({ node }) =>
   logCounts.total === 0 ? (
     <Typography color="textSecondary" component="span" variant="inherit">
       No logs
     </Typography>
   ) : (
-    <Link component={RouterLink} to={`/logs/${node.hostname}`}>
+    <SpanButton onClick={() => setLogDialog(node.hostname, null)}>
       View all logs ({logCounts.total.toLocaleString()}{" "}
       {logCounts.total === 1 ? "line" : "lines"})
-    </Link>
+    </SpanButton>
   );
 
-export const makeWorkerLogs = (logCounts: {
-  perWorker: { [pid: string]: number };
-  total: number;
-}): WorkerFeatureComponent => ({ node, worker }) =>
+export const makeWorkerLogs = (
+  logCounts: {
+    perWorker: { [pid: string]: number };
+    total: number;
+  },
+  setLogDialog: (hostname: string, pid: number | null) => void
+): WorkerFeatureComponent => ({ node, worker }) =>
   logCounts.perWorker[worker.pid] === 0 ? (
     <Typography color="textSecondary" component="span" variant="inherit">
       No logs
     </Typography>
   ) : (
-    <Link component={RouterLink} to={`/logs/${node.hostname}/${worker.pid}`}>
+    <SpanButton onClick={() => setLogDialog(node.hostname, worker.pid)}>
       View log ({logCounts.perWorker[worker.pid].toLocaleString()}{" "}
       {logCounts.perWorker[worker.pid] === 1 ? "line" : "lines"})
-    </Link>
+    </SpanButton>
   );