diff --git a/frontend/src/pages/RecurringRunList.test.tsx b/frontend/src/pages/RecurringRunList.test.tsx index f7b34d7d4e5b..41055a44f856 100644 --- a/frontend/src/pages/RecurringRunList.test.tsx +++ b/frontend/src/pages/RecurringRunList.test.tsx @@ -36,7 +36,7 @@ describe('RecurringRunList', () => { const onErrorSpy = jest.fn(); const listRecurringRunsSpy = jest.spyOn(Apis.recurringRunServiceApi, 'listRecurringRuns'); const getRecurringRunSpy = jest.spyOn(Apis.recurringRunServiceApi, 'getRecurringRun'); - const getExperimentSpy = jest.spyOn(Apis.experimentServiceApiV2, 'getExperiment'); + const listExperimentsSpy = jest.spyOn(Apis.experimentServiceApiV2, 'listExperiments'); // We mock this because it uses toLocaleDateString, which causes mismatches between local and CI // test environments const formatDateStringSpy = jest.spyOn(Utils, 'formatDateString'); @@ -78,7 +78,7 @@ describe('RecurringRunList', () => { }), ); - getExperimentSpy.mockImplementation(() => ({ display_name: 'some experiment' })); + listExperimentsSpy.mockImplementation(() => ({ display_name: 'some experiment' })); } function getMountedInstance(): RecurringRunList { @@ -93,7 +93,7 @@ describe('RecurringRunList', () => { onErrorSpy.mockClear(); listRecurringRunsSpy.mockClear(); getRecurringRunSpy.mockClear(); - getExperimentSpy.mockClear(); + listExperimentsSpy.mockClear(); }); afterEach(async () => { @@ -613,7 +613,14 @@ describe('RecurringRunList', () => { mockNRecurringRuns(1, { experiment_id: 'test-experiment-id', }); - getExperimentSpy.mockImplementationOnce(() => ({ display_name: 'test experiment' })); + listExperimentsSpy.mockImplementationOnce(() => ({ + experiments: [ + { + experiment_id: 'test-experiment-id', + display_name: 'test experiment', + }, + ], + })); const props = generateProps(); tree = shallow(); await (tree.instance() as RecurringRunListTest)._loadRecurringRuns({}); @@ -682,7 +689,7 @@ describe('RecurringRunList', () => { mockNRecurringRuns(1, { experiment_id: 'test-experiment-id', }); - getExperimentSpy.mockImplementationOnce(() => ({ display_name: 'test experiment' })); + listExperimentsSpy.mockImplementationOnce(() => ({ display_name: 'test experiment' })); const props = generateProps(); props.hideExperimentColumn = true; tree = shallow(); diff --git a/frontend/src/pages/RecurringRunList.tsx b/frontend/src/pages/RecurringRunList.tsx index 4d1b514cd9ba..47b2ce878e2f 100644 --- a/frontend/src/pages/RecurringRunList.tsx +++ b/frontend/src/pages/RecurringRunList.tsx @@ -28,6 +28,7 @@ import { V2beta1RecurringRunStatus, V2beta1Trigger, } from 'src/apisv2beta1/recurringrun'; +import { V2beta1ListExperimentsResponse } from 'src/apisv2beta1/experiment'; interface DisplayRecurringRun { experiment?: ExperimentInfo; @@ -269,10 +270,35 @@ class RecurringRunList extends React.PureComponent { + let experimentsResponse: V2beta1ListExperimentsResponse; + let experimentsGetError: string; + try { + experimentsResponse = await Apis.experimentServiceApiV2.listExperiments(); + } catch (error) { + experimentsGetError = 'Failed to get associated experiment: ' + (await errorToMessage(error)); + } + return Promise.all( - displayRecurringRuns.map(async displayRecurringRun => { + displayRecurringRuns.map(displayRecurringRun => { if (!this.props.hideExperimentColumn) { - await this._getAndSetExperimentNames(displayRecurringRun); + const experimentId = displayRecurringRun.recurringRun.experiment_id; + + if (experimentId) { + const experiment = experimentsResponse?.experiments?.find( + e => e.experiment_id === displayRecurringRun.recurringRun.experiment_id, + ); + // If matching experiment id not found (typically because it has + // been deleted), set display name to "-". + const displayName = experiment?.display_name || '-'; + if (experimentsGetError) { + displayRecurringRun.error = experimentsGetError; + } else { + displayRecurringRun.experiment = { + displayName: displayName, + id: experimentId, + }; + } + } } return displayRecurringRun; }), @@ -301,31 +327,6 @@ class RecurringRunList extends React.PureComponent { - const experimentId = displayRecurringRun.recurringRun.experiment_id; - if (experimentId) { - let experimentName; - try { - const experiment = await Apis.experimentServiceApiV2.getExperiment(experimentId); - experimentName = experiment.display_name || ''; - } catch (err) { - displayRecurringRun.error = - 'Failed to get associated experiment: ' + (await errorToMessage(err)); - return; - } - displayRecurringRun.experiment = { - displayName: experimentName, - id: experimentId, - }; - } - } } export default RecurringRunList; diff --git a/frontend/src/pages/RunList.test.tsx b/frontend/src/pages/RunList.test.tsx index 69b58fc958c6..f39609a99118 100644 --- a/frontend/src/pages/RunList.test.tsx +++ b/frontend/src/pages/RunList.test.tsx @@ -40,7 +40,7 @@ describe('RunList', () => { const listRunsSpy = jest.spyOn(Apis.runServiceApiV2, 'listRuns'); const getRunSpy = jest.spyOn(Apis.runServiceApiV2, 'getRun'); const getPipelineVersionSpy = jest.spyOn(Apis.pipelineServiceApiV2, 'getPipelineVersion'); - const getExperimentSpy = jest.spyOn(Apis.experimentServiceApiV2, 'getExperiment'); + const listExperimentsSpy = jest.spyOn(Apis.experimentServiceApiV2, 'listExperiments'); // We mock this because it uses toLocaleDateString, which causes mismatches between local and CI // test enviroments const formatDateStringSpy = jest.spyOn(Utils, 'formatDateString'); @@ -97,7 +97,7 @@ describe('RunList', () => { ); getPipelineVersionSpy.mockImplementation(() => ({ display_name: 'some pipeline version' })); - getExperimentSpy.mockImplementation(() => ({ display_name: 'some experiment' })); + listExperimentsSpy.mockImplementation(() => ({ display_name: 'some experiment' })); } function getMountedInstance(): RunList { @@ -117,7 +117,7 @@ describe('RunList', () => { onErrorSpy.mockClear(); listRunsSpy.mockClear(); getRunSpy.mockClear(); - getExperimentSpy.mockClear(); + listExperimentsSpy.mockClear(); }); afterEach(async () => { @@ -302,7 +302,7 @@ describe('RunList', () => { mockNRuns(1, { experiment_id: 'test-experiment-id', }); - TestUtils.makeErrorResponseOnce(getExperimentSpy, 'bad stuff happened'); + TestUtils.makeErrorResponseOnce(listExperimentsSpy, 'bad stuff happened'); const props = generateProps(); render( @@ -312,7 +312,7 @@ describe('RunList', () => { ); await waitFor(() => { expect(listRunsSpy).toHaveBeenCalled(); - expect(getExperimentSpy).toHaveBeenCalled(); + expect(listExperimentsSpy).toHaveBeenCalled(); }); screen.findByText('Failed to get associated experiment: bad stuff happened'); @@ -488,7 +488,14 @@ describe('RunList', () => { mockNRuns(1, { experiment_id: 'test-experiment-id', }); - getExperimentSpy.mockImplementationOnce(() => ({ display_name: 'test experiment' })); + listExperimentsSpy.mockImplementationOnce(() => ({ + experiments: [ + { + experiment_id: 'test-experiment-id', + display_name: 'test experiment', + }, + ], + })); const props = generateProps(); render( @@ -497,7 +504,7 @@ describe('RunList', () => { ); await waitFor(() => { expect(listRunsSpy).toHaveBeenCalled(); - expect(getExperimentSpy).toHaveBeenCalled(); + expect(listExperimentsSpy).toHaveBeenCalled(); }); screen.getByText('test experiment'); @@ -507,7 +514,7 @@ describe('RunList', () => { mockNRuns(1, { experiment_id: 'test-experiment-id', }); - getExperimentSpy.mockImplementationOnce(() => ({ display_name: 'test experiment' })); + listExperimentsSpy.mockImplementationOnce(() => ({ display_name: 'test experiment' })); const props = generateProps(); props.hideExperimentColumn = true; render( @@ -517,7 +524,7 @@ describe('RunList', () => { ); await waitFor(() => { expect(listRunsSpy).toHaveBeenCalled(); - expect(getExperimentSpy).toHaveBeenCalledTimes(0); + expect(listExperimentsSpy).toHaveBeenCalledTimes(0); }); expect(screen.queryByText('test experiment')).toBeNull(); diff --git a/frontend/src/pages/RunList.tsx b/frontend/src/pages/RunList.tsx index 20c78a3e33ff..871cf5f97aa6 100644 --- a/frontend/src/pages/RunList.tsx +++ b/frontend/src/pages/RunList.tsx @@ -19,6 +19,7 @@ import CustomTable, { Column, Row, CustomRendererProps } from 'src/components/Cu import Metric from 'src/components/Metric'; import { MetricMetadata, ExperimentInfo } from 'src/lib/RunUtils'; import { V2beta1Run, V2beta1RuntimeState, V2beta1RunStorageState } from 'src/apisv2beta1/run'; +import { V2beta1ListExperimentsResponse } from 'src/apisv2beta1/experiment'; import { Apis, RunSortKeys, ListRequest } from 'src/lib/Apis'; import { Link, RouteComponentProps } from 'react-router-dom'; import { V2beta1Filter, V2beta1PredicateOperation } from 'src/apisv2beta1/filter'; @@ -405,6 +406,14 @@ class RunList extends React.PureComponent { } private async _setColumns(displayRuns: DisplayRun[]): Promise { + let experimentsResponse: V2beta1ListExperimentsResponse; + let experimentsGetError: string; + try { + experimentsResponse = await Apis.experimentServiceApiV2.listExperiments(); + } catch (error) { + experimentsGetError = 'Failed to get associated experiment: ' + (await errorToMessage(error)); + } + return Promise.all( displayRuns.map(async displayRun => { this._setRecurringRun(displayRun); @@ -412,7 +421,23 @@ class RunList extends React.PureComponent { await this._getAndSetPipelineVersionNames(displayRun); if (!this.props.hideExperimentColumn) { - await this._getAndSetExperimentNames(displayRun); + const experimentId = displayRun.run.experiment_id; + + if (experimentId) { + const experiment = experimentsResponse?.experiments?.find( + e => e.experiment_id === displayRun.run.experiment_id, + ); + // If matching experiment id not found (typically because it has been deleted), set display name to "-". + const displayName = experiment?.display_name || '-'; + if (experimentsGetError) { + displayRun.error = experimentsGetError; + } else { + displayRun.experiment = { + displayName: displayName, + id: experimentId, + }; + } + } } return displayRun; }), @@ -478,30 +503,6 @@ class RunList extends React.PureComponent { : { usePlaceholder: true }; } } - - /** - * For the given DisplayRun, get its ApiRun and retrieve that ApiRun's Experiment ID if it has - * one, then use that Experiment ID to fetch its associated Experiment and attach that - * Experiment's name to the DisplayRun. If the ApiRun has no Experiment ID, then the corresponding - * DisplayRun will show '-'. - */ - private async _getAndSetExperimentNames(displayRun: DisplayRun): Promise { - const experimentId = displayRun.run.experiment_id; - if (experimentId) { - let experimentName; - try { - const experiment = await Apis.experimentServiceApiV2.getExperiment(experimentId); - experimentName = experiment.display_name || ''; - } catch (err) { - displayRun.error = 'Failed to get associated experiment: ' + (await errorToMessage(err)); - return; - } - displayRun.experiment = { - displayName: experimentName, - id: experimentId, - }; - } - } } export default RunList;