diff --git a/frontend/src/components/CustomTable.test.tsx b/frontend/src/components/CustomTable.test.tsx index df79c04250c..c79a78662f2 100644 --- a/frontend/src/components/CustomTable.test.tsx +++ b/frontend/src/components/CustomTable.test.tsx @@ -85,7 +85,7 @@ describe('CustomTable', () => { }); it('renders some columns with descending sort order on first column', () => { - const tree = shallow(); expect(tree).toMatchSnapshot(); }); @@ -105,7 +105,7 @@ describe('CustomTable', () => { it('calls reload function with an empty page token to get rows', () => { const reload = jest.fn(); shallow(); - expect(reload).toHaveBeenLastCalledWith({ pageToken: '' }); + expect(reload).toHaveBeenLastCalledWith({ pageToken: '', pageSize: 10, orderAscending: false, sortBy: '' }); }); it('calls reload function with sort key of clicked column, while keeping same page', () => { @@ -120,11 +120,11 @@ describe('CustomTable', () => { }]; const reload = jest.fn(); const tree = shallow(); - expect(reload).toHaveBeenLastCalledWith({ pageToken: '' }); + expect(reload).toHaveBeenLastCalledWith({ pageToken: '', pageSize: 10, sortBy: 'col1sortkey desc', orderAscending: false }); - tree.find('WithStyles(TableSortLabel)').at(0).simulate('click'); + tree.find('WithStyles(TableSortLabel)').at(1).simulate('click'); expect(reload).toHaveBeenLastCalledWith({ - orderAscending: true, pageToken: '', sortBy: 'col1sortkey' + orderAscending: true, pageSize: 10, pageToken: '', sortBy: 'col2sortkey', }); }); @@ -140,16 +140,16 @@ describe('CustomTable', () => { }]; const reload = jest.fn(); const tree = shallow(); - expect(reload).toHaveBeenLastCalledWith({ pageToken: '' }); + expect(reload).toHaveBeenLastCalledWith({ pageToken: '', orderAscending: false, pageSize: 10, sortBy: 'col1sortkey desc' }); - tree.find('WithStyles(TableSortLabel)').at(0).simulate('click'); - expect(reload).toHaveBeenLastCalledWith({ - orderAscending: true, pageToken: '', sortBy: 'col1sortkey' + tree.find('WithStyles(TableSortLabel)').at(1).simulate('click'); + expect(reload).toHaveBeenLastCalledWith({ + orderAscending: true, pageSize: 10, pageToken: '', sortBy: 'col2sortkey', }); tree.setProps({ sortBy: 'col1sortkey' }); - tree.find('WithStyles(TableSortLabel)').at(0).simulate('click'); + tree.find('WithStyles(TableSortLabel)').at(1).simulate('click'); expect(reload).toHaveBeenLastCalledWith({ - orderAscending: false, pageToken: '', sortBy: 'col1sortkey' + orderAscending: false, pageSize: 10, pageToken: '', sortBy: 'col2sortkey desc' }); }); @@ -163,10 +163,10 @@ describe('CustomTable', () => { }]; const reload = jest.fn(); const tree = shallow(); - expect(reload).toHaveBeenLastCalledWith({ pageToken: '' }); + expect(reload).toHaveBeenLastCalledWith({ pageToken: '', pageSize: 10, orderAscending: false, sortBy: '' }); tree.find('WithStyles(TableSortLabel)').at(0).simulate('click'); - expect(reload).toHaveBeenLastCalledWith({ pageToken: '' }); + expect(reload).toHaveBeenLastCalledWith({ pageToken: '', pageSize: 10, orderAscending: false, sortBy: '' }); }); it('logs error if row has more cells than columns', () => { @@ -299,7 +299,7 @@ describe('CustomTable', () => { await reloadResult; tree.find('WithStyles(IconButton)').at(1).simulate('click'); - expect(spy).toHaveBeenLastCalledWith({ pageToken: 'some token' }); + expect(spy).toHaveBeenLastCalledWith({ pageToken: 'some token', pageSize: 10, orderAscending: false, sortBy: '' }); }); it('renders new rows after clicking next page, and enables previous page button', async () => { @@ -310,7 +310,7 @@ describe('CustomTable', () => { tree.find('WithStyles(IconButton)').at(1).simulate('click'); await reloadResult; - expect(spy).toHaveBeenLastCalledWith({ pageToken: 'some token' }); + expect(spy).toHaveBeenLastCalledWith({ pageToken: 'some token', pageSize: 10, sortBy: '', orderAscending: false }); expect(tree.state()).toHaveProperty('currentPage', 1); tree.setProps({ rows: [rows[1]] }); expect(tree).toMatchSnapshot(); @@ -328,7 +328,7 @@ describe('CustomTable', () => { tree.find('WithStyles(IconButton)').at(0).simulate('click'); await reloadResult; - expect(spy).toHaveBeenLastCalledWith({ pageToken: '' }); + expect(spy).toHaveBeenLastCalledWith({ pageToken: '', orderAscending: false, sortBy: '', pageSize: 10 }); tree.setProps({ rows }); expect(tree.find('WithStyles(IconButton)').at(0).prop('disabled')).toBeTruthy(); @@ -342,7 +342,7 @@ describe('CustomTable', () => { tree.find('.' + css.rowsPerPage).simulate('change', { target: { value: 1234 } }); await reloadResult; - expect(spy).toHaveBeenLastCalledWith({ pageSize: 1234, pageToken: '' }); + expect(spy).toHaveBeenLastCalledWith({ pageSize: 1234, pageToken: '', orderAscending: false, sortBy: '' }); expect(tree.state()).toHaveProperty('tokenList', ['', 'some token']); }); @@ -353,7 +353,7 @@ describe('CustomTable', () => { tree.find('.' + css.rowsPerPage).simulate('change', { target: { value: 1234 } }); await reloadResult; - expect(spy).toHaveBeenLastCalledWith({ pageSize: 1234, pageToken: '' }); + expect(spy).toHaveBeenLastCalledWith({ pageSize: 1234, pageToken: '', orderAscending: false, sortBy: '' }); expect(tree.state()).toHaveProperty('tokenList', ['']); }); diff --git a/frontend/src/components/CustomTable.tsx b/frontend/src/components/CustomTable.tsx index 3f0067ea647..5bd0f2d567f 100644 --- a/frontend/src/components/CustomTable.tsx +++ b/frontend/src/components/CustomTable.tsx @@ -25,7 +25,7 @@ import TableSortLabel from '@material-ui/core/TableSortLabel'; import TextField from '@material-ui/core/TextField'; import Tooltip from '@material-ui/core/Tooltip'; import WarningIcon from '@material-ui/icons/WarningRounded'; -import { BaseListRequest } from '../lib/Apis'; +import { ListRequest } from '../lib/Apis'; import Checkbox, { CheckboxProps } from '@material-ui/core/Checkbox'; import { TextFieldProps } from '@material-ui/core/TextField'; import { classes, stylesheet } from 'typestyle'; @@ -145,14 +145,13 @@ interface CustomTableProps { disablePaging?: boolean; disableSelection?: boolean; disableSorting?: boolean; - getExpandComponent?: (index: number) => React.ReactNode; emptyMessage?: string; - orderAscending: boolean; - pageSize: number; - reload: (request: BaseListRequest) => Promise; + getExpandComponent?: (index: number) => React.ReactNode; + initialSortColumn?: string; + initialSortOrder?: 'asc' | 'desc'; + reload: (request: ListRequest) => Promise; rows: Row[]; selectedIds?: string[]; - sortBy: string; toggleExpansion?: (rowId: number) => void; updateSelection?: (selectedIds: string[]) => void; useRadioButtons?: boolean; @@ -161,16 +160,23 @@ interface CustomTableProps { interface CustomTableState { currentPage: number; maxPageIndex: number; + sortOrder: 'asc' | 'desc'; + pageSize: number; + sortBy: string; tokenList: string[]; } export default class CustomTable extends React.Component { - constructor(props: any) { + constructor(props: CustomTableProps) { super(props); this.state = { currentPage: 0, maxPageIndex: Number.MAX_SAFE_INTEGER, + pageSize: 10, + sortBy: props.initialSortColumn || + (props.columns.length ? props.columns[0].sortKey || '' : ''), + sortOrder: props.initialSortOrder || 'desc', tokenList: [''], }; } @@ -218,7 +224,7 @@ export default class CustomTable extends React.Component total += (c.flex || 1), 0); const widths = this.props.columns.map(c => (c.flex || 1) / totalFlex * 100); @@ -228,7 +234,7 @@ export default class CustomTable extends React.Component + css.header, (this.props.disableSelection || this.props.useRadioButtons) && padding(20, 'l'))}> {(this.props.disableSelection !== true && this.props.useRadioButtons !== true) && (
- {disableSorting === true &&
{col.label}
} - {!disableSorting && ( + {this.props.disableSorting === true &&
{col.label}
} + {!this.props.disableSorting && ( this._requestSort(this.props.columns[i].sortKey)}> {col.label} @@ -340,10 +346,38 @@ export default class CustomTable extends React.Component { + // Override the current state with incoming request + const request: ListRequest = Object.assign({ + orderAscending: this.state.sortOrder === 'asc', + pageSize: this.state.pageSize, + pageToken: this.state.tokenList[this.state.currentPage], + sortBy: this.state.sortBy, + }, loadRequest); + + this.setState({ + pageSize: request.pageSize!, + sortBy: request.sortBy!, + sortOrder: request.orderAscending ? 'asc' : 'desc', + }); + + if (request.sortBy && !request.orderAscending) { + request.sortBy += ' desc'; + } + + return this.props.reload(request); + } + + private _requestSort(sortBy?: string) { if (sortBy) { - const orderAscending = this.props.sortBy === sortBy ? !this.props.orderAscending : true; - this._resetToFirstPage(await this.props.reload({ pageToken: '', orderAscending, sortBy })); + // Set the sort column to the provided column if it's different, and + // invert the sort order it if it's the same column + const sortOrder = this.state.sortBy === sortBy ? + (this.state.sortOrder === 'asc' ? 'desc' : 'asc') : 'asc'; + this.setState({ sortOrder, sortBy }, async () => { + this._resetToFirstPage( + await this.reload({ pageToken: '', orderAscending: sortOrder === 'asc', sortBy })); + }); } } @@ -353,7 +387,7 @@ export default class CustomTable extends React.Component { + private _tableRef = React.createRef(); constructor(props: any) { super(props); this.state = { displayExperiments: [], - orderAscending: false, - pageSize: 10, - pageToken: '', selectedExperimentIds: [], selectedRunIds: [], selectedTab: 0, @@ -126,37 +121,29 @@ class ExperimentList extends Page<{}, ExperimentListState> { return (
-
); } - public async load(loadRequest?: BaseListRequest): Promise { - await this._reload(loadRequest); + public async load() { + if (this._tableRef.current) { + this._tableRef.current.reload(); + } } - private async _reload(loadRequest?: BaseListRequest): Promise { - // Override the current state with incoming request - const request: ListExperimentsRequest = Object.assign({ - orderAscending: this.state.orderAscending, - pageSize: this.state.pageSize, - pageToken: this.state.pageToken, - sortBy: this.state.sortBy, - }, loadRequest); - + private async _reload(request: ListRequest): Promise { // Fetch the list of experiments let response: ApiListExperimentsResponse; let displayExperiments: DisplayExperiment[]; try { response = await Apis.experimentServiceApi.listExperiment( - request.pageToken, - request.pageSize, - request.sortBy ? request.sortBy + (request.orderAscending ? ' asc' : ' desc') : ''); + request.pageToken, request.pageSize, request.sortBy); displayExperiments = response.experiments || []; displayExperiments.forEach((exp) => exp.expandState = ExpandState.COLLAPSED); } catch (err) { @@ -185,18 +172,7 @@ class ExperimentList extends Page<{}, ExperimentListState> { } })); - // TODO: saw this warning: - // Warning: Can't call setState (or forceUpdate) on an unmounted component. - // This is a no-op, but it indicates a memory leak in your application. - // To fix, cancel all subscriptions and asynchronous tasks in the componentWillUnmount method. - this.setState({ - displayExperiments, - orderAscending: request.orderAscending!, - pageSize: request.pageSize!, - pageToken: request.pageToken!, - sortBy: request.sortBy!, - }); - + this.setState({ displayExperiments, sortBy: request.sortBy! }); return response.next_page_token || ''; } diff --git a/frontend/src/pages/PipelineList.tsx b/frontend/src/pages/PipelineList.tsx index 9b39379d3f3..8c433ea3dca 100644 --- a/frontend/src/pages/PipelineList.tsx +++ b/frontend/src/pages/PipelineList.tsx @@ -19,7 +19,7 @@ import CustomTable, { Column, Row } from '../components/CustomTable'; import AddIcon from '@material-ui/icons/Add'; import UploadPipelineDialog from '../components/UploadPipelineDialog'; import { ApiPipeline, ApiListPipelinesResponse } from '../apis/pipeline'; -import { Apis, PipelineSortKeys, BaseListRequest, ListPipelinesRequest } from '../lib/Apis'; +import { Apis, PipelineSortKeys, ListRequest } from '../lib/Apis'; import { Link } from 'react-router-dom'; import { Page } from './Page'; import { RoutePage, RouteParams } from '../components/Router'; @@ -28,27 +28,20 @@ import { commonCss, padding } from '../Css'; import { logger, formatDateString, errorToMessage } from '../lib/Utils'; interface PipelineListState { - orderAscending: boolean; - pageSize: number; - pageToken: string; pipelines: ApiPipeline[]; selectedIds: string[]; - sortBy: string; uploadDialogOpen: boolean; } class PipelineList extends Page<{}, PipelineListState> { + private _tableRef = React.createRef(); constructor(props: any) { super(props); this.state = { - orderAscending: false, - pageSize: 10, - pageToken: '', pipelines: [], selectedIds: [], - sortBy: PipelineSortKeys.CREATED_AT, uploadDialogOpen: false, }; } @@ -63,7 +56,7 @@ class PipelineList extends Page<{}, PipelineListState> { title: 'Upload pipeline', tooltip: 'Upload pipeline', }, { - action: () => this._reload(), + action: () => this.load(), id: 'refreshBtn', title: 'Refresh', tooltip: 'Refresh', @@ -107,8 +100,7 @@ class PipelineList extends Page<{}, PipelineListState> { return (
- @@ -120,36 +112,21 @@ class PipelineList extends Page<{}, PipelineListState> { } public async load(): Promise { - this._reload(); + if (this._tableRef.current) { + this._tableRef.current.reload(); + } } - private async _reload(loadRequest?: BaseListRequest): Promise { - // Override the current state with incoming request - const request: ListPipelinesRequest = Object.assign({ - orderAscending: this.state.orderAscending, - pageSize: this.state.pageSize, - pageToken: this.state.pageToken, - sortBy: this.state.sortBy, - }, loadRequest); - + private async _reload(request: ListRequest): Promise { let response: ApiListPipelinesResponse | null = null; try { response = await Apis.pipelineServiceApi.listPipelines( - request.pageToken, - request.pageSize, - request.sortBy ? request.sortBy + (request.orderAscending ? ' asc' : ' desc') : '' - ); + request.pageToken, request.pageSize, request.sortBy); } catch (err) { await this.showPageError('Error: failed to retrieve list of pipelines.', err); } - this.setState({ - orderAscending: request.orderAscending!, - pageSize: request.pageSize!, - pageToken: request.pageToken!, - pipelines: response ? response.pipelines || [] : [], - sortBy: request.sortBy!, - }); + this.setState({ pipelines: response ? response.pipelines || [] : [] }); return response ? response.next_page_token || '' : ''; } diff --git a/frontend/src/pages/PipelineSelector.tsx b/frontend/src/pages/PipelineSelector.tsx index f15939e2b26..3f6f35294db 100644 --- a/frontend/src/pages/PipelineSelector.tsx +++ b/frontend/src/pages/PipelineSelector.tsx @@ -17,7 +17,7 @@ import * as React from 'react'; import CustomTable, { Column, Row } from '../components/CustomTable'; import Toolbar, { ToolbarActionConfig } from '../components/Toolbar'; -import { Apis, BaseListRequest, ListPipelinesRequest, PipelineSortKeys } from '../lib/Apis'; +import { Apis, ListRequest, PipelineSortKeys } from '../lib/Apis'; import { RouteComponentProps } from 'react-router-dom'; import { logger, formatDateString, errorToMessage } from '../lib/Utils'; import { ApiPipeline } from '../apis/pipeline'; @@ -29,9 +29,6 @@ interface PipelineSelectorProps extends RouteComponentProps { } interface PipelineSelectorState { - orderAscending: boolean; - pageSize: number; - pageToken: string; pipelines: ApiPipeline[]; selectedIds: string[]; sortBy: string; @@ -39,14 +36,12 @@ interface PipelineSelectorState { } class PipelineSelector extends React.Component { + private _tableRef = React.createRef(); constructor(props: any) { super(props); this.state = { - orderAscending: false, - pageSize: 10, - pageToken: '', pipelines: [], selectedIds: [], sortBy: PipelineSortKeys.CREATED_AT, @@ -55,7 +50,7 @@ class PipelineSelector extends React.Component - { this._pipelineSelectionChanged(ids); this.setState({ selectedIds: ids });}} sortBy={sortBy} + { this._pipelineSelectionChanged(ids); this.setState({ selectedIds: ids }); }} + initialSortColumn={sortBy} reload={this._loadPipelines.bind(this)} emptyMessage={'No pipelines found. Upload a pipeline and then try again.'} /> ); } public async load() { - await this._loadPipelines(); + if (this._tableRef.current) { + this._tableRef.current.reload(); + } } private _pipelineSelectionChanged(selectedIds: string[]): void { @@ -98,23 +95,12 @@ class PipelineSelector extends React.Component { - // Override the current state with incoming request - const request: ListPipelinesRequest = Object.assign({ - orderAscending: this.state.orderAscending, - pageSize: this.state.pageSize, - pageToken: this.state.pageToken, - sortBy: this.state.sortBy, - }, loadRequest); - + private async _loadPipelines(request: ListRequest): Promise { let pipelines: ApiPipeline[] = []; let nextPageToken = ''; try { const response = await Apis.pipelineServiceApi.listPipelines( - request.pageToken, - request.pageSize, - request.sortBy ? request.sortBy + (request.orderAscending ? ' asc' : ' desc') : '', - ); + request.pageToken, request.pageSize, request.sortBy); pipelines = response.pipelines || []; nextPageToken = response.next_page_token || ''; } catch (err) { @@ -127,14 +113,7 @@ class PipelineSelector extends React.Component; - orderAscending: boolean; - pageSize: number; - pageToken: string; runs: ApiJob[]; selectedIds: string[]; sortBy: string; @@ -45,15 +42,13 @@ interface RecurringRunListState { } class RecurringRunsManager extends React.Component { + private _tableRef = React.createRef(); constructor(props: any) { super(props); this.state = { busyIds: new Set(), - orderAscending: false, - pageSize: 10, - pageToken: '', runs: [], selectedIds: [], sortBy: JobSortKeys.CREATED_AT, @@ -62,7 +57,7 @@ class RecurringRunsManager extends React.Component - this.setState({ selectedIds: ids })} sortBy={sortBy} + this.setState({ selectedIds: ids })} initialSortColumn={sortBy} reload={this._loadRuns.bind(this)} emptyMessage={'No recurring runs found in this experiment.'} /> ); } public async load() { - await this._loadRuns(); + if (this._tableRef.current) { + this._tableRef.current.reload(); + } } - private async _loadRuns(loadRequest?: BaseListRequest): Promise { - // Override the current state with incoming request - const request: ListJobsRequest = Object.assign({ - experimentId: this.props.experimentId, - orderAscending: this.state.orderAscending, - pageSize: this.state.pageSize, - pageToken: this.state.pageToken, - sortBy: this.state.sortBy, - }, loadRequest); - + private async _loadRuns(request: ListRequest): Promise { let runs: ApiJob[] = []; let nextPageToken = ''; try { const response = await Apis.jobServiceApi.listJobs( request.pageToken, request.pageSize, - request.sortBy ? request.sortBy + (request.orderAscending ? ' asc' : ' desc') : '', + request.sortBy, + ApiResourceType.EXPERIMENT.toString(), + this.props.experimentId, ); runs = response.jobs || []; nextPageToken = response.next_page_token || ''; @@ -130,14 +120,7 @@ class RecurringRunsManager extends React.Component { pipeline_runtime: { workflow_manifest: '' }, run: {}, } as ApiRunDetail), - listRuns: () => Promise.resolve({ + listRuns: jest.fn(() => Promise.resolve({ runs: [{ id: 'testrun1', name: 'test run1', } as ApiRun], - }), + })), }; const props = generateProps(); const tree = shallow(); - await (tree.instance() as RunList).refresh(); + await (tree.instance() as any)._loadRuns({}); + expect(Apis.runServiceApi.listRuns).toHaveBeenLastCalledWith(undefined, undefined, undefined, undefined, undefined); expect(props.onError).not.toHaveBeenCalled(); expect(tree).toMatchSnapshot(); }); @@ -67,7 +68,7 @@ describe('RunList', () => { }; const props = generateProps(); const tree = shallow(); - await (tree.instance() as RunList).refresh(); + await (tree.instance() as any)._loadRuns({}); expect(props.onError).toHaveBeenLastCalledWith('Error: failed to fetch runs.', 'bad stuff'); }); @@ -88,7 +89,7 @@ describe('RunList', () => { }; const props = generateProps(); const tree = shallow(); - await (tree.instance() as RunList).refresh(); + await (tree.instance() as any)._loadRuns({}); expect(tree).toMatchSnapshot(); }); @@ -105,7 +106,7 @@ describe('RunList', () => { const props = generateProps(); props.runIdListMask = ['run1', 'run2']; const tree = shallow(); - await (tree.instance() as RunList).refresh(); + await (tree.instance() as any)._loadRuns({}); expect(tree).toMatchSnapshot(); }); @@ -132,29 +133,28 @@ describe('RunList', () => { }; const props = generateProps(); const tree = shallow(); - await (tree.instance() as RunList).refresh(); + await (tree.instance() as any)._loadRuns({}); expect(props.onError).not.toHaveBeenCalled(); expect(tree).toMatchSnapshot(); }); it('loads runs for a given experiment id', async () => { - const listRunsSpy = jest.fn(() => Promise.resolve({ - runs: [{ id: 'testRun1' }], - })); (Apis as any).runServiceApi = { getRun: () => Promise.resolve({ pipeline_runtime: { workflow_manifest: '' }, run: {}, } as ApiRunDetail), - listRuns: listRunsSpy, + listRuns: jest.fn(() => Promise.resolve({ + runs: [{ id: 'testRun1' }], + })), }; const props = generateProps(); props.experimentIdMask = 'experiment1'; const tree = shallow(); - await (tree.instance() as RunList).refresh(); + await (tree.instance() as any)._loadRuns({ pageSize: 10, pageToken: '', orderAscending: true }); expect(props.onError).not.toHaveBeenCalled(); - expect(listRunsSpy).toHaveBeenLastCalledWith( - '', 10, 'created_at desc', ApiResourceType.EXPERIMENT.toString(), 'experiment1'); + expect(Apis.runServiceApi.listRuns).toHaveBeenLastCalledWith( + '', 10, undefined, ApiResourceType.EXPERIMENT.toString(), 'experiment1'); expect(tree).toMatchSnapshot(); }); @@ -171,7 +171,7 @@ describe('RunList', () => { const props = generateProps(); props.runIdListMask = ['run1', 'run2']; const tree = shallow(); - await (tree.instance() as RunList).refresh(); + await (tree.instance() as any)._loadRuns({}); expect(props.onError).not.toHaveBeenCalled(); expect(listRunsSpy).not.toHaveBeenCalled(); expect(getRunSpy).toHaveBeenCalledWith('run1'); diff --git a/frontend/src/pages/RunList.tsx b/frontend/src/pages/RunList.tsx index 531a4b4bec7..65629120b7a 100644 --- a/frontend/src/pages/RunList.tsx +++ b/frontend/src/pages/RunList.tsx @@ -17,7 +17,7 @@ import * as React from 'react'; import CustomTable, { Column, Row } from '../components/CustomTable'; import RunUtils from '../../src/lib/RunUtils'; -import { Apis, RunSortKeys, BaseListRequest, ListRunsRequest } from '../lib/Apis'; +import { Apis, RunSortKeys, ListRequest } from '../lib/Apis'; import { ApiListRunsResponse, ApiRunDetail, ApiRun, ApiResourceType, RunMetricFormat, ApiRunMetric } from '../../src/apis/run'; import { Link, RouteComponentProps } from 'react-router-dom'; import { NodePhase, statusToIcon } from './Status'; @@ -70,23 +70,18 @@ export interface RunListProps extends RouteComponentProps { interface RunListState { metrics: MetricMetadata[]; - orderAscending: boolean; - pageSize: number; - pageToken: string; runs: DisplayRun[]; sortBy: string; } class RunList extends React.Component { + private _tableRef = React.createRef(); constructor(props: any) { super(props); this.state = { metrics: [], - orderAscending: false, - pageSize: 10, - pageToken: '', runs: [], sortBy: RunSortKeys.CREATED_AT, }; @@ -157,21 +152,21 @@ class RunList extends React.Component { return row; }); - return ( -
- -
- ); + return (
+ +
); } public async refresh() { - await this._loadRuns(); + if (this._tableRef.current) { + this._tableRef.current.reload(); + } } private _metricBufferCustomRenderer() { @@ -199,15 +194,15 @@ class RunList extends React.Component { 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})`); + + ` ${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})`); + + ` ${displayMetric.metric.number_value}) was greater than the supposed maximum of` + + ` ${displayMetric.metadata.maxValue})`); return
{displayString}
; } @@ -227,7 +222,7 @@ class RunList extends React.Component { ); } - private async _loadRuns(loadRequest?: BaseListRequest): Promise { + private async _loadRuns(loadRequest: ListRequest): Promise { if (Array.isArray(this.props.runIdListMask)) { return await this._loadSpecificRuns(this.props.runIdListMask); } @@ -254,24 +249,15 @@ class RunList extends React.Component { return ''; } - private async _loadAllRuns(loadRequest?: BaseListRequest): Promise { - // Override the current state with incoming request - const request: ListRunsRequest = Object.assign({ - experimentId: this.props.experimentIdMask, - orderAscending: this.state.orderAscending, - pageSize: this.state.pageSize, - pageToken: this.state.pageToken, - sortBy: this.state.sortBy, - }, loadRequest); - + private async _loadAllRuns(request: ListRequest): Promise { let response: ApiListRunsResponse; try { response = await Apis.runServiceApi.listRuns( request.pageToken, request.pageSize, - request.sortBy ? request.sortBy + (request.orderAscending ? ' asc' : ' desc') : '', - request.experimentId ? ApiResourceType.EXPERIMENT.toString() : undefined, - request.experimentId, + request.sortBy, + this.props.experimentIdMask ? ApiResourceType.EXPERIMENT.toString() : undefined, + this.props.experimentIdMask, ); } catch (err) { this.props.onError('Error: failed to fetch runs.', err); @@ -299,9 +285,6 @@ class RunList extends React.Component { this.setState({ metrics: this._extractMetricMetadata(displayRuns), - orderAscending: request.orderAscending!, - pageSize: request.pageSize!, - pageToken: request.pageToken!, runs: displayRuns, sortBy: request.sortBy!, }); @@ -318,17 +301,17 @@ class RunList extends React.Component { return await Promise.all( displayRuns.map(async (displayRun) => { const pipelineId = RunUtils.getPipelineId(displayRun.metadata); - if (pipelineId) { - try { - const pipeline = await Apis.pipelineServiceApi.getPipeline(pipelineId); - displayRun.pipeline = { displayName: pipeline.name || '', id: pipelineId }; - } catch (err) { - // This could be an API exception, or a JSON parse exception. - displayRun.error = await errorToMessage(err); - } + if (pipelineId) { + try { + const pipeline = await Apis.pipelineServiceApi.getPipeline(pipelineId); + displayRun.pipeline = { displayName: pipeline.name || '', id: pipelineId }; + } catch (err) { + // This could be an API exception, or a JSON parse exception. + displayRun.error = await errorToMessage(err); } - return displayRun; - }) + } + return displayRun; + }) ); } @@ -339,7 +322,7 @@ class RunList extends React.Component { } return ( e.stopPropagation()} - to={RoutePage.PIPELINE_DETAILS.replace(':' + RouteParams.pipelineId, pipelineInfo.id)}> + to={RoutePage.PIPELINE_DETAILS.replace(':' + RouteParams.pipelineId, pipelineInfo.id)}> {pipelineInfo.displayName} ); @@ -355,20 +338,20 @@ class RunList extends React.Component { return await Promise.all( displayRuns.map(async (displayRun) => { const experimentId = RunUtils.getFirstExperimentReferenceId(displayRun.metadata); - if (experimentId) { - try { - // TODO: Experiment could be an optional field in state since whenever the RunList is - // created from the ExperimentDetails page, we already have the experiment (and will) - // be fetching the same one over and over here. - const experiment = await Apis.experimentServiceApi.getExperiment(experimentId); - displayRun.experiment = { displayName: experiment.name || '', id: experimentId }; - } catch (err) { - // This could be an API exception, or a JSON parse exception. - displayRun.error = await errorToMessage(err); - } + if (experimentId) { + try { + // TODO: Experiment could be an optional field in state since whenever the RunList is + // created from the ExperimentDetails page, we already have the experiment (and will) + // be fetching the same one over and over here. + const experiment = await Apis.experimentServiceApi.getExperiment(experimentId); + displayRun.experiment = { displayName: experiment.name || '', id: experimentId }; + } catch (err) { + // This could be an API exception, or a JSON parse exception. + displayRun.error = await errorToMessage(err); } - return displayRun; - }) + } + return displayRun; + }) ); } @@ -379,7 +362,7 @@ class RunList extends React.Component { } return ( e.stopPropagation()} - to={RoutePage.EXPERIMENT_DETAILS.replace(':' + RouteParams.experimentId, experimentInfo.id)}> + to={RoutePage.EXPERIMENT_DETAILS.replace(':' + RouteParams.experimentId, experimentInfo.id)}> {experimentInfo.displayName} ); diff --git a/frontend/src/pages/__snapshots__/RunList.test.tsx.snap b/frontend/src/pages/__snapshots__/RunList.test.tsx.snap index 365bc7190f2..e2ee6cb1fac 100644 --- a/frontend/src/pages/__snapshots__/RunList.test.tsx.snap +++ b/frontend/src/pages/__snapshots__/RunList.test.tsx.snap @@ -38,8 +38,7 @@ exports[`RunList displays error in run row if it failed to parse (run list mask) ] } emptyMessage="No runs found." - orderAscending={false} - pageSize={10} + initialSortColumn="created_at" reload={[Function]} rows={ Array [ @@ -69,7 +68,6 @@ exports[`RunList displays error in run row if it failed to parse (run list mask) }, ] } - sortBy="created_at" />
`; @@ -112,8 +110,6 @@ exports[`RunList displays error in run row if it failed to parse 1`] = ` ] } emptyMessage="No runs found." - orderAscending={false} - pageSize={10} reload={[Function]} rows={ Array [ @@ -131,7 +127,6 @@ exports[`RunList displays error in run row if it failed to parse 1`] = ` }, ] } - sortBy="created_at" />
`; @@ -174,8 +169,6 @@ exports[`RunList loads all runs 1`] = ` ] } emptyMessage="No runs found." - orderAscending={false} - pageSize={10} reload={[Function]} rows={ Array [ @@ -193,7 +186,6 @@ exports[`RunList loads all runs 1`] = ` }, ] } - sortBy="created_at" /> `; @@ -236,8 +228,6 @@ exports[`RunList loads runs for a given experiment id 1`] = ` ] } emptyMessage="No runs found for this experiment." - orderAscending={false} - pageSize={10} reload={[Function]} rows={ Array [ @@ -255,7 +245,6 @@ exports[`RunList loads runs for a given experiment id 1`] = ` }, ] } - sortBy="created_at" /> `; @@ -298,11 +287,9 @@ exports[`RunList renders the empty experience 1`] = ` ] } emptyMessage="No runs found." - orderAscending={false} - pageSize={10} + initialSortColumn="created_at" reload={[Function]} rows={Array []} - sortBy="created_at" /> `; @@ -345,8 +332,6 @@ exports[`RunList shows run time for each run 1`] = ` ] } emptyMessage="No runs found." - orderAscending={false} - pageSize={10} reload={[Function]} rows={ Array [ @@ -364,7 +349,6 @@ exports[`RunList shows run time for each run 1`] = ` }, ] } - sortBy="created_at" /> `;