diff --git a/frontend/mock-backend/fixed-data.ts b/frontend/mock-backend/fixed-data.ts index d495009115f..daad605bde9 100644 --- a/frontend/mock-backend/fixed-data.ts +++ b/frontend/mock-backend/fixed-data.ts @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -import * as fs from 'fs'; import helloWorldRun from './hello-world-runtime'; import helloWorldWithStepsRun from './hello-world-with-steps-runtime'; import coinflipRun from './mock-coinflip-runtime'; @@ -23,15 +22,6 @@ import { ApiJob } from '../src/apis/job'; import { ApiPipeline } from '../src/apis/pipeline'; import { ApiRunDetail, ApiResourceType, ApiRelationship, RunMetricFormat } from '../src/apis/run'; -const xgboostTemplate = - JSON.stringify({ template: fs.readFileSync('./mock-backend/mock-template.yaml', 'utf-8') }); - -const conditionalTemplate = - JSON.stringify({ - template: fs.readFileSync('./mock-backend/mock-conditional-template.yaml', - 'utf-8') - }); - function padStartTwoZeroes(str: string): string { let padded = str || ''; while (padded.length < 2) { @@ -146,7 +136,6 @@ const jobs: ApiJob[] = [ } ], pipeline_id: pipelines[0].id, - workflow_manifest: conditionalTemplate, }, resource_references: [{ key: { @@ -188,7 +177,6 @@ const jobs: ApiJob[] = [ } ], pipeline_id: pipelines[1].id, - workflow_manifest: conditionalTemplate, }, resource_references: [{ key: { @@ -233,7 +221,6 @@ const jobs: ApiJob[] = [ } ], pipeline_id: pipelines[2].id, - workflow_manifest: xgboostTemplate, }, resource_references: [{ key: { @@ -303,8 +290,6 @@ const runs: ApiRunDetail[] = [ { name: 'paramName2', value: 'paramVal2' }, ], pipeline_id: '8fbe3bd6-a01f-11e8-98d0-529269fb1459', - pipeline_manifest: 'TBD', - workflow_manifest: conditionalTemplate, }, resource_references: [{ key: { @@ -354,8 +339,6 @@ const runs: ApiRunDetail[] = [ { name: 'paramName2', value: 'paramVal2' }, ], pipeline_id: '8fbe3bd6-a01f-11e8-98d0-529269fb1459', - pipeline_manifest: 'TBD', - workflow_manifest: conditionalTemplate, }, resource_references: [{ key: { @@ -389,8 +372,6 @@ const runs: ApiRunDetail[] = [ { name: 'paramName2', value: 'paramVal2' }, ], pipeline_id: '8fbe41b2-a01f-11e8-98d0-529269fb1459', - pipeline_manifest: 'TBD', - workflow_manifest: conditionalTemplate, }, resource_references: [{ key: { @@ -424,8 +405,6 @@ const runs: ApiRunDetail[] = [ { name: 'paramName2', value: 'paramVal2' }, ], pipeline_id: '8fbe42f2-a01f-11e8-98d0-529269fb1459', - pipeline_manifest: 'TBD', - workflow_manifest: conditionalTemplate, }, scheduled_at: new Date('2018-06-17T22:58:23.000Z'), status: 'Failed', @@ -466,8 +445,6 @@ const runs: ApiRunDetail[] = [ { name: 'paramName2', value: 'paramVal2' }, ], pipeline_id: '8fbe3f78-a01f-11e8-98d0-529269fb1459', - pipeline_manifest: 'TBD', - workflow_manifest: xgboostTemplate, }, resource_references: [{ key: { @@ -516,8 +493,60 @@ const runs: ApiRunDetail[] = [ { name: 'paramName2', value: 'paramVal2' }, ], pipeline_id: '8fbe3f78-a01f-11e8-98d0-529269fb1459', - pipeline_manifest: 'TBD', - workflow_manifest: xgboostTemplate, + }, + resource_references: [{ + key: { + id: 'a4d4f8c6-ce9c-4200-a92e-c48ec759b733', + type: ApiResourceType.EXPERIMENT, + }, + relationship: ApiRelationship.OWNER, + }], + scheduled_at: new Date('2018-08-18T20:58:23.000Z'), + status: 'Succeeded', + }, + }, + { + pipeline_runtime: { + workflow_manifest: JSON.stringify(helloWorldRun), + }, + run: { + created_at: new Date('2018-08-18T20:58:23.000Z'), + description: 'simple run with pipeline spec embedded in it.', + id: '7fc01715-4a93-4c00-8044-a8a72c14253b', + metrics: [ + { + format: RunMetricFormat.PERCENTAGE, + name: 'accuracy', + number_value: 0.5999, + }, + { + format: RunMetricFormat.RAW, + name: 'log_loss', + number_value: -0.223, + } + ], + name: 'hello-world-with-pipeline', + namespace: 'namespace', + pipeline_spec: { + parameters: [ + { name: 'paramName1', value: 'paramVal1' }, + { name: 'paramName2', value: 'paramVal2' }, + ], + workflow_manifest: ` +apiVersion: argoproj.io/v1alpha1 +kind: Workflow +metadata: + generateName: hello-world- +spec: + entrypoint: whalesay + serviceAccountName: pipeline-runner + templates: + - name: whalesay + container: + image: docker/whalesay:latest + command: [cowsay] + args: ["hello world"] +`, }, resource_references: [{ key: { @@ -592,8 +621,6 @@ function generateNRuns(): ApiRunDetail[] { { name: 'paramName2', value: 'paramVal2' }, ], pipeline_id: 'Some-pipeline-id-' + i, - pipeline_manifest: 'TBD', - workflow_manifest: conditionalTemplate, }, resource_references: [{ key: { @@ -640,7 +667,6 @@ function generateNJobs(): ApiJob[] { } ], pipeline_id: pipelines[i % pipelines.length].id, - workflow_manifest: xgboostTemplate, }, resource_references: [{ key: { diff --git a/frontend/src/components/Router.tsx b/frontend/src/components/Router.tsx index 8ac9467da8f..94928baa1f0 100644 --- a/frontend/src/components/Router.tsx +++ b/frontend/src/components/Router.tsx @@ -44,6 +44,17 @@ const css = stylesheet({ }, }); +export enum QUERY_PARAMS { + cloneFromRun = 'cloneFromRun', + experimentId = 'experimentId', + isRecurring = 'recurring', + firstRunInExperiment = 'firstRunInExperiment', + pipelineId = 'pipelineId', + fromRunId = 'fromRun', + runlist = 'runlist', + view = 'view', +} + export enum RouteParams { experimentId = 'eid', pipelineId = 'pid', @@ -58,7 +69,7 @@ export const RoutePage = { NEW_EXPERIMENT: '/experiments/new', NEW_RUN: '/runs/new', PIPELINES: '/pipelines', - PIPELINE_DETAILS: `/pipelines/details/:${RouteParams.pipelineId}`, + PIPELINE_DETAILS: `/pipelines/details/:${RouteParams.pipelineId}?`, // pipelineId is optional RECURRING_RUN: `/recurringrun/details/:${RouteParams.runId}`, RUNS: '/runs', RUN_DETAILS: `/runs/details/:${RouteParams.runId}`, diff --git a/frontend/src/components/__snapshots__/Router.test.tsx.snap b/frontend/src/components/__snapshots__/Router.test.tsx.snap index 59a521156de..c70e34165f5 100644 --- a/frontend/src/components/__snapshots__/Router.test.tsx.snap +++ b/frontend/src/components/__snapshots__/Router.test.tsx.snap @@ -56,7 +56,7 @@ exports[`Router initial render 1`] = ` { diff --git a/frontend/src/pages/ExperimentDetails.tsx b/frontend/src/pages/ExperimentDetails.tsx index 5e0054f0136..88b75fbc0ff 100644 --- a/frontend/src/pages/ExperimentDetails.tsx +++ b/frontend/src/pages/ExperimentDetails.tsx @@ -30,8 +30,8 @@ import { ApiExperiment } from '../apis/experiment'; import { ApiResourceType } from '../apis/job'; import { Apis } from '../lib/Apis'; import { Page } from './Page'; -import { RoutePage, RouteParams } from '../components/Router'; -import { URLParser, QUERY_PARAMS } from '../lib/URLParser'; +import { RoutePage, RouteParams, QUERY_PARAMS } from '../components/Router'; +import { URLParser } from '../lib/URLParser'; import { classes, stylesheet } from 'typestyle'; import { color, commonCss, padding } from '../Css'; import { logger } from '../lib/Utils'; @@ -233,8 +233,8 @@ class ExperimentDetails extends Page<{}, ExperimentDetailsState> { @@ -291,14 +291,14 @@ class ExperimentDetails extends Page<{}, ExperimentDetailsState> { } this.setStateSafe({ activeRecurringRunsCount, experiment }); - + } catch (err) { await this.showPageError(`Error: failed to retrieve experiment: ${experimentId}.`, err); logger.error(`Error loading experiment: ${experimentId}`, err); } if (this._runlistRef.current) { - this._runlistRef.current.refresh(); + this._runlistRef.current.refresh(); } } diff --git a/frontend/src/pages/ExperimentList.test.tsx b/frontend/src/pages/ExperimentList.test.tsx index 9459ca8673a..ca7391fe207 100644 --- a/frontend/src/pages/ExperimentList.test.tsx +++ b/frontend/src/pages/ExperimentList.test.tsx @@ -22,8 +22,7 @@ import { Apis } from '../lib/Apis'; import { ExpandState } from '../components/CustomTable'; import { NodePhase } from './Status'; import { PageProps } from './Page'; -import { QUERY_PARAMS } from '../lib/URLParser'; -import { RoutePage } from '../components/Router'; +import { RoutePage, QUERY_PARAMS } from '../components/Router'; import { range } from 'lodash'; import { shallow, ReactWrapper } from 'enzyme'; diff --git a/frontend/src/pages/ExperimentList.tsx b/frontend/src/pages/ExperimentList.tsx index d0af98b44a3..995aab8a047 100644 --- a/frontend/src/pages/ExperimentList.tsx +++ b/frontend/src/pages/ExperimentList.tsx @@ -24,9 +24,9 @@ import { ApiResourceType, ApiRun } from '../apis/run'; import { Apis, ExperimentSortKeys, ListRequest, RunSortKeys } from '../lib/Apis'; import { Link } from 'react-router-dom'; import { Page } from './Page'; -import { RoutePage, RouteParams } from '../components/Router'; +import { RoutePage, RouteParams, QUERY_PARAMS } from '../components/Router'; import { ToolbarProps } from '../components/Toolbar'; -import { URLParser, QUERY_PARAMS } from '../lib/URLParser'; +import { URLParser } from '../lib/URLParser'; import { classes } from 'typestyle'; import { commonCss, padding } from '../Css'; import { logger } from '../lib/Utils'; diff --git a/frontend/src/pages/NewExperiment.test.tsx b/frontend/src/pages/NewExperiment.test.tsx index 06981cb4c75..66375f9a21e 100644 --- a/frontend/src/pages/NewExperiment.test.tsx +++ b/frontend/src/pages/NewExperiment.test.tsx @@ -20,8 +20,7 @@ import TestUtils from '../TestUtils'; import { shallow } from 'enzyme'; import { PageProps } from './Page'; import { Apis } from '../lib/Apis'; -import { RoutePage } from '../components/Router'; -import { QUERY_PARAMS } from '../lib/URLParser'; +import { RoutePage, QUERY_PARAMS } from '../components/Router'; describe('NewExperiment', () => { const createExperimentSpy = jest.spyOn(Apis.experimentServiceApi, 'createExperiment'); diff --git a/frontend/src/pages/NewExperiment.tsx b/frontend/src/pages/NewExperiment.tsx index 461d5526013..bd67acfb50f 100644 --- a/frontend/src/pages/NewExperiment.tsx +++ b/frontend/src/pages/NewExperiment.tsx @@ -21,10 +21,10 @@ import Input from '../atoms/Input'; import { ApiExperiment } from '../apis/experiment'; import { Apis } from '../lib/Apis'; import { Page } from './Page'; -import { RoutePage } from '../components/Router'; +import { RoutePage, QUERY_PARAMS } from '../components/Router'; import { TextFieldProps } from '@material-ui/core/TextField'; import { ToolbarProps } from '../components/Toolbar'; -import { URLParser, QUERY_PARAMS } from '../lib/URLParser'; +import { URLParser } from '../lib/URLParser'; import { classes, stylesheet } from 'typestyle'; import { commonCss, padding, fontsize } from '../Css'; import { logger, errorToMessage } from '../lib/Utils'; diff --git a/frontend/src/pages/NewRun.test.tsx b/frontend/src/pages/NewRun.test.tsx index 86ef6014d08..e471ff24474 100644 --- a/frontend/src/pages/NewRun.test.tsx +++ b/frontend/src/pages/NewRun.test.tsx @@ -20,10 +20,9 @@ import TestUtils from '../TestUtils'; import { shallow } from 'enzyme'; import { PageProps } from './Page'; import { Apis } from '../lib/Apis'; -import { RoutePage, RouteParams } from '../components/Router'; +import { RoutePage, RouteParams, QUERY_PARAMS } from '../components/Router'; import { ApiExperiment } from '../apis/experiment'; import { ApiPipeline } from '../apis/pipeline'; -import { QUERY_PARAMS } from '../lib/URLParser'; import { ApiResourceType, ApiRunDetail, ApiParameter, ApiRelationship } from '../apis/run'; class TestNewRun extends NewRun { diff --git a/frontend/src/pages/NewRun.tsx b/frontend/src/pages/NewRun.tsx index 7d00bbcf18f..eb63f0e8e4d 100644 --- a/frontend/src/pages/NewRun.tsx +++ b/frontend/src/pages/NewRun.tsx @@ -33,9 +33,9 @@ import { ApiRun, ApiResourceReference, ApiRelationship, ApiResourceType, ApiRunD import { ApiTrigger, ApiJob } from '../apis/job'; import { Apis } from '../lib/Apis'; import { Page } from './Page'; -import { RoutePage, RouteParams } from '../components/Router'; +import { RoutePage, RouteParams, QUERY_PARAMS } from '../components/Router'; import { ToolbarProps } from '../components/Toolbar'; -import { URLParser, QUERY_PARAMS } from '../lib/URLParser'; +import { URLParser } from '../lib/URLParser'; import { Workflow } from '../../../frontend/third_party/argo-ui/argo_template'; import { classes, stylesheet } from 'typestyle'; import { commonCss, padding } from '../Css'; diff --git a/frontend/src/pages/PipelineDetails.test.tsx b/frontend/src/pages/PipelineDetails.test.tsx index 481725256b5..3b10a608b64 100644 --- a/frontend/src/pages/PipelineDetails.test.tsx +++ b/frontend/src/pages/PipelineDetails.test.tsx @@ -19,12 +19,13 @@ import * as StaticGraphParser from '../lib/StaticGraphParser'; import PipelineDetails, { css } from './PipelineDetails'; import TestUtils from '../TestUtils'; import { ApiPipeline } from '../apis/pipeline'; +import { ApiRunDetail, ApiResourceType } from '../apis/run'; import { Apis } from '../lib/Apis'; import { PageProps } from './Page'; -import { QUERY_PARAMS } from '../lib/URLParser'; -import { RouteParams, RoutePage } from '../components/Router'; +import { RouteParams, RoutePage, QUERY_PARAMS } from '../components/Router'; import { graphlib } from 'dagre'; import { shallow, mount, ShallowWrapper, ReactWrapper } from 'enzyme'; +import { ApiExperiment } from 'src/apis/experiment'; describe('PipelineDetails', () => { const updateBannerSpy = jest.fn(); @@ -33,19 +34,27 @@ describe('PipelineDetails', () => { const updateToolbarSpy = jest.fn(); const historyPushSpy = jest.fn(); const getPipelineSpy = jest.spyOn(Apis.pipelineServiceApi, 'getPipeline'); + const getRunSpy = jest.spyOn(Apis.runServiceApi, 'getRun'); + const getExperimentSpy = jest.spyOn(Apis.experimentServiceApi, 'getExperiment'); const deletePipelineSpy = jest.spyOn(Apis.pipelineServiceApi, 'deletePipeline'); const getTemplateSpy = jest.spyOn(Apis.pipelineServiceApi, 'getTemplate'); const createGraphSpy = jest.spyOn(StaticGraphParser, 'createGraph'); let tree: ShallowWrapper | ReactWrapper; let testPipeline: ApiPipeline = {}; + let testRun: ApiRunDetail = {}; - function generateProps(): PageProps { + function generateProps(fromRunSpec = false): PageProps { // getInitialToolbarState relies on page props having been populated, so fill those first const pageProps: PageProps = { history: { push: historyPushSpy } as any, - location: '' as any, - match: { params: { [RouteParams.pipelineId]: testPipeline.id }, isExact: true, path: '', url: '' }, + location: { search: fromRunSpec ? `?${QUERY_PARAMS.fromRunId}=test-run-id` : '' } as any, + match: { + isExact: true, + params: fromRunSpec ? {} : { [RouteParams.pipelineId]: testPipeline.id }, + path: '', + url: '', + }, toolbarProps: { actions: [], breadcrumbs: [], pageTitle: '' }, updateBanner: updateBannerSpy, updateDialog: updateDialogSpy, @@ -63,23 +72,18 @@ describe('PipelineDetails', () => { beforeEach(() => { testPipeline = { created_at: new Date(2018, 8, 5, 4, 3, 2), - description: 'test job description', - enabled: true, - id: 'test-job-id', - max_concurrency: '50', - name: 'test job', - pipeline_spec: { - parameters: [{ name: 'param1', value: 'value1' }], - pipeline_id: 'some-pipeline-id', - }, - trigger: { - periodic_schedule: { - end_time: new Date(2018, 10, 9, 8, 7, 6), - interval_second: '3600', - start_time: new Date(2018, 9, 8, 7, 6), - } + description: 'test pipeline description', + id: 'test-pipeline-id', + name: 'test pipeline', + parameters: [{ name: 'param1', value: 'value1' }], + }; + + testRun = { + run: { + id: 'test-run-id', + name: 'test run', }, - } as ApiPipeline; + }; historyPushSpy.mockClear(); updateBannerSpy.mockClear(); @@ -89,7 +93,12 @@ describe('PipelineDetails', () => { deletePipelineSpy.mockReset(); getPipelineSpy.mockImplementation(() => Promise.resolve(testPipeline)); getPipelineSpy.mockClear(); - getTemplateSpy.mockImplementation(() => Promise.resolve('{}')); + getRunSpy.mockImplementation(() => Promise.resolve(testRun)); + getRunSpy.mockClear(); + getExperimentSpy.mockImplementation(() => + Promise.resolve({ id: 'test-experiment-id', name: 'test experiment' } as ApiExperiment)); + getExperimentSpy.mockClear(); + getTemplateSpy.mockImplementation(() => Promise.resolve({ template: 'test template' })); getTemplateSpy.mockClear(); createGraphSpy.mockImplementation(() => new graphlib.Graph()); createGraphSpy.mockClear(); @@ -105,6 +114,89 @@ describe('PipelineDetails', () => { expect(tree).toMatchSnapshot(); }); + it('shows pipeline name in page name, and breadcrumb to go back to pipelines', async () => { + tree = shallow(); + await getTemplateSpy; + await TestUtils.flushPromises(); + expect(updateToolbarSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + breadcrumbs: [{ displayName: 'Pipelines', href: RoutePage.PIPELINES }], + pageTitle: testPipeline.name, + })); + }); + + it('shows all runs breadcrumbs, and "Pipeline details" as page title when the pipeline ' + + 'comes from a run spec that does not have an experiment', async () => { + tree = shallow(); + await getRunSpy; + await getTemplateSpy; + await TestUtils.flushPromises(); + expect(updateToolbarSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + breadcrumbs: [ + { displayName: 'All runs', href: RoutePage.RUNS }, + { + displayName: testRun.run!.name, href: RoutePage.RUN_DETAILS.replace( + ':' + RouteParams.runId, testRun.run!.id!) + }, + ], + pageTitle: 'Pipeline details', + })); + }); + + it('shows all runs breadcrumbs, and "Pipeline details" as page title when the pipeline ' + + 'comes from a run spec that has an experiment', async () => { + testRun.run!.resource_references = [ + { key: { id: 'test-experiment-id', type: ApiResourceType.EXPERIMENT } }, + ]; + tree = shallow(); + await getRunSpy; + await getExperimentSpy; + await getTemplateSpy; + await TestUtils.flushPromises(); + expect(updateToolbarSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + breadcrumbs: [ + { displayName: 'Experiments', href: RoutePage.EXPERIMENTS }, + { + displayName: 'test experiment', href: RoutePage.EXPERIMENT_DETAILS.replace( + ':' + RouteParams.experimentId, 'test-experiment-id') + }, + { + displayName: testRun.run!.name, href: RoutePage.RUN_DETAILS.replace( + ':' + RouteParams.runId, testRun.run!.id!) + }, + ], + pageTitle: 'Pipeline details', + })); + }); + + it('shows load error banner when failing to get run details, when loading from run spec', async () => { + TestUtils.makeErrorResponseOnce(getRunSpy, 'woops'); + tree = shallow(); + await getPipelineSpy; + await TestUtils.flushPromises(); + expect(updateBannerSpy).toHaveBeenCalledTimes(2); // Once to clear banner, once to show error + expect(updateBannerSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + additionalInfo: 'woops', + message: 'Cannot retrieve run details. Click Details for more information.', + mode: 'error', + })); + }); + + it('shows load error banner when failing to get experiment details, when loading from run spec', async () => { + testRun.run!.resource_references = [ + { key: { id: 'test-experiment-id', type: ApiResourceType.EXPERIMENT } }, + ]; + TestUtils.makeErrorResponseOnce(getExperimentSpy, 'woops'); + tree = shallow(); + await getPipelineSpy; + await TestUtils.flushPromises(); + expect(updateBannerSpy).toHaveBeenCalledTimes(2); // Once to clear banner, once to show error + expect(updateBannerSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + additionalInfo: 'woops', + message: 'Cannot retrieve run details. Click Details for more information.', + mode: 'error', + })); + }); + it('shows load error banner when failing to get pipeline', async () => { TestUtils.makeErrorResponseOnce(getPipelineSpy, 'woops'); tree = shallow(); @@ -178,7 +270,7 @@ describe('PipelineDetails', () => { expect(tree.state('summaryShown')).toBe(true); }); - it('has a new experiment button', async () => { + it('has a new experiment button if it has a pipeline reference', async () => { tree = shallow(); await getTemplateSpy; await TestUtils.flushPromises(); @@ -188,6 +280,14 @@ describe('PipelineDetails', () => { expect(newExperimentBtn).toBeDefined(); }); + it('does not have any toolbar buttons if it has an embedded pipeline', async () => { + tree = shallow(); + await getTemplateSpy; + await TestUtils.flushPromises(); + const instance = tree.instance() as PipelineDetails; + expect(instance.getInitialToolbarState().actions).toEqual([]); + }); + it('clicking new experiment button navigates to new experiment page', async () => { tree = shallow(); await getTemplateSpy; diff --git a/frontend/src/pages/PipelineDetails.tsx b/frontend/src/pages/PipelineDetails.tsx index 93cd3af1950..91ed78341a1 100644 --- a/frontend/src/pages/PipelineDetails.tsx +++ b/frontend/src/pages/PipelineDetails.tsx @@ -25,26 +25,28 @@ import Graph from '../components/Graph'; import InfoIcon from '@material-ui/icons/InfoOutlined'; import MD2Tabs from '../atoms/MD2Tabs'; import Paper from '@material-ui/core/Paper'; +import RunUtils from '../lib/RunUtils'; import SidePanel from '../components/SidePanel'; import StaticNodeDetails from '../components/StaticNodeDetails'; +import { ApiExperiment } from '../apis/experiment'; import { ApiPipeline, ApiGetTemplateResponse } from '../apis/pipeline'; import { Apis } from '../lib/Apis'; import { Page } from './Page'; -import { RoutePage, RouteParams } from '../components/Router'; +import { RoutePage, RouteParams, QUERY_PARAMS } from '../components/Router'; import { ToolbarProps } from '../components/Toolbar'; -import { URLParser, QUERY_PARAMS } from '../lib/URLParser'; +import { URLParser } from '../lib/URLParser'; import { UnControlled as CodeMirror } from 'react-codemirror2'; import { Workflow } from '../../third_party/argo-ui/argo_template'; import { classes, stylesheet } from 'typestyle'; import { color, commonCss, padding, fontsize, fonts } from '../Css'; -import { logger, errorToMessage } from '../lib/Utils'; +import { logger, errorToMessage, formatDateString } from '../lib/Utils'; interface PipelineDetailsState { graph?: dagre.graphlib.Graph; - selectedNodeInfo: JSX.Element | null; pipeline: ApiPipeline | null; - selectedTab: number; selectedNodeId: string; + selectedNodeInfo: JSX.Element | null; + selectedTab: number; summaryShown: boolean; template?: Workflow; templateYaml?: string; @@ -111,30 +113,41 @@ class PipelineDetails extends Page<{}, PipelineDetailsState> { } public getInitialToolbarState(): ToolbarProps { - return { - actions: [{ - action: this._createNewExperiment.bind(this), - icon: AddIcon, - id: 'startNewExperimentBtn', - primary: true, - title: 'Start an experiment', - tooltip: 'Create a new experiment beginning with this pipeline', - }, { - action: () => this.props.updateDialog({ - buttons: [ - { onClick: () => this._deleteDialogClosed(true), text: 'Delete' }, - { onClick: () => this._deleteDialogClosed(false), text: 'Cancel' }, - ], - onClose: () => this._deleteDialogClosed(false), - title: 'Delete this pipeline?', - }), - id: 'deleteBtn', - title: 'Delete', - tooltip: 'Delete this pipeline', - }], - breadcrumbs: [{ displayName: 'Pipelines', href: RoutePage.PIPELINES }], - pageTitle: this.props.match.params[RouteParams.pipelineId], - }; + const fromRunId = new URLParser(this.props).get(QUERY_PARAMS.fromRunId); + if (fromRunId) { + return { + actions: [], + breadcrumbs: [ + { displayName: fromRunId, href: RoutePage.RUN_DETAILS.replace(':' + RouteParams.runId, fromRunId) } + ], + pageTitle: 'Pipeline details', + }; + } else { + return { + actions: [{ + action: this._createNewExperiment.bind(this), + icon: AddIcon, + id: 'startNewExperimentBtn', + primary: true, + title: 'Start an experiment', + tooltip: 'Create a new experiment beginning with this pipeline', + }, { + action: () => this.props.updateDialog({ + buttons: [ + { onClick: () => this._deleteDialogClosed(true), text: 'Delete' }, + { onClick: () => this._deleteDialogClosed(false), text: 'Cancel' }, + ], + onClose: () => this._deleteDialogClosed(false), + title: 'Delete this pipeline?', + }), + id: 'deleteBtn', + title: 'Delete', + tooltip: 'Delete this pipeline', + }], + breadcrumbs: [{ displayName: 'Pipelines', href: RoutePage.PIPELINES }], + pageTitle: this.props.match.params[RouteParams.pipelineId], + }; + } } public render(): JSX.Element { @@ -151,77 +164,77 @@ class PipelineDetails extends Page<{}, PipelineDetailsState> { return (
- {pipeline && ( +
+ this.setStateSafe({ selectedTab: tab })} + tabs={['Graph', 'Source']} + />
- this.setStateSafe({ selectedTab: tab })} - tabs={['Graph', 'Source']} - /> -
- {selectedTab === 0 &&
- {this.state.graph &&
- {summaryShown && ( - -
-
- Summary + {selectedTab === 0 &&
+ {this.state.graph &&
+ {!!pipeline && summaryShown && ( + +
+
+ Summary
- -
-
Uploaded on
-
{new Date(pipeline.created_at!).toLocaleString()}
-
Description
-
{pipeline.description}
-
- )} +
+
Uploaded on
+
{formatDateString(pipeline.created_at)}
+
Description
+
{pipeline.description}
+ + )} - this.setStateSafe({ selectedNodeId: id })} /> + this.setStateSafe({ selectedNodeId: id })} /> - this.setStateSafe({ selectedNodeId: '' })}> -
- {!selectedNodeInfo &&
Unable to retrieve node info
} - {!!selectedNodeInfo &&
- -
} -
-
-
- {!summaryShown && ( - + this.setStateSafe({ selectedNodeId: '' })}> +
+ {!selectedNodeInfo && ( +
Unable to retrieve node info
)} -
- - Static pipeline graph -
+ {!!selectedNodeInfo &&
+ +
} +
+
+
+ {!summaryShown && ( + + )} +
+ + Static pipeline graph
-
} - {!this.state.graph && No graph to show} -
} - {selectedTab === 1 && -
- editor.refresh()} - options={{ - lineNumbers: true, - lineWrapping: true, - mode: 'text/yaml', - readOnly: true, - theme: 'default', - }} - />
- } -
+
} + {!this.state.graph && No graph to show} +
} + {selectedTab === 1 && !!templateYaml && +
+ editor.refresh()} + options={{ + lineNumbers: true, + lineWrapping: true, + mode: 'text/yaml', + readOnly: true, + theme: 'default', + }} + /> +
+ }
- )} +
); } @@ -236,48 +249,91 @@ class PipelineDetails extends Page<{}, PipelineDetailsState> { public async load(): Promise { this.clearBanner(); - const pipelineId = this.props.match.params[RouteParams.pipelineId]; + const fromRunId = new URLParser(this.props).get(QUERY_PARAMS.fromRunId); - let pipeline: ApiPipeline; - let templateResponse: ApiGetTemplateResponse; + let pipeline: ApiPipeline | null = null; + let templateYaml = ''; + let template: Workflow | undefined; + let breadcrumbs: Array<{ displayName: string, href: string }> = []; + const toolbarActions = [...this.props.toolbarProps.actions]; + let pageTitle = ''; - try { - pipeline = await Apis.pipelineServiceApi.getPipeline(pipelineId); - } catch (err) { - await this.showPageError('Cannot retrieve pipeline details.', err); - logger.error('Cannot retrieve pipeline details.', err); - return; - } + // If fromRunId is specified, load the run and get the pipeline template from it + if (fromRunId) { + try { + const runDetails = await Apis.runServiceApi.getRun(fromRunId); + templateYaml = RunUtils.getPipelineSpec(runDetails.run) || ''; - try { - templateResponse = await Apis.pipelineServiceApi.getTemplate(pipelineId); - } catch (err) { - await this.showPageError('Cannot retrieve pipeline template.', err); - logger.error('Cannot retrieve pipeline details.', err); - return; + const relatedExperimentId = RunUtils.getFirstExperimentReferenceId(runDetails.run); + let experiment: ApiExperiment | undefined; + if (relatedExperimentId) { + experiment = await Apis.experimentServiceApi.getExperiment(relatedExperimentId); + } + + // Build the breadcrumbs, by adding experiment and run names + if (experiment) { + breadcrumbs.push( + { displayName: 'Experiments', href: RoutePage.EXPERIMENTS }, + { + displayName: experiment.name!, + href: RoutePage.EXPERIMENT_DETAILS.replace(':' + RouteParams.experimentId, experiment.id!) + }); + } else { + breadcrumbs.push( + { displayName: 'All runs', href: RoutePage.RUNS } + ); + } + breadcrumbs.push({ + displayName: runDetails.run!.name!, + href: RoutePage.RUN_DETAILS.replace(':' + RouteParams.runId, fromRunId), + }); + pageTitle = 'Pipeline details'; + } catch (err) { + await this.showPageError('Cannot retrieve run details.', err); + logger.error('Cannot retrieve run details.', err); + } + } else { + // if fromRunId is not specified, then we have a full pipeline + const pipelineId = this.props.match.params[RouteParams.pipelineId]; + let templateResponse: ApiGetTemplateResponse; + + try { + pipeline = await Apis.pipelineServiceApi.getPipeline(pipelineId); + pageTitle = pipeline.name!; + } catch (err) { + await this.showPageError('Cannot retrieve pipeline details.', err); + logger.error('Cannot retrieve pipeline details.', err); + return; + } + + try { + templateResponse = await Apis.pipelineServiceApi.getTemplate(pipelineId); + templateYaml = templateResponse.template || ''; + } catch (err) { + await this.showPageError('Cannot retrieve pipeline template.', err); + logger.error('Cannot retrieve pipeline details.', err); + return; + } + + breadcrumbs = [{ displayName: 'Pipelines', href: RoutePage.PIPELINES }]; + toolbarActions[0].disabled = false; } - let template: Workflow | undefined; + this.props.updateToolbar({ breadcrumbs, actions: toolbarActions, pageTitle }); + let g: dagre.graphlib.Graph | undefined; try { - template = JsYaml.safeLoad(templateResponse.template || '{}'); + template = JsYaml.safeLoad(templateYaml); g = StaticGraphParser.createGraph(template!); } catch (err) { await this.showPageError('Error: failed to generate Pipeline graph.', err); } - const breadcrumbs = [{ displayName: 'Pipelines', href: RoutePage.PIPELINES }]; - const pageTitle = pipeline.name!; - - const toolbarActions = [...this.props.toolbarProps.actions]; - toolbarActions[0].disabled = false; - this.props.updateToolbar({ breadcrumbs, actions: toolbarActions, pageTitle }); - this.setStateSafe({ graph: g, pipeline, template, - templateYaml: templateResponse.template, + templateYaml, }); } diff --git a/frontend/src/pages/PipelineList.test.tsx b/frontend/src/pages/PipelineList.test.tsx index d843033392c..dad24d44507 100644 --- a/frontend/src/pages/PipelineList.test.tsx +++ b/frontend/src/pages/PipelineList.test.tsx @@ -196,7 +196,7 @@ describe('PipelineList', () => { const link = tree.find('a[children="test pipeline name0"]'); expect(link).toHaveLength(1); expect(link.prop('href')).toBe(RoutePage.PIPELINE_DETAILS.replace( - ':' + RouteParams.pipelineId, 'test-pipeline-id0' + ':' + RouteParams.pipelineId + '?', 'test-pipeline-id0' )); tree.unmount(); }); diff --git a/frontend/src/pages/RunDetails.test.tsx b/frontend/src/pages/RunDetails.test.tsx index 320f48a89b1..7f8875614a4 100644 --- a/frontend/src/pages/RunDetails.test.tsx +++ b/frontend/src/pages/RunDetails.test.tsx @@ -23,8 +23,7 @@ import { Apis } from '../lib/Apis'; import { OutputArtifactLoader } from '../lib/OutputArtifactLoader'; import { PageProps } from './Page'; import { PlotType } from '../components/viewers/Viewer'; -import { QUERY_PARAMS } from '../lib/URLParser'; -import { RouteParams, RoutePage } from '../components/Router'; +import { RouteParams, RoutePage, QUERY_PARAMS } from '../components/Router'; import { Workflow } from 'third_party/argo-ui/argo_template'; import { shallow } from 'enzyme'; diff --git a/frontend/src/pages/RunDetails.tsx b/frontend/src/pages/RunDetails.tsx index 2ce5306b55c..b0936a33aba 100644 --- a/frontend/src/pages/RunDetails.tsx +++ b/frontend/src/pages/RunDetails.tsx @@ -30,17 +30,17 @@ import { ApiExperiment } from '../apis/experiment'; import { ApiRun } from '../apis/run'; import { Apis } from '../lib/Apis'; import { NodePhase, statusToIcon } from './Status'; +import { OutputArtifactLoader } from '../lib/OutputArtifactLoader'; import { Page } from './Page'; -import { RoutePage, RouteParams } from '../components/Router'; +import { RoutePage, RouteParams, QUERY_PARAMS } from '../components/Router'; import { ToolbarProps } from '../components/Toolbar'; -import { URLParser, QUERY_PARAMS } from '../lib/URLParser'; +import { URLParser } from '../lib/URLParser'; import { ViewerConfig } from '../components/viewers/Viewer'; import { Workflow } from '../../third_party/argo-ui/argo_template'; +import { classes } from 'typestyle'; import { commonCss, padding } from '../Css'; import { componentMap } from '../components/viewers/ViewerContainer'; import { formatDateString, getRunTime, logger, errorToMessage } from '../lib/Utils'; -import { OutputArtifactLoader } from '../lib/OutputArtifactLoader'; -import { classes } from 'typestyle'; enum SidePaneTab { ARTIFACTS, diff --git a/frontend/src/pages/RunList.test.tsx b/frontend/src/pages/RunList.test.tsx index 824c1142e65..e4772647d7c 100644 --- a/frontend/src/pages/RunList.test.tsx +++ b/frontend/src/pages/RunList.test.tsx @@ -19,12 +19,18 @@ import RunList, { RunListProps } from './RunList'; import TestUtils from '../TestUtils'; import produce from 'immer'; import { ApiRun, ApiRunDetail, ApiResourceType, ApiRunMetric, RunMetricFormat } from '../apis/run'; -import { Apis, RunSortKeys } from '../lib/Apis'; +import { Apis, RunSortKeys, ListRequest } from '../lib/Apis'; import { MetricMetadata } from '../lib/RunUtils'; import { NodePhase } from './Status'; import { range } from 'lodash'; import { shallow } from 'enzyme'; +class RunListTest extends RunList { + public _loadRuns(request: ListRequest): Promise { + return super._loadRuns(request); + } +} + describe('RunList', () => { const onErrorSpy = jest.fn(); const listRunsSpy = jest.spyOn(Apis.runServiceApi, 'listRuns'); @@ -35,7 +41,7 @@ describe('RunList', () => { function generateProps(): RunListProps { return { history: {} as any, - location: '' as any, + location: { search: '' } as any, match: '' as any, onError: onErrorSpy, }; @@ -78,7 +84,7 @@ describe('RunList', () => { mockNRuns(1, {}); const props = generateProps(); const tree = shallow(); - await (tree.instance() as any)._loadRuns({}); + await (tree.instance() as RunListTest)._loadRuns({}); expect(Apis.runServiceApi.listRuns).toHaveBeenLastCalledWith(undefined, undefined, undefined, undefined, undefined); expect(props.onError).not.toHaveBeenCalled(); expect(tree).toMatchSnapshot(); @@ -100,7 +106,7 @@ describe('RunList', () => { mockNRuns(5, {}); const props = generateProps(); const tree = shallow(); - await (tree.instance() as any)._loadRuns({}); + await (tree.instance() as RunListTest)._loadRuns({}); expect(props.onError).not.toHaveBeenCalled(); expect(tree).toMatchSnapshot(); }); @@ -109,7 +115,7 @@ describe('RunList', () => { TestUtils.makeErrorResponseOnce(jest.spyOn(Apis.runServiceApi, 'listRuns'), 'bad stuff happened'); const props = generateProps(); const tree = shallow(); - await (tree.instance() as any)._loadRuns({}); + await (tree.instance() as RunListTest)._loadRuns({}); expect(props.onError).toHaveBeenLastCalledWith('Error: failed to fetch runs.', new Error('bad stuff happened')); }); @@ -117,7 +123,7 @@ describe('RunList', () => { mockNRuns(1, { pipeline_runtime: { workflow_manifest: 'bad json' } }); const props = generateProps(); const tree = shallow(); - await (tree.instance() as any)._loadRuns({}); + await (tree.instance() as RunListTest)._loadRuns({}); expect(tree).toMatchSnapshot(); }); @@ -126,7 +132,7 @@ describe('RunList', () => { TestUtils.makeErrorResponseOnce(getPipelineSpy, 'bad stuff happened'); const props = generateProps(); const tree = shallow(); - await (tree.instance() as any)._loadRuns({}); + await (tree.instance() as RunListTest)._loadRuns({}); expect(tree).toMatchSnapshot(); }); @@ -141,7 +147,7 @@ describe('RunList', () => { TestUtils.makeErrorResponseOnce(getExperimentSpy, 'bad stuff happened'); const props = generateProps(); const tree = shallow(); - await (tree.instance() as any)._loadRuns({}); + await (tree.instance() as RunListTest)._loadRuns({}); expect(tree).toMatchSnapshot(); }); @@ -150,7 +156,7 @@ describe('RunList', () => { const props = generateProps(); props.runIdListMask = ['testrun1', 'testrun2']; const tree = shallow(); - await (tree.instance() as any)._loadRuns({}); + await (tree.instance() as RunListTest)._loadRuns({}); expect(tree).toMatchSnapshot(); }); @@ -168,7 +174,7 @@ describe('RunList', () => { }); const props = generateProps(); const tree = shallow(); - await (tree.instance() as any)._loadRuns({}); + await (tree.instance() as RunListTest)._loadRuns({}); expect(props.onError).not.toHaveBeenCalled(); expect(tree).toMatchSnapshot(); }); @@ -178,7 +184,7 @@ describe('RunList', () => { const props = generateProps(); props.experimentIdMask = 'experiment1'; const tree = shallow(); - await (tree.instance() as any)._loadRuns({}); + await (tree.instance() as RunListTest)._loadRuns({}); expect(props.onError).not.toHaveBeenCalled(); expect(Apis.runServiceApi.listRuns).toHaveBeenLastCalledWith( undefined, undefined, undefined, ApiResourceType.EXPERIMENT.toString(), 'experiment1'); @@ -189,7 +195,7 @@ describe('RunList', () => { const props = generateProps(); props.runIdListMask = ['run1', 'run2']; const tree = shallow(); - await (tree.instance() as any)._loadRuns({}); + await (tree.instance() as RunListTest)._loadRuns({}); expect(props.onError).not.toHaveBeenCalled(); expect(Apis.runServiceApi.listRuns).not.toHaveBeenCalled(); expect(Apis.runServiceApi.getRun).toHaveBeenCalledTimes(2); @@ -215,7 +221,7 @@ describe('RunList', () => { }); const props = generateProps(); const tree = shallow(); - await (tree.instance() as any)._loadRuns({}); + await (tree.instance() as RunListTest)._loadRuns({}); expect(props.onError).not.toHaveBeenCalled(); expect(tree).toMatchSnapshot(); }); @@ -225,7 +231,7 @@ describe('RunList', () => { getPipelineSpy.mockImplementationOnce(() => ({ name: 'test pipeline' })); const props = generateProps(); const tree = shallow(); - await (tree.instance() as any)._loadRuns({}); + await (tree.instance() as RunListTest)._loadRuns({}); expect(props.onError).not.toHaveBeenCalled(); expect(tree).toMatchSnapshot(); }); @@ -241,91 +247,102 @@ describe('RunList', () => { getExperimentSpy.mockImplementationOnce(() => ({ name: 'test experiment' })); const props = generateProps(); const tree = shallow(); - await (tree.instance() as any)._loadRuns({}); + await (tree.instance() as RunListTest)._loadRuns({}); expect(props.onError).not.toHaveBeenCalled(); expect(tree).toMatchSnapshot(); }); it('renders run name as link to its details page', () => { - const tree = TestUtils.mountWithRouter((RunList.prototype as any) + const tree = TestUtils.mountWithRouter((RunList.prototype as RunListTest) ._nameCustomRenderer('test run', 'run-id')); expect(tree).toMatchSnapshot(); }); it('renders pipeline name as link to its details page', () => { - const tree = TestUtils.mountWithRouter((RunList.prototype as any) - ._pipelineCustomRenderer({ displayName: 'test pipeline', id: 'pipeline-id' })); + const tree = TestUtils.mountWithRouter(new RunList(generateProps()) + ._pipelineCustomRenderer({ displayName: 'test pipeline', id: 'pipeline-id', showLink: false }, 'run-id')); + expect(tree).toMatchSnapshot(); + }); + + it('handles no pipeline id given', () => { + const tree = TestUtils.mountWithRouter(new RunListTest(generateProps()) + ._pipelineCustomRenderer({ displayName: 'test pipeline' } as any, 'run-id')); + expect(tree).toMatchSnapshot(); + }); + + it('shows "View pipeline" button if pipeline is embedded in run', () => { + const tree = TestUtils.mountWithRouter(new RunListTest(generateProps())._pipelineCustomRenderer( + { displayName: 'test pipeline', id: 'pipeline-id', showLink: true }, 'test-run-id')); expect(tree).toMatchSnapshot(); }); - it('renders no pipeline name', () => { - const tree = TestUtils.mountWithRouter((RunList.prototype as any) - ._pipelineCustomRenderer()); + it('handles no pipeline name', () => { + const tree = TestUtils.mountWithRouter(new RunListTest(generateProps()) + ._pipelineCustomRenderer(undefined as any, 'run-id')); expect(tree).toMatchSnapshot(); }); it('renders experiment name as link to its details page', () => { - const tree = TestUtils.mountWithRouter((RunList.prototype as any) - ._experimentCustomRenderer({ displayName: 'test experiment', id: 'experiment-id' })); + const tree = TestUtils.mountWithRouter(RunListTest.prototype._experimentCustomRenderer( + { displayName: 'test experiment', id: 'experiment-id' })); expect(tree).toMatchSnapshot(); }); it('renders no experiment name', () => { - const tree = TestUtils.mountWithRouter((RunList.prototype as any) - ._experimentCustomRenderer()); + const tree = TestUtils.mountWithRouter(RunListTest.prototype._experimentCustomRenderer()); expect(tree).toMatchSnapshot(); }); it('renders status as icon', () => { - const tree = shallow((RunList.prototype as any)._statusCustomRenderer(NodePhase.SUCCEEDED)); + const tree = shallow(RunListTest.prototype._statusCustomRenderer(NodePhase.SUCCEEDED)); expect(tree).toMatchSnapshot(); }); it('renders metric buffer', () => { - const tree = shallow((RunList.prototype as any)._metricBufferCustomRenderer()); + const tree = shallow(RunListTest.prototype._metricBufferCustomRenderer()); expect(tree).toMatchSnapshot(); }); it('renders an empty metric when there is no data', () => { - const noMetricTree = shallow((RunList.prototype as any)._metricCustomRenderer()); + const noMetricTree = shallow(RunListTest.prototype._metricCustomRenderer(undefined as any)); expect(noMetricTree).toMatchSnapshot(); - const emptyMetricTree = shallow((RunList.prototype as any)._metricCustomRenderer({})); + const emptyMetricTree = shallow(RunListTest.prototype._metricCustomRenderer({} as any)); expect(emptyMetricTree).toMatchSnapshot(); - const noMetricValueTree = shallow((RunList.prototype as any)._metricCustomRenderer({ metric: {} })); + const noMetricValueTree = shallow(RunListTest.prototype._metricCustomRenderer({ metric: {} } as any)); expect(noMetricValueTree).toMatchSnapshot(); }); it('renders a empty metric container when a metric has value of zero', () => { - const noMetricTree = shallow((RunList.prototype as any)._metricCustomRenderer( - { metric: { number_value: 0 } })); + const noMetricTree = shallow(RunListTest.prototype._metricCustomRenderer( + { metric: { number_value: 0 } } as any)); expect(noMetricTree).toMatchSnapshot(); }); it('renders a metric container when a percentage metric has value of zero', () => { - const noMetricTree = shallow((RunList.prototype as any)._metricCustomRenderer( - { metric: { number_value: 0, format: RunMetricFormat.PERCENTAGE } })); + const noMetricTree = shallow(RunListTest.prototype._metricCustomRenderer( + { metric: { number_value: 0, format: RunMetricFormat.PERCENTAGE } } as any)); expect(noMetricTree).toMatchSnapshot(); }); it('renders a metric container when a raw metric has value of zero', () => { - const noMetricTree = shallow((RunList.prototype as any)._metricCustomRenderer( + const noMetricTree = shallow(RunListTest.prototype._metricCustomRenderer( { - metadata: { maxValue: 10, minValue: 0 }, + metadata: { maxValue: 10, minValue: 0 } as any, metric: { number_value: 0, format: RunMetricFormat.RAW }, })); expect(noMetricTree).toMatchSnapshot(); }); it('renders percentage metric', () => { - const tree = shallow((RunList.prototype as any)._metricCustomRenderer( - { metric: { number_value: 0.3, format: RunMetricFormat.PERCENTAGE } as ApiRunMetric })); + const tree = shallow(RunListTest.prototype._metricCustomRenderer( + { metric: { number_value: 0.3, format: RunMetricFormat.PERCENTAGE } as ApiRunMetric } as any)); expect(tree).toMatchSnapshot(); }); it('renders raw metric', () => { - const tree = shallow((RunList.prototype as any)._metricCustomRenderer( + const tree = shallow(RunListTest.prototype._metricCustomRenderer( { metadata: { count: 1, maxValue: 100, minValue: 10 } as MetricMetadata, metric: { number_value: 55 } as ApiRunMetric, @@ -334,7 +351,7 @@ describe('RunList', () => { }); it('renders raw metric with zero max/min values', () => { - const tree = shallow((RunList.prototype as any)._metricCustomRenderer( + const tree = shallow(RunListTest.prototype._metricCustomRenderer( { metadata: { count: 1, maxValue: 0, minValue: 0 } as MetricMetadata, metric: { number_value: 15 } as ApiRunMetric, @@ -344,7 +361,7 @@ describe('RunList', () => { it('renders raw metric that is less than its min value, logs error to console', () => { const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); - const tree = shallow((RunList.prototype as any)._metricCustomRenderer( + const tree = shallow(RunListTest.prototype._metricCustomRenderer( { metadata: { count: 1, maxValue: 100, minValue: 10 } as MetricMetadata, metric: { number_value: 5 } as ApiRunMetric, @@ -355,7 +372,7 @@ describe('RunList', () => { it('renders raw metric that is greater than its max value, logs error to console', () => { const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); - const tree = shallow((RunList.prototype as any)._metricCustomRenderer( + const tree = shallow(RunListTest.prototype._metricCustomRenderer( { metadata: { count: 1, maxValue: 100, minValue: 10 } as MetricMetadata, metric: { number_value: 105 } as ApiRunMetric, diff --git a/frontend/src/pages/RunList.tsx b/frontend/src/pages/RunList.tsx index 8e369f7e22d..402512234b1 100644 --- a/frontend/src/pages/RunList.tsx +++ b/frontend/src/pages/RunList.tsx @@ -21,7 +21,8 @@ import { ApiRunDetail, ApiRun, ApiResourceType, RunMetricFormat, ApiRunMetric } import { Apis, RunSortKeys, ListRequest } from '../lib/Apis'; import { Link, RouteComponentProps } from 'react-router-dom'; import { NodePhase, statusToIcon } from './Status'; -import { RoutePage, RouteParams } from '../components/Router'; +import { RoutePage, RouteParams, QUERY_PARAMS } from '../components/Router'; +import { URLParser } from '../lib/URLParser'; import { Workflow } from '../../../frontend/third_party/argo-ui/argo_template'; import { commonCss, color } from '../Css'; import { getRunTime, formatDateString, logger, errorToMessage } from '../lib/Utils'; @@ -49,8 +50,10 @@ interface ExperimentInfo { } interface PipelineInfo { - displayName: string; - id: string; + displayName?: string; + id?: string; + runId?: string; + showLink: boolean; } interface DisplayRun { @@ -176,7 +179,101 @@ class RunList extends React.Component { } } - private async _loadRuns(request: ListRequest): Promise { + public _nameCustomRenderer(value: string, id: string): JSX.Element { + return e.stopPropagation()} + to={RoutePage.RUN_DETAILS.replace(':' + RouteParams.runId, id)}>{value}; + } + + public _pipelineCustomRenderer(pipelineInfo: PipelineInfo, id: string): JSX.Element { + // If the getPipeline call failed or a run has no pipeline, we display a placeholder. + if (!pipelineInfo || (!pipelineInfo.showLink && !pipelineInfo.id)) { + return
-
; + } + const search = new URLParser(this.props).build({ [QUERY_PARAMS.fromRunId]: id }); + const url = pipelineInfo.showLink ? + RoutePage.PIPELINE_DETAILS.replace(':' + RouteParams.pipelineId + '?', '') + search : + RoutePage.PIPELINE_DETAILS.replace(':' + RouteParams.pipelineId, pipelineInfo.id || ''); + return ( + e.stopPropagation()} + to={url}> + {pipelineInfo.showLink ? 'View pipeline' : pipelineInfo.displayName} + + ); + } + + public _experimentCustomRenderer(experimentInfo?: ExperimentInfo): JSX.Element { + // If the getExperiment call failed or a run has no experiment, we display a placeholder. + if (!experimentInfo || !experimentInfo.id) { + return
-
; + } + return ( + e.stopPropagation()} + to={RoutePage.EXPERIMENT_DETAILS.replace(':' + RouteParams.experimentId, experimentInfo.id)}> + {experimentInfo.displayName} + + ); + } + + public _statusCustomRenderer(status: NodePhase): JSX.Element { + return statusToIcon(status); + } + + public _metricBufferCustomRenderer(): JSX.Element { + return
; + } + + public _metricCustomRenderer(displayMetric: DisplayMetric): JSX.Element { + if (!displayMetric || !displayMetric.metric || + displayMetric.metric.number_value === undefined || + (displayMetric.metric.format !== RunMetricFormat.PERCENTAGE && !displayMetric.metadata)) { + return
; + } + + const leftSpace = 6; + let displayString = ''; + let width = ''; + + if (displayMetric.metric.format === RunMetricFormat.PERCENTAGE) { + displayString = (displayMetric.metric.number_value * 100).toFixed(3) + '%'; + width = `calc(${displayString})`; + } else { + displayString = displayMetric.metric.number_value.toFixed(3); + + if (displayMetric.metadata.maxValue === 0 && displayMetric.metadata.minValue === 0) { + return
{displayString}
; + } + + if (displayMetric.metric.number_value - displayMetric.metadata.minValue < 0) { + logger.error(`Run ${arguments[1]}'s metric ${displayMetric.metadata.name}'s value:` + + ` (${displayMetric.metric.number_value}) was lower than the supposed minimum of` + + ` (${displayMetric.metadata.minValue})`); + return
{displayString}
; + } + + if (displayMetric.metadata.maxValue - displayMetric.metric.number_value < 0) { + logger.error(`Run ${arguments[1]}'s metric ${displayMetric.metadata.name}'s value:` + + ` (${displayMetric.metric.number_value}) was greater than the supposed maximum of` + + ` (${displayMetric.metadata.maxValue})`); + return
{displayString}
; + } + + const barWidth = + (displayMetric.metric.number_value - displayMetric.metadata.minValue) + / (displayMetric.metadata.maxValue - displayMetric.metadata.minValue) + * 100; + + width = `calc(${barWidth}%)`; + } + return ( +
+
+ {displayString} +
+
+ ); + } + + protected async _loadRuns(request: ListRequest): Promise { let displayRuns: DisplayRun[] = []; let nextPageToken = ''; @@ -246,11 +343,13 @@ class RunList extends React.Component { if (pipelineId) { try { const pipeline = await Apis.pipelineServiceApi.getPipeline(pipelineId); - displayRun.pipeline = { displayName: pipeline.name || '', id: pipelineId }; + displayRun.pipeline = { displayName: pipeline.name || '', id: pipelineId, showLink: false }; } catch (err) { // This could be an API exception, or a JSON parse exception. displayRun.error = 'Failed to get associated pipeline: ' + await errorToMessage(err); } + } else if (!!RunUtils.getPipelineSpec(displayRun.metadata)) { + displayRun.pipeline = { showLink: true }; } return displayRun; }) @@ -283,96 +382,6 @@ class RunList extends React.Component { }) ); } - - private _nameCustomRenderer(value: string, id: string): JSX.Element { - return e.stopPropagation()} - to={RoutePage.RUN_DETAILS.replace(':' + RouteParams.runId, id)}>{value}; - } - - private _pipelineCustomRenderer(pipelineInfo?: PipelineInfo): JSX.Element { - // If the getPipeline call failed or a run has no pipeline, we display a placeholder. - if (!pipelineInfo || !pipelineInfo.id) { - return
-
; - } - return ( - e.stopPropagation()} - to={RoutePage.PIPELINE_DETAILS.replace(':' + RouteParams.pipelineId, pipelineInfo.id)}> - {pipelineInfo.displayName} - - ); - } - - private _experimentCustomRenderer(experimentInfo?: ExperimentInfo): JSX.Element { - // If the getExperiment call failed or a run has no experiment, we display a placeholder. - if (!experimentInfo || !experimentInfo.id) { - return
-
; - } - return ( - e.stopPropagation()} - to={RoutePage.EXPERIMENT_DETAILS.replace(':' + RouteParams.experimentId, experimentInfo.id)}> - {experimentInfo.displayName} - - ); - } - - private _statusCustomRenderer(status: NodePhase): JSX.Element { - return statusToIcon(status); - } - - private _metricBufferCustomRenderer(): JSX.Element { - return
; - } - - private _metricCustomRenderer(displayMetric: DisplayMetric): JSX.Element { - if (!displayMetric || !displayMetric.metric || - displayMetric.metric.number_value === undefined || - (displayMetric.metric.format !== RunMetricFormat.PERCENTAGE && !displayMetric.metadata)) { - return
; - } - - const leftSpace = 6; - let displayString = ''; - let width = ''; - - if (displayMetric.metric.format === RunMetricFormat.PERCENTAGE) { - displayString = (displayMetric.metric.number_value * 100).toFixed(3) + '%'; - width = `calc(${displayString})`; - } else { - displayString = displayMetric.metric.number_value.toFixed(3); - - if (displayMetric.metadata.maxValue === 0 && displayMetric.metadata.minValue === 0) { - return
{displayString}
; - } - - if (displayMetric.metric.number_value - displayMetric.metadata.minValue < 0) { - logger.error(`Run ${arguments[1]}'s metric ${displayMetric.metadata.name}'s value:` - + ` (${displayMetric.metric.number_value}) was lower than the supposed minimum of` - + ` (${displayMetric.metadata.minValue})`); - return
{displayString}
; - } - - if (displayMetric.metadata.maxValue - displayMetric.metric.number_value < 0) { - logger.error(`Run ${arguments[1]}'s metric ${displayMetric.metadata.name}'s value:` - + ` (${displayMetric.metric.number_value}) was greater than the supposed maximum of` - + ` (${displayMetric.metadata.maxValue})`); - return
{displayString}
; - } - - const barWidth = - (displayMetric.metric.number_value - displayMetric.metadata.minValue) - / (displayMetric.metadata.maxValue - displayMetric.metadata.minValue) - * 100; - - width = `calc(${barWidth}%)`; - } - return ( -
-
- {displayString} -
-
- ); - } } export default RunList; diff --git a/frontend/src/pages/__snapshots__/PipelineDetails.test.tsx.snap b/frontend/src/pages/__snapshots__/PipelineDetails.test.tsx.snap index 45d258be928..427c0192759 100644 --- a/frontend/src/pages/__snapshots__/PipelineDetails.test.tsx.snap +++ b/frontend/src/pages/__snapshots__/PipelineDetails.test.tsx.snap @@ -70,7 +70,7 @@ exports[`PipelineDetails closes side panel when close button is clicked 1`] = ` Description
- test job description + test pipeline description
-
+
Unable to retrieve node info
@@ -197,7 +199,9 @@ exports[`PipelineDetails collapses summary card when summary shown state is fals
-
+
Unable to retrieve node info
@@ -307,7 +311,7 @@ exports[`PipelineDetails opens side panel on clicked node, shows message when no Description
- test job description + test pipeline description
-
+
Unable to retrieve node info
@@ -443,7 +449,7 @@ exports[`PipelineDetails shows clicked node info in the side panel if it is in t Description
- test job description + test pipeline description
- test job description + test pipeline description
-
+
Unable to retrieve node info
@@ -801,7 +809,7 @@ exports[`PipelineDetails shows pipeline source code when config tab is clicked 1 "theme": "default", } } - value="" + value="test template" />
diff --git a/frontend/src/pages/__snapshots__/RunList.test.tsx.snap b/frontend/src/pages/__snapshots__/RunList.test.tsx.snap index f87476e85c9..4e9607723f1 100644 --- a/frontend/src/pages/__snapshots__/RunList.test.tsx.snap +++ b/frontend/src/pages/__snapshots__/RunList.test.tsx.snap @@ -389,6 +389,18 @@ exports[`RunList displays error in run row if pipeline could not be fetched 1`]
`; +exports[`RunList handles no pipeline id given 1`] = ` +
+ - +
+`; + +exports[`RunList handles no pipeline name 1`] = ` +
+ - +
+`; + exports[`RunList loads multiple runs 1`] = `
@@ -6664,12 +6680,6 @@ exports[`RunList renders no experiment name 1`] = `
`; -exports[`RunList renders no pipeline name 1`] = ` -
- - -
-`; - exports[`RunList renders percentage metric 1`] = `