Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add recurring run column to run lists #1635

Merged
merged 4 commits into from
Jul 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions frontend/src/lib/RunUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,26 @@ function extractMetricMetadata(runs: ApiRun[]): MetricMetadata[] {
return orderBy(metrics, ['count', 'name'], ['desc', 'asc']);
}

function getRecurringRunId(run?: ApiRun): string {
if (!run) {
return '';
}

for (const ref of run.resource_references || []) {
if (ref.key && ref.key.type === ApiResourceType.JOB) {
return ref.key.id || '';
}
}
return '';
}

export default {
extractMetricMetadata,
getAllExperimentReferences,
getFirstExperimentReference,
getFirstExperimentReferenceId,
getPipelineId,
getPipelineSpec,
getRecurringRunId,
runsToMetricMetadataMap,
};
19 changes: 19 additions & 0 deletions frontend/src/pages/RunList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,21 @@ describe('RunList', () => {
expect(tree).toMatchSnapshot();
});

it('shows link to recurring run config', async () => {
mockNRuns(1, {
run: {
resource_references: [{
key: { id: 'test-recurring-run-id', type: ApiResourceType.JOB }
}]
}
});
const props = generateProps();
tree = shallow(<RunList {...props} />);
await (tree.instance() as RunListTest)._loadRuns({});
expect(props.onError).not.toHaveBeenCalled();
expect(tree).toMatchSnapshot();
});

