Skip to content

Commit

Permalink
Add recurring run column to run lists (#1635)
Browse files Browse the repository at this point in the history 
* Adds recurring run column to run list with link to run config

* Adds tests

* Show recurring run ID

* Update snapshots
  • Loading branch information
@rileyjbauer @k8s-ci-robot
rileyjbauer authored and k8s-ci-robot committed Jul 22, 2019
1 parent 2778632 commit befb547
Show file tree
Hide file tree
Showing 4 changed files with 1,048 additions and 238 deletions.
14 changes: 14 additions & 0 deletions frontend/src/lib/RunUtils.ts
View file
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
View file
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
View file
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

0 comments on commit befb547

Please sign in to comment.