Skip to content

Commit

Permalink
[Dashboard] [Serve] Add application row with dropdown for deployment …
Browse files Browse the repository at this point in the history
…rows (ray-project#43506)

Adds back "application" rows to the top-level deployments page, which were removed in ray-project#42079. We keep the deployment rows as a dropdown under the application row, similar to how worker rows are a dropdown under node rows in the "Cluster" page (and we mimic the implementation from that part as well).

Per discussion offline:

Delete the "filters" component because it's not necessary for most use cases, and the user can always cmd-F to find in page if necessary
Start all the rows expanded
Replace the "deployments status" chip with "application status". If there's time, we can try to include both by making the metadata section number of columns configurable and set it to 4 instead of 3.
I also took a couple liberties which hadn't been discussed yet, but happy to revert them if they don't make sense:

Un-bold the the deployment name, but keep the application name bold. This highlights the hierarchical nature and makes the "deployment" rows more visually distinct from the "application" rows.
Make the number of replicas link to the deployment page where it shows the replicas. Reason: when I first saw "2" under replicas my first thought was "where can I see more about these 2 replicas", and I naturally wanted to click it.
---------

Signed-off-by: Archit Kulkarni <[email protected]>
Co-authored-by: Huaiwei Sun <[email protected]>
  • Loading branch information
architkulkarni and scottsun94 committed Feb 29, 2024
1 parent 7af6abc commit 9e419f4
Show file tree
Hide file tree
Showing 9 changed files with 206 additions and 183 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type RecentServeCardProps = {
export const RecentServeCard = ({ className }: RecentServeCardProps) => {
const classes = useStyles();

const { allServeDeployments: deployments } = useServeDeployments();
const { serveDeployments: deployments } = useServeDeployments();

const sortedDeployments = _.orderBy(
deployments,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ const columns: { label: string; helpInfo?: ReactElement; width?: string }[] = [
{ label: "Status message", width: "30%" },
{ label: "Num replicas" },
{ label: "Actions" },
{ label: "Application" },
{ label: "Route prefix" },
{ label: "Last deployed at" },
{ label: "Duration (since last deploy)" },
Expand Down Expand Up @@ -250,6 +249,7 @@ export const ServeApplicationDetailPage = () => {
key={deployment.name}
deployment={deployment}
application={application}
showExpandColumn={false}
/>
))}
</TableBody>
Expand Down
146 changes: 104 additions & 42 deletions dashboard/client/src/pages/serve/ServeApplicationRow.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
import { TableCell, TableRow } from "@material-ui/core";
import React from "react";
import {
createStyles,
IconButton,
Link,
makeStyles,
TableCell,
TableRow,
} from "@material-ui/core";
import React, { useState } from "react";
import { RiArrowDownSLine, RiArrowRightSLine } from "react-icons/ri";
import { Link as RouterLink } from "react-router-dom";
import {
CodeDialogButton,
Expand All @@ -9,14 +17,30 @@ import { DurationText } from "../../common/DurationText";
import { formatDateFromTimeMs } from "../../common/formatUtils";
import { StatusChip } from "../../components/StatusChip";
import { ServeApplication } from "../../type/serve";
import { ServeDeploymentRow } from "./ServeDeploymentRow";

export type ServeApplicationRowProps = {
export type ServeApplicationRowsProps = {
application: ServeApplication;
startExpanded?: boolean;
};

export const ServeApplicationRow = ({
const useStyles = makeStyles((theme) =>
createStyles({
applicationName: {
fontWeight: 500,
},
expandCollapseIcon: {
color: theme.palette.text.secondary,
fontSize: "1.5em",
verticalAlign: "middle",
},
}),
);
export const ServeApplicationRows = ({
application,
}: ServeApplicationRowProps) => {
startExpanded = true,
}: ServeApplicationRowsProps) => {
const [isExpanded, setExpanded] = useState(startExpanded);

const {
name,
message,
Expand All @@ -27,44 +51,82 @@ export const ServeApplicationRow = ({
deployed_app_config,
} = application;

const deploymentsList = Object.values(deployments);

const classes = useStyles();

const onExpandButtonClick = () => {
setExpanded(!isExpanded);
};

// TODO(aguo): Add duration and end time once available in the API
return (
<TableRow>
<TableCell align="center">
<RouterLink to={`applications/${name ? name : "-"}`}>
{name ? name : "-"}
</RouterLink>
</TableCell>
<TableCell align="center">{route_prefix}</TableCell>
<TableCell align="center">
<StatusChip type="serveApplication" status={status} />
</TableCell>
<TableCell align="center">
{message ? (
<CodeDialogButtonWithPreview title="Message details" code={message} />
) : (
"-"
)}
</TableCell>
<TableCell align="center">{Object.values(deployments).length}</TableCell>
<TableCell align="center">
{formatDateFromTimeMs(last_deployed_time_s * 1000)}
</TableCell>
<TableCell align="center">
<DurationText startTime={last_deployed_time_s * 1000} />
</TableCell>
<TableCell align="center">
{deployed_app_config ? (
<CodeDialogButton
title={
name ? `Application config for ${name}` : `Application config`
}
code={deployed_app_config}
<React.Fragment>
<TableRow>
<TableCell>
<IconButton size="small" onClick={onExpandButtonClick}>
{!isExpanded ? (
<RiArrowRightSLine className={classes.expandCollapseIcon} />
) : (
<RiArrowDownSLine className={classes.expandCollapseIcon} />
)}
</IconButton>
</TableCell>
<TableCell align="center" className={classes.applicationName}>
<Link
component={RouterLink}
to={`applications/${name ? encodeURIComponent(name) : "-"}`}
>
{name ? name : "-"}
</Link>
</TableCell>
<TableCell align="center">
<StatusChip type="serveApplication" status={status} />
</TableCell>
<TableCell align="center">
{message ? (
<CodeDialogButtonWithPreview
title="Message details"
code={message}
/>
) : (
"-"
)}
</TableCell>
<TableCell align="center">
{/* placeholder for num_replicas, which does not apply to an application */}
-
</TableCell>
<TableCell align="center">
{deployed_app_config ? (
<CodeDialogButton
title={
name ? `Application config for ${name}` : `Application config`
}
code={deployed_app_config}
buttonText="View config"
/>
) : (
"-"
)}
</TableCell>
<TableCell align="center">{route_prefix}</TableCell>
<TableCell align="center">
{formatDateFromTimeMs(last_deployed_time_s * 1000)}
</TableCell>
<TableCell align="center">
<DurationText startTime={last_deployed_time_s * 1000} />
</TableCell>
</TableRow>
{isExpanded &&
deploymentsList.map((deployment) => (
<ServeDeploymentRow
key={`${application.name}-${deployment.name}`}
deployment={deployment}
application={application}
showExpandColumn
/>
) : (
"-"
)}
</TableCell>
</TableRow>
))}
</React.Fragment>
);
};
50 changes: 33 additions & 17 deletions dashboard/client/src/pages/serve/ServeDeploymentRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,10 @@ import { useViewServeDeploymentMetricsButtonUrl } from "./ServeDeploymentMetrics
const useStyles = makeStyles((theme) =>
createStyles({
deploymentName: {
fontWeight: 500,
fontWeight: 400,
},
expandCollapseIcon: {
color: theme.palette.text.secondary,
fontSize: "1.5em",
verticalAlign: "middle",
deploymentNameAsFirstColumn: {
fontWeight: 500, // bold style for when name is the first column, e.g. on the Deployment page
},
statusMessage: {
maxWidth: 400,
Expand All @@ -38,25 +36,38 @@ const useStyles = makeStyles((theme) =>
}),
);

export type ServeDeployentRowProps = {
export type ServeDeploymentRowProps = {
deployment: ServeDeployment;
application: ServeApplication;
// Optional prop to control the visibility of the first column.
// This is used to display an expand/collapse button on the applications page, but not the deployment page.
showExpandColumn?: boolean;
};

export const ServeDeploymentRow = ({
deployment,
application: { last_deployed_time_s, name: applicationName, route_prefix },
}: ServeDeployentRowProps) => {
application: { last_deployed_time_s, name: applicationName },
showExpandColumn = false,
}: ServeDeploymentRowProps) => {
const { name, status, message, deployment_config, replicas } = deployment;

const classes = useStyles();

const metricsUrl = useViewServeDeploymentMetricsButtonUrl(name);

const deploymentNameClass = showExpandColumn
? classes.deploymentName
: `${classes.deploymentName} ${classes.deploymentNameAsFirstColumn}`;

return (
<React.Fragment>
<TableRow>
<TableCell align="center" className={classes.deploymentName}>
{showExpandColumn && (
<TableCell>
{/* Empty column for expand/unexpand button in the row of the parent Serve application. */}
</TableCell>
)}
<TableCell align="center" className={deploymentNameClass}>
<Link
component={RouterLink}
to={`/serve/applications/${encodeURIComponent(
Expand All @@ -80,7 +91,17 @@ export const ServeDeploymentRow = ({
"-"
)}
</TableCell>
<TableCell align="center">{replicas.length}</TableCell>
<TableCell align="center">
{" "}
<Link
component={RouterLink}
to={`/serve/applications/${encodeURIComponent(
applicationName,
)}/${encodeURIComponent(name)}`}
>
{replicas.length}
</Link>
</TableCell>
<TableCell align="center">
<CodeDialogButton
title={`Deployment config for ${name}`}
Expand All @@ -106,14 +127,9 @@ export const ServeDeploymentRow = ({
)}
</TableCell>
<TableCell align="center">
<Link
component={RouterLink}
to={`/serve/applications/${encodeURIComponent(applicationName)}`}
>
{applicationName}
</Link>
{/* placeholder for route_prefix, which does not apply to a deployment */}
-
</TableCell>
<TableCell align="center">{route_prefix}</TableCell>
<TableCell align="center">
{formatDateFromTimeMs(last_deployed_time_s * 1000)}
</TableCell>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const mockGetActor = jest.mocked(getActor);

describe("ServeDeploymentsListPage", () => {
it("renders list", async () => {
expect.assertions(6);
expect.assertions(4);

// Mock ServeController actor fetch
mockGetActor.mockResolvedValue({
Expand Down Expand Up @@ -88,15 +88,13 @@ describe("ServeDeploymentsListPage", () => {
} as any);

render(<ServeDeploymentsListPage />, { wrapper: TEST_APP_WRAPPER });
await screen.findByText("Deployments status");
await screen.findByText("Application status");

// First row
expect(screen.getByText("FirstDeployment")).toBeVisible();
expect(screen.getAllByText("/")[0]).toBeVisible();

// Second row
expect(screen.getByText("SecondDeployment")).toBeVisible();
expect(screen.getAllByText("/")[1]).toBeVisible();

// Third row
expect(screen.getByText("ThirdDeployment")).toBeVisible();
Expand Down
Loading

0 comments on commit 9e419f4

Please sign in to comment.