Skip to content

Commit

Permalink
Execution detail page shows inputs and outputs (#2155)
Browse files Browse the repository at this point in the history
* Utils to convert metadata api from callback paradigm to promise paradigm

* Show input and output in execution details page

* Change execution detail page input/output table styling

* Make artifact names in execution detail page a deep link

* Change deep link to artifact ID instead

* Fix absolute import

* Fix lint errors
  • Loading branch information
Bobgy authored and k8s-ci-robot committed Sep 20, 2019
1 parent 0a38ced commit 7659e23
Show file tree
Hide file tree
Showing 6 changed files with 236 additions and 20 deletions.
9 changes: 9 additions & 0 deletions frontend/src/components/Router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,15 @@ export const RoutePage = {
RUN_DETAILS: `/runs/details/:${RouteParams.runId}`,
};

// tslint:disable-next-line:variable-name
export const RoutePageFactory = {
artifactDetails: (artifactType: string, artifactId: number) => {
return RoutePage.ARTIFACT_DETAILS
.replace(`:${RouteParams.ARTIFACT_TYPE}+`, artifactType)
.replace(`:${RouteParams.ID}`, '' + artifactId);
}
};

export interface DialogProps {
buttons?: Array<{ onClick?: () => any, text: string }>;
// TODO: This should be generalized to any react component.
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/lib/Apis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ function makePromiseApi<T, R>(apiMethod: MetadataApiMethod<T, R>): PromiseBasedM
const metadataServiceClient = new MetadataStoreServiceClient('');
// TODO: add all other api methods we need here.
const metadataServicePromiseClient = {
getArtifactTypes: makePromiseApi(metadataServiceClient.getArtifactTypes.bind(metadataServiceClient)),
getArtifactsByID: makePromiseApi(metadataServiceClient.getArtifactsByID.bind(metadataServiceClient)),
getEventsByArtifactIDs: makePromiseApi(metadataServiceClient.getEventsByArtifactIDs.bind(metadataServiceClient)),
getEventsByExecutionIDs: makePromiseApi(metadataServiceClient.getEventsByExecutionIDs.bind(metadataServiceClient)),
getExecutionsByID: makePromiseApi(metadataServiceClient.getExecutionsByID.bind(metadataServiceClient)),
Expand Down
24 changes: 21 additions & 3 deletions frontend/src/lib/MetadataUtils.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { Event } from '../generated/src/apis/metadata/metadata_store_pb';
import { GetEventsByArtifactIDsRequest, GetEventsByArtifactIDsResponse } from '../generated/src/apis/metadata/metadata_store_service_pb';
import { Apis } from '../lib/Apis';
import { ArtifactType, Event } from '../generated/src/apis/metadata/metadata_store_pb';
import { GetArtifactTypesRequest, GetEventsByArtifactIDsRequest, GetEventsByArtifactIDsResponse } from '../generated/src/apis/metadata/metadata_store_service_pb';
import { Apis } from './Apis';
import { formatDateString, serviceErrorToString } from './Utils';

export type EventTypes = Event.TypeMap[keyof Event.TypeMap];

export const getArtifactCreationTime = async (artifactId: number): Promise<string> => {
const eventsRequest = new GetEventsByArtifactIDsRequest();
if (!artifactId) {
Expand All @@ -29,3 +31,19 @@ export const getArtifactCreationTime = async (artifactId: number): Promise<strin
return '';
}
};

export const getArtifactTypeMap = async (): Promise<Map<number, ArtifactType>> => {
const map = new Map<number, ArtifactType>();
const { response, error } = await Apis.getMetadataServicePromiseClient().getArtifactTypes(new GetArtifactTypesRequest());
if (error) {
throw new Error(serviceErrorToString(error));
}

(response && response.getArtifactTypesList() || []).forEach((artifactType) => {
const id = artifactType.getId();
if (id) {
map.set(id, artifactType);
}
});
return map;
};
7 changes: 2 additions & 5 deletions frontend/src/pages/ArtifactList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { ToolbarProps } from '../components/Toolbar';
import { classes } from 'typestyle';
import { commonCss, padding } from '../Css';
import { getResourceProperty, rowCompareFn, rowFilterFn, groupRows, getExpandedRow, serviceErrorToString } from '../lib/Utils';
import { RoutePage, RouteParams } from '../components/Router';
import { RoutePageFactory } from '../components/Router';
import { Link } from 'react-router-dom';
import { Artifact, ArtifactType } from '../generated/src/apis/metadata/metadata_store_pb';
import { ArtifactProperties, ArtifactCustomProperties, ListRequest, Apis } from '../lib/Apis';
Expand Down Expand Up @@ -122,13 +122,10 @@ class ArtifactList extends Page<{}, ArtifactListState> {
private nameCustomRenderer: React.FC<CustomRendererProps<string>> =
(props: CustomRendererProps<string>) => {
const [artifactType, artifactId] = props.id.split(':');
const link = RoutePage.ARTIFACT_DETAILS
.replace(`:${RouteParams.ARTIFACT_TYPE}+`, artifactType)
.replace(`:${RouteParams.ID}`, artifactId);
return (
<Link onClick={(e) => e.stopPropagation()}
className={commonCss.link}
to={link}>
to={RoutePageFactory.artifactDetails(artifactType, Number(artifactId))}>
{props.value}
</Link>
);
Expand Down
208 changes: 197 additions & 11 deletions frontend/src/pages/ExecutionDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,28 @@
* limitations under the License.
*/

import * as React from 'react';
import React, { Component } from 'react';
import { Page } from './Page';
import { ToolbarProps } from '../components/Toolbar';
import { RoutePage, RouteParams } from '../components/Router';
import { classes } from 'typestyle';
import { RoutePage, RouteParams, RoutePageFactory } from '../components/Router';
import { classes, stylesheet } from 'typestyle';
import { commonCss, padding } from '../Css';
import { CircularProgress } from '@material-ui/core';
import { titleCase, getResourceProperty, serviceErrorToString } from '../lib/Utils';
import { titleCase, getResourceProperty, serviceErrorToString, logger } from '../lib/Utils';
import { ResourceInfo, ResourceType } from '../components/ResourceInfo';
import { Execution } from '../generated/src/apis/metadata/metadata_store_pb';
import { Apis, ExecutionProperties } from '../lib/Apis';
import { GetExecutionsByIDRequest } from '../generated/src/apis/metadata/metadata_store_service_pb';
import { Execution, ArtifactType } from '../generated/src/apis/metadata/metadata_store_pb';
import { Apis, ExecutionProperties, ArtifactProperties } from '../lib/Apis';
import { GetExecutionsByIDRequest, GetEventsByExecutionIDsRequest, GetEventsByExecutionIDsResponse, GetArtifactsByIDRequest } from '../generated/src/apis/metadata/metadata_store_service_pb';
import { EventTypes, getArtifactTypeMap } from '../lib/MetadataUtils';
import { Event } from '../generated/src/apis/metadata/metadata_store_pb';
import { Link } from 'react-router-dom';

type ArtifactIdList = number[];

interface ExecutionDetailsState {
execution?: Execution;
events?: Record<EventTypes, ArtifactIdList>;
artifactTypeMap?: Map<number, ArtifactType>;
}

export default class ExecutionDetails extends Page<{}, ExecutionDetailsState> {
Expand Down Expand Up @@ -61,7 +68,7 @@ export default class ExecutionDetails extends Page<{}, ExecutionDetailsState> {
}

public render(): JSX.Element {
if (!this.state.execution) {
if (!this.state.execution || !this.state.events) {
return <CircularProgress />;
}

Expand All @@ -72,6 +79,26 @@ export default class ExecutionDetails extends Page<{}, ExecutionDetailsState> {
typeName={this.properTypeName}
resource={this.state.execution}
/>}
<SectionIO
title={'Declared Inputs'}
artifactIds={this.state.events[Event.Type.DECLARED_INPUT]}
artifactTypeMap={this.state.artifactTypeMap}
/>
<SectionIO
title={'Inputs'}
artifactIds={this.state.events[Event.Type.INPUT]}
artifactTypeMap={this.state.artifactTypeMap}
/>
<SectionIO
title={'Declared Outputs'}
artifactIds={this.state.events[Event.Type.DECLARED_OUTPUT]}
artifactTypeMap={this.state.artifactTypeMap}
/>
<SectionIO
title={'Outputs'}
artifactIds={this.state.events[Event.Type.OUTPUT]}
artifactTypeMap={this.state.artifactTypeMap}
/>
</div >
);
}
Expand All @@ -89,18 +116,36 @@ export default class ExecutionDetails extends Page<{}, ExecutionDetailsState> {
}

private async load(): Promise<void> {
// this runs parallelly because it's not a critical resource
getArtifactTypeMap().then((artifactTypeMap) => {
this.setState({
artifactTypeMap,
});
}).catch((err) => {
this.showPageError('Failed to fetch artifact types', err);
});

const numberId = parseInt(this.id, 10);
if (isNaN(numberId) || numberId < 0) {
const error = new Error(`Invalid execution id: ${this.id}`);
this.showPageError(error.message, error);
return Promise.reject(error);
return;
}

const getExecutionsRequest = new GetExecutionsByIDRequest();
getExecutionsRequest.setExecutionIdsList([numberId]);
const getEventsRequest = new GetEventsByExecutionIDsRequest();
getEventsRequest.setExecutionIdsList([numberId]);

const executionResponse = await Apis.getMetadataServicePromiseClient().getExecutionsByID(getExecutionsRequest);
const [executionResponse, eventResponse] = await Promise.all([
Apis.getMetadataServicePromiseClient().getExecutionsByID(getExecutionsRequest),
Apis.getMetadataServicePromiseClient().getEventsByExecutionIDs(getEventsRequest),
]);

if (eventResponse.error) {
this.showPageError(serviceErrorToString(eventResponse.error));
// events data is optional, no need to skip the following
}
if (executionResponse.error) {
this.showPageError(serviceErrorToString(executionResponse.error));
return;
Expand All @@ -115,14 +160,155 @@ export default class ExecutionDetails extends Page<{}, ExecutionDetailsState> {
}

const execution = executionResponse.response.getExecutionsList()[0];

const executionName = getResourceProperty(execution, ExecutionProperties.COMPONENT_ID);
this.props.updateToolbar({
pageTitle: executionName ? executionName.toString() : ''
});

const events = parseEventsByType(eventResponse.response);

this.setState({
events,
execution,
});
}
}

function parseEventsByType(response: GetEventsByExecutionIDsResponse | null): Record<EventTypes, ArtifactIdList> {
const events: Record<EventTypes, ArtifactIdList> = {
[Event.Type.UNKNOWN]: [],
[Event.Type.DECLARED_INPUT]: [],
[Event.Type.INPUT]: [],
[Event.Type.DECLARED_OUTPUT]: [],
[Event.Type.OUTPUT]: [],
};

if (!response) {
return events;
}

response.getEventsList().forEach(event => {
const type = event.getType();
const id = event.getArtifactId();
if (type != null && id != null) {
events[type].push(id);
}
});

return events;
}

interface ArtifactInfo {
id: number;
name: string;
typeId?: number;
uri: string;
}

interface SectionIOProps {
title: string;
artifactIds: number[];
artifactTypeMap?: Map<number, ArtifactType>;
}
class SectionIO extends Component<SectionIOProps, { artifactDataMap: { [id: number]: ArtifactInfo } }> {
constructor(props: any) {
super(props);

this.state = {
artifactDataMap: {},
};
}

public async componentDidMount(): Promise<void> {
// loads extra metadata about artifacts
const request = new GetArtifactsByIDRequest();
request.setArtifactIdsList(this.props.artifactIds);
const { error, response } = await Apis.getMetadataServicePromiseClient().getArtifactsByID(request);
if (error || !response) {
return;
}

const artifactDataMap = {};
response.getArtifactsList().forEach(artifact => {
const id = artifact.getId();
if (!id) {
logger.error('Artifact has empty id', artifact.toObject());
return;
}
const data: ArtifactInfo = {
id,
name: (getResourceProperty(artifact, ArtifactProperties.NAME) || '') as string, // TODO: assert name is string
typeId: artifact.getTypeId(),
uri: artifact.getUri() || '',
};
artifactDataMap[id] = data;
});
this.setState({
artifactDataMap,
});
}

public render(): JSX.Element | null {
const { title, artifactIds } = this.props;
if (artifactIds.length === 0) {
return null;
}

return <section>
<h2 className={commonCss.header2}>{title}</h2>
<table>
<thead>
<tr>
<th className={css.tableCell}>Artifact ID</th>
<th className={css.tableCell}>Name</th>
<th className={css.tableCell}>Type</th>
<th className={css.tableCell}>URI</th>
</tr>
</thead>
<tbody>
{artifactIds.map(id => {
const data = this.state.artifactDataMap[id] || {};
const type = (this.props.artifactTypeMap && data.typeId)
? this.props.artifactTypeMap.get(data.typeId)
: null;
return <ArtifactRow
key={id}
id={id}
name={data.name || ''}
type={type ? type.getName() : undefined}
uri={data.uri}
/>;
}
)}
</tbody>
</table>
</section>;
}
}

// tslint:disable-next-line:variable-name
const ArtifactRow: React.FC<{ id: number, name: string, type?: string, uri: string }> =
({ id, name, type, uri }) => (
<tr>
<td className={css.tableCell}>
{type && id ?
<Link
className={commonCss.link}
to={RoutePageFactory.artifactDetails(type, id)}>
{id}
</Link>
: id
}
</td>
<td className={css.tableCell}>{name}</td>
<td className={css.tableCell}>{type}</td>
<td className={css.tableCell}>{uri}</td>
</tr>
);

const css = stylesheet({
tableCell: {
padding: 6,
textAlign: 'left',
},
});
6 changes: 5 additions & 1 deletion frontend/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
{
"compilerOptions": {
"allowSyntheticDefaultImports": true,
"baseUrl": ".",
"outDir": "build/dist",
"module": "esnext",
"target": "es5",
"lib": ["es6", "dom"],
"lib": [
"es6",
"dom"
],
"sourceMap": true,
"allowJs": true,
"jsx": "react",
Expand Down

0 comments on commit 7659e23

Please sign in to comment.