From 7bc893cfdf5eeb1f690e085aefea94c754296d44 Mon Sep 17 00:00:00 2001 From: Alan Guo Date: Mon, 20 Mar 2023 19:18:20 -0700 Subject: [PATCH 1/8] [Dashboard] Add Replicas to Serve Dashboard UI Signed-off-by: Alan Guo --- dashboard/client/src/App.tsx | 18 +- .../client/src/components/StatusChip.tsx | 14 +- .../client/src/pages/layout/MainNavLayout.tsx | 11 +- .../serve/ServeApplicationDetailPage.tsx | 66 ++++--- .../pages/serve/ServeApplicationsListPage.tsx | 6 + .../src/pages/serve/ServeDeploymentRow.tsx | 141 +++++++++++++-- .../pages/serve/ServeReplicaDetailPage.tsx | 165 ++++++++++++++++++ .../pages/serve/hook/useServeApplications.ts | 40 ++++- dashboard/client/src/type/serve.ts | 36 +++- python/ray/serve/_private/common.py | 1 + python/ray/serve/config.py | 1 + 11 files changed, 448 insertions(+), 51 deletions(-) create mode 100644 dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx diff --git a/dashboard/client/src/App.tsx b/dashboard/client/src/App.tsx index ec2d769602f2..11c13713d744 100644 --- a/dashboard/client/src/App.tsx +++ b/dashboard/client/src/App.tsx @@ -21,9 +21,13 @@ import { ClusterDetailInfoPage } from "./pages/node/ClusterDetailInfoPage"; import { ClusterLayout } from "./pages/node/ClusterLayout"; import NodeDetailPage from "./pages/node/NodeDetail"; import { OverviewPage } from "./pages/overview/OverviewPage"; -import { ServeApplicationDetailPage } from "./pages/serve/ServeApplicationDetailPage"; +import { + ServeApplicationDetailLayout, + ServeApplicationDetailPage, +} from "./pages/serve/ServeApplicationDetailPage"; import { ServeApplicationsListPage } from "./pages/serve/ServeApplicationsListPage"; import { ServeLayout } from "./pages/serve/ServeLayout"; +import { ServeReplicaDetailPage } from "./pages/serve/ServeReplicaDetailPage"; import { getNodeList } from "./service/node"; import { lightTheme } from "./theme"; @@ -191,9 +195,15 @@ const App = () => { } path="serve"> } path="" /> } - path="applications/:name" - /> + element={} + path="applications/:applicationName" + > + } path="" /> + } + path=":deploymentName/:replicaId" + /> + } path="logs"> {/* TODO(aguo): Refactor Logs component to use optional query diff --git a/dashboard/client/src/components/StatusChip.tsx b/dashboard/client/src/components/StatusChip.tsx index 5fb03eaf677a..ab4869db7998 100644 --- a/dashboard/client/src/components/StatusChip.tsx +++ b/dashboard/client/src/components/StatusChip.tsx @@ -6,6 +6,7 @@ import { green, grey, lightBlue, + orange, red, yellow, } from "@material-ui/core/colors"; @@ -13,7 +14,11 @@ import { CSSProperties } from "@material-ui/core/styles/withStyles"; import React, { ReactNode } from "react"; import { ActorEnum } from "../type/actor"; import { PlacementGroupState } from "../type/placementGroup"; -import { ServeApplicationStatus, ServeDeploymentStatus } from "../type/serve"; +import { + ServeApplicationStatus, + ServeDeploymentStatus, + ServeReplicaState, +} from "../type/serve"; import { TypeTaskStatus } from "../type/task"; const colorMap = { @@ -70,6 +75,13 @@ const colorMap = { [ServeDeploymentStatus.HEALTHY]: green, [ServeDeploymentStatus.UNHEALTHY]: red, }, + serveReplica: { + [ServeReplicaState.STARTING]: yellow, + [ServeReplicaState.UPDATING]: yellow, + [ServeReplicaState.RECOVERING]: orange, + [ServeReplicaState.RUNNING]: green, + [ServeReplicaState.STOPPING]: red, + }, } as { [key: string]: { [key: string]: Color | string; diff --git a/dashboard/client/src/pages/layout/MainNavLayout.tsx b/dashboard/client/src/pages/layout/MainNavLayout.tsx index c7e3a3b225b4..35da4906b8fd 100644 --- a/dashboard/client/src/pages/layout/MainNavLayout.tsx +++ b/dashboard/client/src/pages/layout/MainNavLayout.tsx @@ -141,12 +141,11 @@ const NAV_ITEMS = [ path: "/jobs", id: "jobs", }, - // TODO(aguo): Add this nav button in once the page is fully implemented. - // { - // title: "Serve", - // path: "/serve", - // id: "serve", - // }, + { + title: "Serve", + path: "/serve", + id: "serve", + }, { title: "Cluster", path: "/cluster", diff --git a/dashboard/client/src/pages/serve/ServeApplicationDetailPage.tsx b/dashboard/client/src/pages/serve/ServeApplicationDetailPage.tsx index 4fa0e039e77b..94ebeafa94b8 100644 --- a/dashboard/client/src/pages/serve/ServeApplicationDetailPage.tsx +++ b/dashboard/client/src/pages/serve/ServeApplicationDetailPage.tsx @@ -15,7 +15,7 @@ import { } from "@material-ui/core"; import { Autocomplete, Pagination } from "@material-ui/lab"; import React, { ReactElement } from "react"; -import { useParams } from "react-router-dom"; +import { Outlet, useParams } from "react-router-dom"; import { CodeDialogButton } from "../../common/CodeDialogButton"; import { CollapsibleSection } from "../../common/CollapsibleSection"; import { DurationText } from "../../common/DurationText"; @@ -39,16 +39,19 @@ const useStyles = makeStyles((theme) => ); const columns: { label: string; helpInfo?: ReactElement; width?: string }[] = [ + { label: "" }, // For expand/collapse button { label: "Name" }, { label: "Status" }, { label: "Status message", width: "30%" }, + { label: "Actions" }, + { label: "Last deployed at" }, + { label: "Duration" }, { label: "Replicas" }, - { label: "Deployment config" }, ]; export const ServeApplicationDetailPage = () => { const classes = useStyles(); - const { name } = useParams(); + const { applicationName } = useParams(); const { application, @@ -57,12 +60,12 @@ export const ServeApplicationDetailPage = () => { setPage, changeFilter, allDeployments, - } = useServeApplicationDetails(name); + } = useServeApplicationDetails(applicationName); if (!application) { return ( - Application with name "{name}" not found. + Application with name "{applicationName}" not found. ); } @@ -70,20 +73,6 @@ export const ServeApplicationDetailPage = () => { const appName = application.name ? application.name : "-"; return (
- {/* Extra MainNavPageInfo to add an extra layer of nesting in breadcrumbs */} - - { }, ]} /> - +
{ ))} @@ -237,3 +227,39 @@ export const ServeApplicationDetailPage = () => {
); }; + +export const ServeApplicationDetailLayout = () => { + const { applicationName } = useParams(); + + const { application } = useServeApplicationDetails(applicationName); + + if (!application) { + return ( + + Application with name "{applicationName}" not found. + + ); + } + + const appName = application.name ? application.name : "-"; + + return ( + + {/* Extra MainNavPageInfo to add an extra layer of nesting in breadcrumbs */} + + + + + ); +}; diff --git a/dashboard/client/src/pages/serve/ServeApplicationsListPage.tsx b/dashboard/client/src/pages/serve/ServeApplicationsListPage.tsx index 21d8bfb228dd..1dd67cfcb4f0 100644 --- a/dashboard/client/src/pages/serve/ServeApplicationsListPage.tsx +++ b/dashboard/client/src/pages/serve/ServeApplicationsListPage.tsx @@ -87,6 +87,12 @@ export const ServeApplicationsListPage = () => { value: `${serveDetails.port}`, }, }, + { + label: "Proxy location", + content: { + value: `${serveDetails.proxy_location}`, + }, + }, ]} /> ) : ( diff --git a/dashboard/client/src/pages/serve/ServeDeploymentRow.tsx b/dashboard/client/src/pages/serve/ServeDeploymentRow.tsx index b6b8821e1ad2..ae5e99bb5e69 100644 --- a/dashboard/client/src/pages/serve/ServeDeploymentRow.tsx +++ b/dashboard/client/src/pages/serve/ServeDeploymentRow.tsx @@ -1,42 +1,149 @@ -import { TableCell, TableRow } from "@material-ui/core"; -import React from "react"; +import { + createStyles, + makeStyles, + TableCell, + TableRow, +} from "@material-ui/core"; +import AddIcon from "@material-ui/icons/Add"; +import RemoveIcon from "@material-ui/icons/Remove"; +import React, { useState } from "react"; +import { Link } from "react-router-dom"; import { CodeDialogButton, CodeDialogButtonWithPreview, } from "../../common/CodeDialogButton"; +import { DurationText } from "../../common/DurationText"; +import { formatDateFromTimeMs } from "../../common/formatUtils"; import { StatusChip } from "../../components/StatusChip"; -import { ServeDeployment } from "../../type/serve"; +import { + ServeApplication, + ServeDeployment, + ServeReplica, +} from "../../type/serve"; +import { ServeReplicaLogsLink } from "./ServeReplicaDetailPage"; + +const useStyles = makeStyles((theme) => + createStyles({ + deploymentName: { + fontWeight: 500, + }, + expandCollapseIcon: { + color: theme.palette.text.secondary, + fontSize: "1.5em", + verticalAlign: "middle", + }, + }), +); export type ServeDeployentRowProps = { deployment: ServeDeployment; + application: ServeApplication; }; export const ServeDeploymentRow = ({ - deployment: { name, status, message, deployment_config }, + deployment, + application: { last_deployed_time_s }, }: ServeDeployentRowProps) => { + const { name, status, message, deployment_config, replicas } = deployment; + + const classes = useStyles(); + + const [expanded, setExpanded] = useState(false); + + return ( + + + { + setExpanded(!expanded); + }} + > + {!expanded ? ( + + ) : ( + + )} + + + {name} + + + + + + {message ? ( + + ) : ( + "-" + )} + + + + + + {formatDateFromTimeMs(last_deployed_time_s * 1000)} + + + + + {Object.keys(replicas).length} + + {expanded && + replicas.map((replica) => ( + + ))} + + ); +}; + +export type ServeReplicaRowProps = { + replica: ServeReplica; + deployment: ServeDeployment; +}; + +export const ServeReplicaRow = ({ + replica, + deployment, +}: ServeReplicaRowProps) => { + const { replica_id, state, start_time_s } = replica; + const { name } = deployment; + return ( - {name} + + + + {replica_id} + + - + + - - {message ? ( - - ) : ( - "-" - )} + - {/* TODO(aguo): Add number of replicas (instead of target number of replicas) once available from API */} - {deployment_config.num_replicas} + {formatDateFromTimeMs(start_time_s * 1000)} - + + - ); }; diff --git a/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx new file mode 100644 index 000000000000..f20f7aa1551d --- /dev/null +++ b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx @@ -0,0 +1,165 @@ +import { Link, Typography } from "@material-ui/core"; +import React, { useContext } from "react"; +import { Link as RouterLink, useParams } from "react-router-dom"; +import { GlobalContext } from "../../App"; +import { DurationText } from "../../common/DurationText"; +import { formatDateFromTimeMs } from "../../common/formatUtils"; +import { MetadataSection } from "../../components/MetadataSection"; +import { StatusChip } from "../../components/StatusChip"; +import { ServeDeployment, ServeReplica } from "../../type/serve"; +import { MainNavPageInfo } from "../layout/mainNavContext"; +import { useServeReplicaDetails } from "./hook/useServeApplications"; + +export const ServeReplicaDetailPage = () => { + const { applicationName, deploymentName, replicaId } = useParams(); + + const { application, deployment, replica, error } = useServeReplicaDetails( + applicationName, + deploymentName, + replicaId, + ); + + if (error) { + return {error.toString()}; + } + + if (!replica || !deployment || !application) { + return ( + + {applicationName} / {deploymentName} / {replicaId} not found. + + ); + } + + const appName = application.name ? application.name : "-"; + const { + replica_id, + state, + actor_id, + actor_name, + node_id, + node_ip, + pid, + start_time_s, + } = replica; + return ( +
+ {/* Extra MainNavPageInfo to add an extra layer of nesting in breadcrumbs */} + + + , + }, + { + label: "Logs", + content: ( + + ), + }, + { + label: "Actor ID", + content: { + value: actor_id ? actor_id : "-", + copyableValue: actor_id ? actor_id : undefined, + }, + }, + { + label: "Actor name", + content: { + value: actor_name, + copyableValue: actor_name, + }, + }, + { + label: "Node ID", + content: { + value: node_id ? node_id : "-", + copyableValue: node_id ? node_id : undefined, + }, + }, + { + label: "Node IP", + content: { + value: node_ip ? node_ip : "-", + copyableValue: node_ip ? node_ip : undefined, + }, + }, + { + label: "PID", + content: { + value: pid ? pid : "-", + copyableValue: pid ? pid : undefined, + }, + }, + { + label: "Started at", + content: { + value: formatDateFromTimeMs(start_time_s * 1000), + }, + }, + { + label: "Duration", + content: , + }, + ]} + /> +
+ ); +}; + +export type ServeReplicaLogsLinkProps = { + replica: ServeReplica; + deployment: ServeDeployment; +}; + +export const ServeReplicaLogsLink = ({ + replica: { replica_id, node_ip }, + deployment: { name: deploymentName }, +}: ServeReplicaLogsLinkProps) => { + const { ipLogMap } = useContext(GlobalContext); + + let link: string | undefined; + + if (node_ip && ipLogMap[node_ip]) { + // TODO(aguo): Have API return the location of the logs. + const path = `/serve/deployment_${deploymentName}_${replica_id}.log`; + link = `/logs/${encodeURIComponent(ipLogMap[node_ip])}/${encodeURIComponent( + path, + )}`; + } + + if (link) { + return ( + + Log + + ); + } + + return -; +}; diff --git a/dashboard/client/src/pages/serve/hook/useServeApplications.ts b/dashboard/client/src/pages/serve/hook/useServeApplications.ts index 321dae525efe..7d0c2540fa4b 100644 --- a/dashboard/client/src/pages/serve/hook/useServeApplications.ts +++ b/dashboard/client/src/pages/serve/hook/useServeApplications.ts @@ -35,7 +35,9 @@ export const useServeApplications = () => { { refreshInterval: API_REFRESH_INTERVAL_MS }, ); - const serveDetails = data ? { host: data.host, port: data.port } : undefined; + const serveDetails = data + ? { ...data.http_options, proxy_location: data.proxy_location } + : undefined; const serveApplicationsList = data ? Object.values(data.applications).sort( (a, b) => (b.last_deployed_time_s ?? 0) - (a.last_deployed_time_s ?? 0), @@ -118,3 +120,39 @@ export const useServeApplicationDetails = ( allDeployments: deployments, }; }; + +export const useServeReplicaDetails = ( + applicationName: string | undefined, + deploymentName: string | undefined, + replicaId: string | undefined, +) => { + // TODO(aguo): Use a fetch by replicaId endpoint? + const { data, error } = useSWR( + "useServeReplicaDetails", + async () => { + const rsp = await getServeApplications(); + + if (rsp) { + return rsp.data; + } + }, + { refreshInterval: API_REFRESH_INTERVAL_MS }, + ); + + const application = applicationName + ? data?.applications?.[applicationName !== "-" ? applicationName : ""] + : undefined; + const deployment = deploymentName + ? application?.deployments[deploymentName] + : undefined; + const replica = deployment?.replicas.find( + ({ replica_id }) => replica_id === replicaId, + ); + + return { + application, + deployment, + replica, + error, + }; +}; diff --git a/dashboard/client/src/type/serve.ts b/dashboard/client/src/type/serve.ts index d72f4b4ed039..839613ec548f 100644 --- a/dashboard/client/src/type/serve.ts +++ b/dashboard/client/src/type/serve.ts @@ -32,6 +32,7 @@ export type ServeDeployment = { status: ServeDeploymentStatus; message: string; deployment_config: ServeDeploymentConfig; + replicas: ServeReplica[]; }; export type ServeDeploymentConfig = { @@ -55,9 +56,40 @@ export type ServeAutoscalingConfig = { target_num_ongoing_requests_per_replica: number; }; +export enum ServeReplicaState { + // Keep in sync with ReplicaState in python/ray/serve/_private/common.py + STARTING = "STARTING", + UPDATING = "UPDATING", + RECOVERING = "RECOVERING", + RUNNING = "RUNNING", + STOPPING = "STOPPING", +} + +export type ServeReplica = { + replica_id: string; + state: ServeReplicaState; + pid: string | null; + actor_name: string; + actor_id: string | null; + node_id: string | null; + node_ip: string | null; + start_time_s: number; +}; + +// Keep in sync with DeploymentMode in python/ray/serve/config.py +export enum ServeDeploymentMode { + NoServer = "NoServer", + HeadOnly = "HeadOnly", + EveryNode = "EveryNode", + FixedNumber = "FixedNumber", +} + export type ServeApplicationsRsp = { - host: string; - port: number; + http_options: { + host: string; + port: number; + }; + proxy_location: ServeDeploymentMode; applications: { [name: string]: ServeApplication; }; diff --git a/python/ray/serve/_private/common.py b/python/ray/serve/_private/common.py index 889ce578ff57..7ccfbce34fa8 100644 --- a/python/ray/serve/_private/common.py +++ b/python/ray/serve/_private/common.py @@ -28,6 +28,7 @@ class EndpointInfo: route: str +# Keep in sync with ServeReplicaState in dashboard/client/src/type/serve.ts class ReplicaState(str, Enum): STARTING = "STARTING" UPDATING = "UPDATING" diff --git a/python/ray/serve/config.py b/python/ray/serve/config.py index 93d9b85bd875..1e4029cce9cc 100644 --- a/python/ray/serve/config.py +++ b/python/ray/serve/config.py @@ -514,6 +514,7 @@ def to_proto_bytes(self): return self.to_proto().SerializeToString() +# Keep in sync with ServeDeploymentMode in dashboard/client/src/type/serve.ts @DeveloperAPI class DeploymentMode(str, Enum): NoServer = "NoServer" From b55faf0ccaf2379850391efb50796f0354081624 Mon Sep 17 00:00:00 2001 From: Alan Guo Date: Mon, 20 Mar 2023 21:43:43 -0700 Subject: [PATCH 2/8] fixup Signed-off-by: Alan Guo --- dashboard/client/src/pages/layout/MainNavLayout.tsx | 1 + .../src/pages/serve/ServeApplicationDetailPage.tsx | 8 ++++++-- .../src/pages/serve/ServeReplicaDetailPage.tsx | 12 ++++++------ .../src/pages/serve/hook/useServeApplications.ts | 6 ++++++ 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/dashboard/client/src/pages/layout/MainNavLayout.tsx b/dashboard/client/src/pages/layout/MainNavLayout.tsx index 35da4906b8fd..a5a8f9a0154b 100644 --- a/dashboard/client/src/pages/layout/MainNavLayout.tsx +++ b/dashboard/client/src/pages/layout/MainNavLayout.tsx @@ -245,6 +245,7 @@ const useMainNavBreadcrumbsStyles = makeStyles((theme) => }, breadcrumbItem: { fontWeight: 500, + color: "#8C9196", "&:not(:first-child)": { marginLeft: theme.spacing(1), }, diff --git a/dashboard/client/src/pages/serve/ServeApplicationDetailPage.tsx b/dashboard/client/src/pages/serve/ServeApplicationDetailPage.tsx index 94ebeafa94b8..64eaee02b608 100644 --- a/dashboard/client/src/pages/serve/ServeApplicationDetailPage.tsx +++ b/dashboard/client/src/pages/serve/ServeApplicationDetailPage.tsx @@ -20,9 +20,11 @@ import { CodeDialogButton } from "../../common/CodeDialogButton"; import { CollapsibleSection } from "../../common/CollapsibleSection"; import { DurationText } from "../../common/DurationText"; import { formatDateFromTimeMs } from "../../common/formatUtils"; +import Loading from "../../components/Loading"; import { MetadataSection } from "../../components/MetadataSection"; import { StatusChip } from "../../components/StatusChip"; import { HelpInfo } from "../../components/Tooltip"; +import { ServeApplication } from "../../type/serve"; import { MainNavPageInfo } from "../layout/mainNavContext"; import { useServeApplicationDetails } from "./hook/useServeApplications"; import { ServeDeploymentRow } from "./ServeDeploymentRow"; @@ -231,9 +233,11 @@ export const ServeApplicationDetailPage = () => { export const ServeApplicationDetailLayout = () => { const { applicationName } = useParams(); - const { application } = useServeApplicationDetails(applicationName); + const { loading, application } = useServeApplicationDetails(applicationName); - if (!application) { + if (loading) { + return ; + } else if (!application) { return ( Application with name "{applicationName}" not found. diff --git a/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx index f20f7aa1551d..e350595716c1 100644 --- a/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx +++ b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx @@ -4,6 +4,7 @@ import { Link as RouterLink, useParams } from "react-router-dom"; import { GlobalContext } from "../../App"; import { DurationText } from "../../common/DurationText"; import { formatDateFromTimeMs } from "../../common/formatUtils"; +import Loading from "../../components/Loading"; import { MetadataSection } from "../../components/MetadataSection"; import { StatusChip } from "../../components/StatusChip"; import { ServeDeployment, ServeReplica } from "../../type/serve"; @@ -13,17 +14,16 @@ import { useServeReplicaDetails } from "./hook/useServeApplications"; export const ServeReplicaDetailPage = () => { const { applicationName, deploymentName, replicaId } = useParams(); - const { application, deployment, replica, error } = useServeReplicaDetails( - applicationName, - deploymentName, - replicaId, - ); + const { loading, application, deployment, replica, error } = + useServeReplicaDetails(applicationName, deploymentName, replicaId); if (error) { return {error.toString()}; } - if (!replica || !deployment || !application) { + if (loading) { + return ; + } else if (!replica || !deployment || !application) { return ( {applicationName} / {deploymentName} / {replicaId} not found. diff --git a/dashboard/client/src/pages/serve/hook/useServeApplications.ts b/dashboard/client/src/pages/serve/hook/useServeApplications.ts index 7d0c2540fa4b..4ab54eb23158 100644 --- a/dashboard/client/src/pages/serve/hook/useServeApplications.ts +++ b/dashboard/client/src/pages/serve/hook/useServeApplications.ts @@ -103,7 +103,10 @@ export const useServeApplicationDetails = ( ) : []; + // Need to expose loading because it's not clear if undefined values + // for application means loading or missing data. return { + loading: !data && !error, application, filteredDeployments: deployments.filter((deployment) => filter.every((f) => @@ -149,7 +152,10 @@ export const useServeReplicaDetails = ( ({ replica_id }) => replica_id === replicaId, ); + // Need to expose loading because it's not clear if undefined values + // for application, deployment, or replica means loading or missing data. return { + loading: !data && !error, application, deployment, replica, From b0ff53d89fe0e1be63113394fa408adc322f6216 Mon Sep 17 00:00:00 2001 From: Alan Guo Date: Mon, 20 Mar 2023 21:47:47 -0700 Subject: [PATCH 3/8] fixup Signed-off-by: Alan Guo --- .../client/src/pages/serve/ServeApplicationDetailPage.tsx | 7 ++++--- .../serve/ServeApplicationsListPage.component.test.tsx | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/dashboard/client/src/pages/serve/ServeApplicationDetailPage.tsx b/dashboard/client/src/pages/serve/ServeApplicationDetailPage.tsx index 64eaee02b608..565de06e181b 100644 --- a/dashboard/client/src/pages/serve/ServeApplicationDetailPage.tsx +++ b/dashboard/client/src/pages/serve/ServeApplicationDetailPage.tsx @@ -24,7 +24,6 @@ import Loading from "../../components/Loading"; import { MetadataSection } from "../../components/MetadataSection"; import { StatusChip } from "../../components/StatusChip"; import { HelpInfo } from "../../components/Tooltip"; -import { ServeApplication } from "../../type/serve"; import { MainNavPageInfo } from "../layout/mainNavContext"; import { useServeApplicationDetails } from "./hook/useServeApplications"; import { ServeDeploymentRow } from "./ServeDeploymentRow"; @@ -104,8 +103,10 @@ export const ServeApplicationDetailPage = () => { { label: "Replicas", content: { - // TODO (aguo): Add number of replicas across all deployments once available in the UI - value: "0", + value: Object.values(application.deployments) + .map(({ replicas }) => replicas.length) + .reduce((acc, curr) => acc + curr) + .toString(), }, }, { diff --git a/dashboard/client/src/pages/serve/ServeApplicationsListPage.component.test.tsx b/dashboard/client/src/pages/serve/ServeApplicationsListPage.component.test.tsx index f39e5fe01d85..3dd76457a58d 100644 --- a/dashboard/client/src/pages/serve/ServeApplicationsListPage.component.test.tsx +++ b/dashboard/client/src/pages/serve/ServeApplicationsListPage.component.test.tsx @@ -2,7 +2,7 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import React from "react"; import { getServeApplications } from "../../service/serve"; -import { ServeApplicationStatus } from "../../type/serve"; +import { ServeApplicationStatus, ServeDeploymentMode } from "../../type/serve"; import { TEST_APP_WRAPPER } from "../../util/test-utils"; import { ServeApplicationsListPage } from "./ServeApplicationsListPage"; @@ -16,8 +16,8 @@ describe("ServeApplicationsListPage", () => { mockGetServeApplications.mockResolvedValue({ data: { - host: "1.2.3.4", - port: 8000, + http_options: { host: "1.2.3.4", port: 8000 }, + proxy_location: ServeDeploymentMode.EveryNode, applications: { home: { name: "home", From e25a8e825c61db0f06c34016c271528dbc0865ce Mon Sep 17 00:00:00 2001 From: Alan Guo Date: Tue, 21 Mar 2023 13:04:24 -0700 Subject: [PATCH 4/8] Address comments and add tests Also add links to actor detail and node detail page to everywhere we show actor id and node id Signed-off-by: Alan Guo --- dashboard/client/src/common/links.tsx | 68 ++++++++++++ .../client/src/components/ActorTable.tsx | 20 ++-- .../MetadataSection.component.test.tsx | 31 +++++- .../MetadataSection/MetadataSection.tsx | 15 ++- dashboard/client/src/components/TaskTable.tsx | 9 +- .../client/src/pages/actor/ActorDetail.tsx | 4 + dashboard/client/src/pages/node/NodeRow.tsx | 18 +-- ...veApplicationDetailPage.component.test.tsx | 80 +++++++++++-- .../serve/ServeApplicationDetailPage.tsx | 11 +- .../src/pages/serve/ServeDeploymentRow.tsx | 54 +++++---- .../ServeReplicaDetailPage.component.test.tsx | 105 ++++++++++++++++++ .../pages/serve/ServeReplicaDetailPage.tsx | 10 +- 12 files changed, 349 insertions(+), 76 deletions(-) create mode 100644 dashboard/client/src/common/links.tsx create mode 100644 dashboard/client/src/pages/serve/ServeReplicaDetailPage.component.test.tsx diff --git a/dashboard/client/src/common/links.tsx b/dashboard/client/src/common/links.tsx new file mode 100644 index 000000000000..850cedf33cb3 --- /dev/null +++ b/dashboard/client/src/common/links.tsx @@ -0,0 +1,68 @@ +import { Link } from "@material-ui/core"; +import React, { PropsWithChildren } from "react"; +import { Link as RouterLink } from "react-router-dom"; +import { ClassNameProps } from "./props"; + +type ActorLinkProps = PropsWithChildren< + { + actorId: string; + /** + * This can be provided to override where we link to. + */ + to?: string; + } & ClassNameProps +>; + +export const generateActorLink = (actorId: string) => `/actors/${actorId}`; + +/** + * A link to the top-level actors detail page. + */ +export const ActorLink = ({ + actorId, + to, + children, + className, +}: ActorLinkProps) => { + return ( + + {children ?? actorId} + + ); +}; + +type NodeLinkProps = PropsWithChildren< + { + nodeId: string; + /** + * This can be provided to override where we link to. + */ + to?: string; + } & ClassNameProps +>; + +export const generateNodeLink = (nodeId: string) => `/cluster/nodes/${nodeId}`; + +/** + * A link to the top-level Cluster node detail page. + */ +export const NodeLink = ({ + nodeId, + to, + children, + className, +}: NodeLinkProps) => { + return ( + + {children ?? nodeId} + + ); +}; diff --git a/dashboard/client/src/components/ActorTable.tsx b/dashboard/client/src/components/ActorTable.tsx index 7e31e6d1375a..cfa90149c1cd 100644 --- a/dashboard/client/src/components/ActorTable.tsx +++ b/dashboard/client/src/components/ActorTable.tsx @@ -19,6 +19,7 @@ import React, { useContext, useState } from "react"; import { Link } from "react-router-dom"; import { GlobalContext } from "../App"; import { DurationText } from "../common/DurationText"; +import { ActorLink } from "../common/links"; import { CpuProfilingLink, CpuStackTraceLink } from "../common/ProfilingLink"; import rowStyles from "../common/RowStyles"; import { Actor } from "../type/actor"; @@ -346,15 +347,16 @@ const ActorTable = ({ arrow interactive > - - {actorId} - +
+ +
{actorClass} diff --git a/dashboard/client/src/components/MetadataSection/MetadataSection.component.test.tsx b/dashboard/client/src/components/MetadataSection/MetadataSection.component.test.tsx index 12511a4909a0..227c1ca9e82f 100644 --- a/dashboard/client/src/components/MetadataSection/MetadataSection.component.test.tsx +++ b/dashboard/client/src/components/MetadataSection/MetadataSection.component.test.tsx @@ -10,49 +10,68 @@ const COPY_BUTTON_LABEL = "copy"; describe("MetadataContentField", () => { it("renders the content string", () => { - expect.assertions(3); + expect.assertions(4); - render(); + render( + , + ); expect(screen.getByText(CONTENT_VALUE)).toBeInTheDocument(); expect(screen.getByText(CONTENT_VALUE)).not.toHaveAttribute("href"); expect(screen.queryByLabelText(COPY_BUTTON_LABEL)).not.toBeInTheDocument(); + expect( + screen.getByTestId("metadata-content-for-test-label"), + ).toBeInTheDocument(); }); it("renders the content string with label", () => { - expect.assertions(3); + expect.assertions(4); render( , ); expect(screen.getByText(CONTENT_VALUE)).toBeInTheDocument(); expect(screen.getByText(CONTENT_VALUE)).toHaveAttribute("href", LINK_VALUE); expect(screen.queryByLabelText(COPY_BUTTON_LABEL)).not.toBeInTheDocument(); + expect( + screen.getByTestId("metadata-content-for-test-label"), + ).toBeInTheDocument(); }); it("renders the content string with copyable value", () => { - expect.assertions(3); + expect.assertions(4); render( , ); expect(screen.getByText(CONTENT_VALUE)).toBeInTheDocument(); expect(screen.getByText(CONTENT_VALUE)).not.toHaveAttribute("href"); expect(screen.getByLabelText(COPY_BUTTON_LABEL)).toBeInTheDocument(); + expect( + screen.getByTestId("metadata-content-for-test-label"), + ).toBeInTheDocument(); }); it("renders the content string with a JSX element", () => { - expect.assertions(2); + expect.assertions(3); const CUSTOM_TEST_ID = "custom-test-id"; const customElement =

Test

; - render(); + render(); expect(screen.queryByLabelText(COPY_BUTTON_LABEL)).not.toBeInTheDocument(); expect(screen.getByTestId(CUSTOM_TEST_ID)).toBeInTheDocument(); + expect( + screen.getByTestId("metadata-content-for-test-label"), + ).toBeInTheDocument(); }); }); diff --git a/dashboard/client/src/components/MetadataSection/MetadataSection.tsx b/dashboard/client/src/components/MetadataSection/MetadataSection.tsx index 531ca020c050..eb2c8af78b50 100644 --- a/dashboard/client/src/components/MetadataSection/MetadataSection.tsx +++ b/dashboard/client/src/components/MetadataSection/MetadataSection.tsx @@ -88,7 +88,8 @@ const useStyles = makeStyles((theme) => */ export const MetadataContentField: React.FC<{ content: Metadata["content"]; -}> = ({ content }) => { + label: string; +}> = ({ content, label }) => { const classes = useStyles(); const [copyIconClicked, setCopyIconClicked] = useState(false); @@ -99,6 +100,7 @@ export const MetadataContentField: React.FC<{ className={classes.content} variant="body2" title={content?.value} + data-testid={`metadata-content-for-${label}`} > {content?.value ?? "-"} @@ -127,7 +129,11 @@ export const MetadataContentField: React.FC<{ )}
) : content.link.startsWith("http") ? ( - + {content.value} ) : ( @@ -135,12 +141,13 @@ export const MetadataContentField: React.FC<{ className={classes.content} component={RouterLink} to={content.link} + data-testid={`metadata-content-for-${label}`} > {content.value} ); } - return content; + return
{content}
; }; /** @@ -168,7 +175,7 @@ const MetadataList: React.FC<{ )} - + ))} diff --git a/dashboard/client/src/components/TaskTable.tsx b/dashboard/client/src/components/TaskTable.tsx index a8202eed9506..4c630316c0fa 100644 --- a/dashboard/client/src/components/TaskTable.tsx +++ b/dashboard/client/src/components/TaskTable.tsx @@ -20,6 +20,7 @@ import { Link } from "react-router-dom"; import { GlobalContext } from "../App"; import DialogWithTitle from "../common/DialogWithTitle"; import { DurationText } from "../common/DurationText"; +import { ActorLink, NodeLink } from "../common/links"; import rowStyles from "../common/RowStyles"; import { Task } from "../type/task"; import { useFilter } from "../util/hook"; @@ -259,7 +260,7 @@ const TaskTable = ({ arrow interactive > -
{node_id ? node_id : "-"}
+ {node_id ? :
-
} @@ -269,7 +270,11 @@ const TaskTable = ({ arrow interactive > -
{actor_id ? actor_id : "-"}
+ {actor_id ? ( + + ) : ( +
-
+ )} diff --git a/dashboard/client/src/pages/actor/ActorDetail.tsx b/dashboard/client/src/pages/actor/ActorDetail.tsx index 44c09feb2612..a53b24ee0ec7 100644 --- a/dashboard/client/src/pages/actor/ActorDetail.tsx +++ b/dashboard/client/src/pages/actor/ActorDetail.tsx @@ -4,6 +4,7 @@ import { Link } from "react-router-dom"; import { GlobalContext } from "../../App"; import { DurationText } from "../../common/DurationText"; import { formatDateFromTimeMs } from "../../common/formatUtils"; +import { generateNodeLink } from "../../common/links"; import { CpuProfilingLink, CpuStackTraceLink, @@ -107,6 +108,9 @@ const ActorDetailPage = () => { ? { value: actorDetail.address?.rayletId, copyableValue: actorDetail.address?.rayletId, + link: actorDetail.address.rayletId + ? generateNodeLink(actorDetail.address.rayletId) + : undefined, } : { value: "-" }, }, diff --git a/dashboard/client/src/pages/node/NodeRow.tsx b/dashboard/client/src/pages/node/NodeRow.tsx index 92a5aece24a0..1a19160e59a3 100644 --- a/dashboard/client/src/pages/node/NodeRow.tsx +++ b/dashboard/client/src/pages/node/NodeRow.tsx @@ -5,13 +5,13 @@ import { TableRow, Tooltip, } from "@material-ui/core"; -import AddIcon from "@material-ui/icons/Add"; -import RemoveIcon from "@material-ui/icons/Remove"; import { sortBy } from "lodash"; import React, { useState } from "react"; +import { RiArrowDownSLine, RiArrowRightSLine } from "react-icons/ri"; import { Link } from "react-router-dom"; import useSWR from "swr"; import { API_REFRESH_INTERVAL_MS } from "../../common/constants"; +import { NodeLink } from "../../common/links"; import rowStyles from "../../common/RowStyles"; import PercentageBar from "../../components/PercentageBar"; import { StatusChip } from "../../components/StatusChip"; @@ -65,9 +65,9 @@ export const NodeRow = ({ {!expanded ? ( - + ) : ( - + )} @@ -79,9 +79,13 @@ export const NodeRow = ({ - - {raylet.nodeId} - +
+ +
diff --git a/dashboard/client/src/pages/serve/ServeApplicationDetailPage.component.test.tsx b/dashboard/client/src/pages/serve/ServeApplicationDetailPage.component.test.tsx index 55f5ba4ee501..ce8a0c6632db 100644 --- a/dashboard/client/src/pages/serve/ServeApplicationDetailPage.component.test.tsx +++ b/dashboard/client/src/pages/serve/ServeApplicationDetailPage.component.test.tsx @@ -1,4 +1,4 @@ -import { render, screen } from "@testing-library/react"; +import { render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import React from "react"; import { useParams } from "react-router-dom"; @@ -6,6 +6,7 @@ import { getServeApplications } from "../../service/serve"; import { ServeApplicationStatus, ServeDeploymentStatus, + ServeReplicaState, } from "../../type/serve"; import { TEST_APP_WRAPPER } from "../../util/test-utils"; import { ServeApplicationDetailPage } from "./ServeApplicationDetailPage"; @@ -20,11 +21,11 @@ const mockUseParams = jest.mocked(useParams); const mockGetServeApplications = jest.mocked(getServeApplications); describe("ServeApplicationDetailPage", () => { - it("renders page with deployments", async () => { - expect.assertions(12); + it("renders page with deployments and replicas", async () => { + expect.assertions(19); mockUseParams.mockReturnValue({ - name: "home", + applicationName: "home", }); mockGetServeApplications.mockResolvedValue({ data: { @@ -49,12 +50,46 @@ describe("ServeApplicationDetailPage", () => { }, status: ServeDeploymentStatus.HEALTHY, message: "deployment is healthy", + replicas: [ + { + actor_id: "test-actor-id", + actor_name: "FirstDeployment", + node_id: "test-node-id", + node_ip: "123.456.789.123", + pid: "12345", + replica_id: "test-replica-1", + start_time_s: new Date().getTime() / 1000, + state: ServeReplicaState.STARTING, + }, + { + actor_id: "test-actor-id-2", + actor_name: "FirstDeployment", + node_id: "test-node-id", + node_ip: "123.456.789.123", + pid: "12346", + replica_id: "test-replica-2", + start_time_s: new Date().getTime() / 1000, + state: ServeReplicaState.RUNNING, + }, + ], }, SecondDeployment: { name: "SecondDeployment", deployment_config: {}, status: ServeDeploymentStatus.UPDATING, message: "deployment is updating", + replicas: [ + { + actor_id: "test-actor-id-3", + actor_name: "SecondDeployment", + node_id: "test-node-id", + node_ip: "123.456.789.123", + pid: "12347", + replica_id: "test-replica-3", + start_time_s: new Date().getTime() / 1000, + state: ServeReplicaState.STARTING, + }, + ], }, }, }, @@ -73,9 +108,18 @@ describe("ServeApplicationDetailPage", () => { const user = userEvent.setup(); await screen.findByText("home"); - expect(screen.getByText("home")).toBeVisible(); - expect(screen.getByText("/home")).toBeVisible(); - expect(screen.getByText("RUNNING")).toBeVisible(); + expect(screen.getByTestId("metadata-content-for-Name")).toHaveTextContent( + "home", + ); + expect( + screen.getByTestId("metadata-content-for-Route prefix"), + ).toHaveTextContent("/home"); + expect(screen.getByTestId("metadata-content-for-Status")).toHaveTextContent( + "RUNNING", + ); + expect( + screen.getByTestId("metadata-content-for-Replicas"), + ).toHaveTextContent("3"); // First deployment expect(screen.getByText("FirstDeployment")).toBeVisible(); @@ -88,14 +132,32 @@ describe("ServeApplicationDetailPage", () => { expect(screen.getByText("UPDATING")).toBeVisible(); // Config dialog for application - await user.click(screen.getAllByText("View")[0]); + await user.click( + within( + screen.getByTestId("metadata-content-for-Application config"), + ).getByText("View"), + ); await screen.findByText(/import_path: home:graph/); expect(screen.getByText(/import_path: home:graph/)).toBeVisible(); // Config dialog for first deployment - await user.click(screen.getAllByText("View")[1]); + await user.click(screen.getAllByText("Deployment config")[0]); await screen.findByText(/test-config: 1/); expect(screen.getByText(/test-config: 1/)).toBeVisible(); expect(screen.getByText(/autoscaling-value: 2/)).toBeVisible(); + + // Expand the first deployment + await user.click(screen.getAllByTitle("Expand")[0]); + await screen.findByText("test-replica-1"); + expect(screen.getByText("test-replica-1")).toBeVisible(); + expect(screen.getByText("test-replica-2")).toBeVisible(); + expect(screen.queryByText("test-replica-3")).toBeNull(); + + // Collapse the first deployment + await user.click(screen.getByTitle("Collapse")); + await waitFor(() => screen.queryByText("test-replica-1") === null); + expect(screen.queryByText("test-replica-1")).toBeNull(); + expect(screen.queryByText("test-replica-2")).toBeNull(); + expect(screen.queryByText("test-replica-3")).toBeNull(); }); }); diff --git a/dashboard/client/src/pages/serve/ServeApplicationDetailPage.tsx b/dashboard/client/src/pages/serve/ServeApplicationDetailPage.tsx index 565de06e181b..cd2bdd9d877f 100644 --- a/dashboard/client/src/pages/serve/ServeApplicationDetailPage.tsx +++ b/dashboard/client/src/pages/serve/ServeApplicationDetailPage.tsx @@ -42,12 +42,12 @@ const useStyles = makeStyles((theme) => const columns: { label: string; helpInfo?: ReactElement; width?: string }[] = [ { label: "" }, // For expand/collapse button { label: "Name" }, + { label: "Replicas" }, { label: "Status" }, - { label: "Status message", width: "30%" }, { label: "Actions" }, + { label: "Status message", width: "30%" }, { label: "Last deployed at" }, { label: "Duration" }, - { label: "Replicas" }, ]; export const ServeApplicationDetailPage = () => { @@ -250,13 +250,6 @@ export const ServeApplicationDetailLayout = () => { return ( - {/* Extra MainNavPageInfo to add an extra layer of nesting in breadcrumbs */} - - { - setExpanded(!expanded); - }} - > - {!expanded ? ( - - ) : ( - - )} + + { + setExpanded(!expanded); + }} + > + {!expanded ? ( + + ) : ( + + )} + {name} + {Object.keys(replicas).length} + + + {message ? ( - - - {formatDateFromTimeMs(last_deployed_time_s * 1000)} - {Object.keys(replicas).length} {expanded && replicas.map((replica) => ( @@ -130,20 +138,20 @@ export const ServeReplicaRow = ({ {replica_id} + - - - + - {formatDateFromTimeMs(start_time_s * 1000)} - - ); }; diff --git a/dashboard/client/src/pages/serve/ServeReplicaDetailPage.component.test.tsx b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.component.test.tsx new file mode 100644 index 000000000000..05018ab214e5 --- /dev/null +++ b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.component.test.tsx @@ -0,0 +1,105 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import React from "react"; +import { useParams } from "react-router-dom"; +import { getServeApplications } from "../../service/serve"; +import { ServeReplicaState } from "../../type/serve"; +import { TEST_APP_WRAPPER } from "../../util/test-utils"; +import { ServeReplicaDetailPage } from "./ServeReplicaDetailPage"; + +jest.mock("react-router-dom", () => ({ + ...jest.requireActual("react-router-dom"), + useParams: jest.fn(), +})); +jest.mock("../../service/serve"); + +const mockUseParams = jest.mocked(useParams); +const mockGetServeApplications = jest.mocked(getServeApplications); + +describe("ServeReplicaDetailPage", () => { + it("renders", async () => { + expect.assertions(7); + + mockUseParams.mockReturnValue({ + applicationName: "home", + deploymentName: "FirstDeployment", + replicaId: "test-replica-1", + }); + mockGetServeApplications.mockResolvedValue({ + data: { + applications: { + home: { + name: "home", + route_prefix: "/home", + deployments: { + FirstDeployment: { + name: "FirstDeployment", + deployment_config: { + "test-config": 1, + autoscaling_config: { + "autoscaling-value": 2, + }, + }, + replicas: [ + { + actor_id: "test-actor-id", + actor_name: "FirstDeployment", + node_id: "test-node-id", + node_ip: "123.456.789.123", + pid: "12345", + replica_id: "test-replica-1", + start_time_s: new Date().getTime() / 1000, + state: ServeReplicaState.STARTING, + }, + { + // Decoy second replica + }, + ], + }, + SecondDeployment: { + name: "SecondDeployment", + deployment_config: {}, + replicas: [ + { + // Decoy third replica + }, + ], + }, + }, + }, + "second-app": { + // Decoy second app + }, + "third-app": { + // Decoy third app + }, + }, + }, + } as any); + + render(, { wrapper: TEST_APP_WRAPPER }); + + await screen.findByTestId("metadata-content-for-ID"); + expect(screen.getByTestId("metadata-content-for-ID")).toHaveTextContent( + "test-replica-1", + ); + expect(screen.getByTestId("metadata-content-for-State")).toHaveTextContent( + "STARTING", + ); + expect( + screen.getByTestId("metadata-content-for-Actor ID"), + ).toHaveTextContent("test-actor-id"); + expect( + screen.getByTestId("metadata-content-for-Actor name"), + ).toHaveTextContent("FirstDeployment"); + expect( + screen.getByTestId("metadata-content-for-Node ID"), + ).toHaveTextContent("test-node-id"); + expect( + screen.getByTestId("metadata-content-for-Node IP"), + ).toHaveTextContent("123.456.789.123"); + expect(screen.getByTestId("metadata-content-for-PID")).toHaveTextContent( + "12345", + ); + }); +}); diff --git a/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx index e350595716c1..14e90ae94508 100644 --- a/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx +++ b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx @@ -4,6 +4,7 @@ import { Link as RouterLink, useParams } from "react-router-dom"; import { GlobalContext } from "../../App"; import { DurationText } from "../../common/DurationText"; import { formatDateFromTimeMs } from "../../common/formatUtils"; +import { generateActorLink, generateNodeLink } from "../../common/links"; import Loading from "../../components/Loading"; import { MetadataSection } from "../../components/MetadataSection"; import { StatusChip } from "../../components/StatusChip"; @@ -44,13 +45,6 @@ export const ServeReplicaDetailPage = () => { } = replica; return (
- {/* Extra MainNavPageInfo to add an extra layer of nesting in breadcrumbs */} - { content: { value: actor_id ? actor_id : "-", copyableValue: actor_id ? actor_id : undefined, + link: actor_id ? generateActorLink(actor_id) : undefined, }, }, { @@ -100,6 +95,7 @@ export const ServeReplicaDetailPage = () => { content: { value: node_id ? node_id : "-", copyableValue: node_id ? node_id : undefined, + link: node_id ? generateNodeLink(node_id) : undefined, }, }, { From 0be747d2a3a1742b72f12214df6fb38ff47a1476 Mon Sep 17 00:00:00 2001 From: Alan Guo Date: Tue, 21 Mar 2023 13:30:44 -0700 Subject: [PATCH 5/8] fix lint Signed-off-by: Alan Guo --- .../src/pages/serve/ServeReplicaDetailPage.component.test.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/dashboard/client/src/pages/serve/ServeReplicaDetailPage.component.test.tsx b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.component.test.tsx index 05018ab214e5..5c38c9dc9f72 100644 --- a/dashboard/client/src/pages/serve/ServeReplicaDetailPage.component.test.tsx +++ b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.component.test.tsx @@ -1,5 +1,4 @@ import { render, screen } from "@testing-library/react"; -import userEvent from "@testing-library/user-event"; import React from "react"; import { useParams } from "react-router-dom"; import { getServeApplications } from "../../service/serve"; From 698bf0299c9421647e55943852c1b8f37cee553d Mon Sep 17 00:00:00 2001 From: Alan Guo Date: Tue, 21 Mar 2023 13:34:46 -0700 Subject: [PATCH 6/8] fixup Signed-off-by: Alan Guo --- .../client/src/pages/serve/ServeReplicaDetailPage.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx index 14e90ae94508..7b333b0e730c 100644 --- a/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx +++ b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx @@ -2,6 +2,7 @@ import { Link, Typography } from "@material-ui/core"; import React, { useContext } from "react"; import { Link as RouterLink, useParams } from "react-router-dom"; import { GlobalContext } from "../../App"; +import { CodeDialogButton } from "../../common/CodeDialogButton"; import { DurationText } from "../../common/DurationText"; import { formatDateFromTimeMs } from "../../common/formatUtils"; import { generateActorLink, generateNodeLink } from "../../common/links"; @@ -112,6 +113,15 @@ export const ServeReplicaDetailPage = () => { copyableValue: pid ? pid : undefined, }, }, + { + label: "Deployment config", + content: ( + + ), + }, { label: "Started at", content: { From e1f20b234522e66add86c5f24f1bcce8b337266a Mon Sep 17 00:00:00 2001 From: Alan Guo Date: Tue, 21 Mar 2023 16:49:27 -0700 Subject: [PATCH 7/8] Add tasks history Signed-off-by: Alan Guo --- dashboard/client/src/pages/actor/ActorDetail.tsx | 2 +- .../pages/serve/ServeReplicaDetailPage.component.test.tsx | 3 ++- dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx | 5 +++++ dashboard/client/src/pages/state/hook/useStateApi.ts | 4 ++-- dashboard/client/src/pages/state/task.tsx | 2 +- 5 files changed, 11 insertions(+), 5 deletions(-) diff --git a/dashboard/client/src/pages/actor/ActorDetail.tsx b/dashboard/client/src/pages/actor/ActorDetail.tsx index a53b24ee0ec7..f8158e62ed88 100644 --- a/dashboard/client/src/pages/actor/ActorDetail.tsx +++ b/dashboard/client/src/pages/actor/ActorDetail.tsx @@ -168,7 +168,7 @@ const ActorDetailPage = () => {
diff --git a/dashboard/client/src/pages/serve/ServeReplicaDetailPage.component.test.tsx b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.component.test.tsx index 5c38c9dc9f72..a3080589b0a1 100644 --- a/dashboard/client/src/pages/serve/ServeReplicaDetailPage.component.test.tsx +++ b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.component.test.tsx @@ -17,7 +17,7 @@ const mockGetServeApplications = jest.mocked(getServeApplications); describe("ServeReplicaDetailPage", () => { it("renders", async () => { - expect.assertions(7); + expect.assertions(8); mockUseParams.mockReturnValue({ applicationName: "home", @@ -100,5 +100,6 @@ describe("ServeReplicaDetailPage", () => { expect(screen.getByTestId("metadata-content-for-PID")).toHaveTextContent( "12345", ); + expect(screen.getByText("Tasks History")).toBeVisible(); }); }); diff --git a/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx index 7b333b0e730c..96e0bf36afec 100644 --- a/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx +++ b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx @@ -3,6 +3,7 @@ import React, { useContext } from "react"; import { Link as RouterLink, useParams } from "react-router-dom"; import { GlobalContext } from "../../App"; import { CodeDialogButton } from "../../common/CodeDialogButton"; +import { CollapsibleSection } from "../../common/CollapsibleSection"; import { DurationText } from "../../common/DurationText"; import { formatDateFromTimeMs } from "../../common/formatUtils"; import { generateActorLink, generateNodeLink } from "../../common/links"; @@ -11,6 +12,7 @@ import { MetadataSection } from "../../components/MetadataSection"; import { StatusChip } from "../../components/StatusChip"; import { ServeDeployment, ServeReplica } from "../../type/serve"; import { MainNavPageInfo } from "../layout/mainNavContext"; +import TaskList from "../state/task"; import { useServeReplicaDetails } from "./hook/useServeApplications"; export const ServeReplicaDetailPage = () => { @@ -134,6 +136,9 @@ export const ServeReplicaDetailPage = () => { }, ]} /> + + +
); }; diff --git a/dashboard/client/src/pages/state/hook/useStateApi.ts b/dashboard/client/src/pages/state/hook/useStateApi.ts index ee24d5d74a55..d8ec1187784b 100644 --- a/dashboard/client/src/pages/state/hook/useStateApi.ts +++ b/dashboard/client/src/pages/state/hook/useStateApi.ts @@ -1,5 +1,5 @@ import { AxiosResponse } from "axios"; -import useSWR from "swr"; +import useSWR, { Key } from "swr"; import { PER_JOB_PAGE_REFRESH_INTERVAL_MS } from "../../../common/constants"; import { AsyncFunction, @@ -8,7 +8,7 @@ import { } from "../../../type/stateApi"; export const useStateApiList = ( - key: string, + key: Key, getFunc: AsyncFunction>>, ) => { /** diff --git a/dashboard/client/src/pages/state/task.tsx b/dashboard/client/src/pages/state/task.tsx index 5792be05d56c..fae1e0312f8f 100644 --- a/dashboard/client/src/pages/state/task.tsx +++ b/dashboard/client/src/pages/state/task.tsx @@ -18,7 +18,7 @@ const TaskList = ({ actorId?: string; } & Pick) => { const [timeStamp] = useState(dayjs()); - const data: Task[] | undefined = useStateApiList("useTasks", () => + const data: Task[] | undefined = useStateApiList(["useTasks", jobId], () => getTasks(jobId), ); const tasks = data ? data : []; From 912c7d9117a62fbd755d4068b51be72c70b98285 Mon Sep 17 00:00:00 2001 From: Alan Guo Date: Tue, 21 Mar 2023 21:26:53 -0700 Subject: [PATCH 8/8] fix logs link Signed-off-by: Alan Guo --- .../src/pages/serve/ServeReplicaDetailPage.tsx | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx index 96e0bf36afec..466013fe87e7 100644 --- a/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx +++ b/dashboard/client/src/pages/serve/ServeReplicaDetailPage.tsx @@ -157,11 +157,16 @@ export const ServeReplicaLogsLink = ({ let link: string | undefined; if (node_ip && ipLogMap[node_ip]) { + // TODO(aguo): Clean up this logic after re-writing the log viewer + const logsRoot = ipLogMap[node_ip].endsWith("/logs") + ? ipLogMap[node_ip].substring( + 0, + ipLogMap[node_ip].length - "/logs".length, + ) + : ipLogMap[node_ip]; // TODO(aguo): Have API return the location of the logs. - const path = `/serve/deployment_${deploymentName}_${replica_id}.log`; - link = `/logs/${encodeURIComponent(ipLogMap[node_ip])}/${encodeURIComponent( - path, - )}`; + const path = `/logs/serve/deployment_${deploymentName}_${replica_id}.log`; + link = `/logs/${encodeURIComponent(logsRoot)}/${encodeURIComponent(path)}`; } if (link) {