it('shows experiment name', async () => {
mockNRuns(1, {
run: {
Expand Down Expand Up @@ -372,6 +387,10 @@ describe('RunList', () => {
expect(getMountedInstance()._pipelineCustomRenderer({ value: { /* no displayName */ showLink: true }, id: 'run-id' })).toMatchSnapshot();
});

it('renders pipeline name as link to its details page', () => {
expect(getMountedInstance()._recurringRunCustomRenderer({ value: { id: 'recurring-run-id', }, id: 'run-id' })).toMatchSnapshot();
});

it('renders experiment name as link to its details page', () => {
expect(getMountedInstance()._experimentCustomRenderer({ value: { displayName: 'test experiment', id: 'experiment-id' }, id: 'run-id' })).toMatchSnapshot();
});
Expand Down
131 changes: 81 additions & 50 deletions frontend/src/pages/RunList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,14 @@ interface PipelineInfo {
showLink: boolean;
}

interface RecurringRunInfo {
displayName?: string;
id?: string;
}

interface DisplayRun {
experiment?: ExperimentInfo;
recurringRun?: RecurringRunInfo;
run: ApiRun;
pipeline?: PipelineInfo;
error?: string;
Expand Down Expand Up @@ -90,13 +96,14 @@ class RunList extends React.PureComponent<RunListProps, RunListState> {
const columns: Column[] = [
{
customRenderer: this._nameCustomRenderer,
flex: 2,
flex: 1.5,
label: 'Run name',
sortKey: RunSortKeys.NAME,
},
{ customRenderer: this._statusCustomRenderer, flex: 0.5, label: 'Status' },
{ label: 'Duration', flex: 0.5 },
{ customRenderer: this._pipelineCustomRenderer, label: 'Pipeline', flex: 1 },
{ customRenderer: this._recurringRunCustomRenderer, label: 'Recurring Run', flex: 0.5 },
{ label: 'Start time', flex: 1, sortKey: RunSortKeys.CREATED_AT },
];

Expand All @@ -116,7 +123,7 @@ class RunList extends React.PureComponent<RunListProps, RunListState> {
columns.push(...metricMetadata.map((metadata) => {
return {
customRenderer: this._metricCustomRenderer,
flex: 1,
flex: 0.5,
label: metadata.name!
};
}));
Expand All @@ -141,6 +148,7 @@ class RunList extends React.PureComponent<RunListProps, RunListState> {
r.run.status || '-',
getRunDuration(r.run),
r.pipeline,
r.recurringRun,
formatDateString(r.run.created_at),
] as any,
};
Expand Down Expand Up @@ -198,6 +206,20 @@ class RunList extends React.PureComponent<RunListProps, RunListState> {
);
}

public _recurringRunCustomRenderer: React.FC<CustomRendererProps<RecurringRunInfo>> = (props: CustomRendererProps<RecurringRunInfo>) => {
// If the getJob call failed or a run has no job, we display a placeholder.
if (!props.value || !props.value.id) {
return <div>-</div>;
}
const url = RoutePage.RECURRING_RUN.replace(':' + RouteParams.runId, props.value.id || '');
return (
<Link className={commonCss.link} onClick={(e) => e.stopPropagation()}
to={url}>
{props.value.displayName || '[View config]'}
</Link>
);
}

public _experimentCustomRenderer: React.FC<CustomRendererProps<ExperimentInfo>> = (props: CustomRendererProps<ExperimentInfo>) => {
// If the getExperiment call failed or a run has no experiment, we display a placeholder.
if (!props.value || !props.value.id) {
Expand Down Expand Up @@ -277,10 +299,7 @@ class RunList extends React.PureComponent<RunListProps, RunListState> {
}
}

await this._getAndSetPipelineNames(displayRuns);
if (!this.props.hideExperimentColumn) {
await this._getAndSetExperimentNames(displayRuns);
}
await this._setColumns(displayRuns);

this.setState({
metrics: RunUtils.extractMetricMetadata(displayRuns.map(r => r.run)),
Expand All @@ -289,6 +308,30 @@ class RunList extends React.PureComponent<RunListProps, RunListState> {
return nextPageToken;
}

private async _setColumns(displayRuns: DisplayRun[]): Promise<DisplayRun[]> {
return Promise.all(
displayRuns.map(async (displayRun) => {
this._setRecurringRun(displayRun);

await this._getAndSetPipelineNames(displayRun);

if (!this.props.hideExperimentColumn) {
await this._getAndSetExperimentNames(displayRun);
}
return displayRun;
})
);
}

private _setRecurringRun(displayRun: DisplayRun): void {
const recurringRunId = RunUtils.getRecurringRunId(displayRun.run);
if (recurringRunId) {
// TODO: It would be better to use name here, but that will require another n API calls at
// this time.
displayRun.recurringRun = { id: recurringRunId, displayName: recurringRunId };
}
}

/**
* For each run ID, fetch its corresponding run, and set it in DisplayRuns
*/
Expand All @@ -306,55 +349,43 @@ class RunList extends React.PureComponent<RunListProps, RunListState> {
}

/**
* For each DisplayRun, get its ApiRun and retrieve that ApiRun's Pipeline ID if it has one, then
* use that Pipeline ID to fetch its associated Pipeline and attach that Pipeline's name to the
* DisplayRun. If the ApiRun has no Pipeline ID, then the corresponding DisplayRun will show '-'.
* For the given DisplayRun, get its ApiRun and retrieve that ApiRun's Pipeline ID if it has one,
* then use that Pipeline ID to fetch its associated Pipeline and attach that Pipeline's name to
* the DisplayRun. If the ApiRun has no Pipeline ID, then the corresponding DisplayRun will show
* '-'.
*/
private _getAndSetPipelineNames(displayRuns: DisplayRun[]): Promise<DisplayRun[]> {
return Promise.all(
displayRuns.map(async (displayRun) => {
const pipelineId = RunUtils.getPipelineId(displayRun.run);
if (pipelineId) {
try {
const pipeline = await Apis.pipelineServiceApi.getPipeline(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.run)) {
displayRun.pipeline = { showLink: true };
}
return displayRun;
})
);
private async _getAndSetPipelineNames(displayRun: DisplayRun): Promise<void> {
const pipelineId = RunUtils.getPipelineId(displayRun.run);
if (pipelineId) {
try {
const pipeline = await Apis.pipelineServiceApi.getPipeline(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.run)) {
displayRun.pipeline = { showLink: true };
}
}

/**
* For each 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 '-'.
* 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 _getAndSetExperimentNames(displayRuns: DisplayRun[]): Promise<DisplayRun[]> {
return Promise.all(
displayRuns.map(async (displayRun) => {
const experimentId = RunUtils.getFirstExperimentReferenceId(displayRun.run);
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 = 'Failed to get associated experiment: ' + await errorToMessage(err);
}
}
return displayRun;
})
);
private async _getAndSetExperimentNames(displayRun: DisplayRun): Promise<void> {
const experimentId = RunUtils.getFirstExperimentReferenceId(displayRun.run);
if (experimentId) {
try {
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 = 'Failed to get associated experiment: ' + await errorToMessage(err);
}
}
}
}

Expand Down
Loading