From 7659e238bf886c215a7351dc711382888f1472f8 Mon Sep 17 00:00:00 2001 From: "Yuan (Bob) Gong" Date: Fri, 20 Sep 2019 22:57:00 +0800 Subject: [PATCH] Execution detail page shows inputs and outputs (#2155) * 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 --- frontend/src/components/Router.tsx | 9 + frontend/src/lib/Apis.ts | 2 + frontend/src/lib/MetadataUtils.ts | 24 ++- frontend/src/pages/ArtifactList.tsx | 7 +- frontend/src/pages/ExecutionDetails.tsx | 208 ++++++++++++++++++++++-- frontend/tsconfig.json | 6 +- 6 files changed, 236 insertions(+), 20 deletions(-) diff --git a/frontend/src/components/Router.tsx b/frontend/src/components/Router.tsx index 6be54ac3aa6..521f5ead63a 100644 --- a/frontend/src/components/Router.tsx +++ b/frontend/src/components/Router.tsx @@ -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. diff --git a/frontend/src/lib/Apis.ts b/frontend/src/lib/Apis.ts index c26bb611b3e..1a5e7726a1a 100644 --- a/frontend/src/lib/Apis.ts +++ b/frontend/src/lib/Apis.ts @@ -90,6 +90,8 @@ function makePromiseApi(apiMethod: MetadataApiMethod): 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)), diff --git a/frontend/src/lib/MetadataUtils.ts b/frontend/src/lib/MetadataUtils.ts index 1c1423dbf40..35f3fa0a70b 100644 --- a/frontend/src/lib/MetadataUtils.ts +++ b/frontend/src/lib/MetadataUtils.ts @@ -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 => { const eventsRequest = new GetEventsByArtifactIDsRequest(); if (!artifactId) { @@ -29,3 +31,19 @@ export const getArtifactCreationTime = async (artifactId: number): Promise> => { + const map = new Map(); + 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; +}; diff --git a/frontend/src/pages/ArtifactList.tsx b/frontend/src/pages/ArtifactList.tsx index a6c80210082..8d8fb98c74d 100644 --- a/frontend/src/pages/ArtifactList.tsx +++ b/frontend/src/pages/ArtifactList.tsx @@ -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'; @@ -122,13 +122,10 @@ class ArtifactList extends Page<{}, ArtifactListState> { private nameCustomRenderer: React.FC> = (props: CustomRendererProps) => { const [artifactType, artifactId] = props.id.split(':'); - const link = RoutePage.ARTIFACT_DETAILS - .replace(`:${RouteParams.ARTIFACT_TYPE}+`, artifactType) - .replace(`:${RouteParams.ID}`, artifactId); return ( e.stopPropagation()} className={commonCss.link} - to={link}> + to={RoutePageFactory.artifactDetails(artifactType, Number(artifactId))}> {props.value} ); diff --git a/frontend/src/pages/ExecutionDetails.tsx b/frontend/src/pages/ExecutionDetails.tsx index c1fd4611021..0c1e864cd9b 100644 --- a/frontend/src/pages/ExecutionDetails.tsx +++ b/frontend/src/pages/ExecutionDetails.tsx @@ -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; + artifactTypeMap?: Map; } export default class ExecutionDetails extends Page<{}, ExecutionDetailsState> { @@ -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 ; } @@ -72,6 +79,26 @@ export default class ExecutionDetails extends Page<{}, ExecutionDetailsState> { typeName={this.properTypeName} resource={this.state.execution} />} + + + + ); } @@ -89,18 +116,36 @@ export default class ExecutionDetails extends Page<{}, ExecutionDetailsState> { } private async load(): Promise { + // 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; @@ -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 { + const events: Record = { + [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; +} +class SectionIO extends Component { + constructor(props: any) { + super(props); + + this.state = { + artifactDataMap: {}, + }; + } + + public async componentDidMount(): Promise { + // 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
+

{title}

+ + + + + + + + + + + {artifactIds.map(id => { + const data = this.state.artifactDataMap[id] || {}; + const type = (this.props.artifactTypeMap && data.typeId) + ? this.props.artifactTypeMap.get(data.typeId) + : null; + return ; + } + )} + +
Artifact IDNameTypeURI
+
; + } +} + +// tslint:disable-next-line:variable-name +const ArtifactRow: React.FC<{ id: number, name: string, type?: string, uri: string }> = + ({ id, name, type, uri }) => ( + + + {type && id ? + + {id} + + : id + } + + {name} + {type} + {uri} + + ); + +const css = stylesheet({ + tableCell: { + padding: 6, + textAlign: 'left', + }, +}); diff --git a/frontend/tsconfig.json b/frontend/tsconfig.json index 129a0696b58..a1ec2da63b1 100644 --- a/frontend/tsconfig.json +++ b/frontend/tsconfig.json @@ -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",