Skip to content

Commit

Permalink
Fixes cloning of recurring runs (#1712)
Browse files Browse the repository at this point in the history
* Fixes cloning of recurring runs

Simplifies NewRun a little

Creates NewRunParameters component

* Fixes all NewRun tests

* All tests pass

* Adds tests for NewRunParameters

* Adds clone test to RecurringRunDetails

* Clean up
  • Loading branch information
rileyjbauer authored and k8s-ci-robot committed Aug 6, 2019
1 parent 351f456 commit 66883b0
Show file tree
Hide file tree
Showing 11 changed files with 614 additions and 1,379 deletions.
55 changes: 55 additions & 0 deletions frontend/src/components/NewRunParameters.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright 2019 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as React from 'react';
import { shallow } from 'enzyme';
import NewRunParameters, { NewRunParametersProps } from './NewRunParameters';

describe('NewRunParameters', () => {
it('shows parameters', () => {
const props = {
handleParamChange: jest.fn(),
initialParams: [{ name: 'testParam', value: 'testVal' }],
titleMessage: 'Specify parameters required by the pipeline',
} as NewRunParametersProps;
expect(shallow(<NewRunParameters {...props} />)).toMatchSnapshot();
});

it('does not display any text fields if there are parameters', () => {
const props = {
handleParamChange: jest.fn(),
initialParams: [],
titleMessage: 'This pipeline has no parameters',
} as NewRunParametersProps;
expect(shallow(<NewRunParameters {...props} />)).toMatchSnapshot();
});

it('fires handleParamChange callback on change', () => {
const handleParamChange = jest.fn();
const props = {
handleParamChange,
initialParams: [
{ name: 'testParam1', value: 'testVal1' },
{ name: 'testParam2', value: 'testVal2' }
],
titleMessage: 'Specify parameters required by the pipeline',
} as NewRunParametersProps;
const tree = shallow(<NewRunParameters {...props} />);
tree.find('#newRunPipelineParam1').simulate('change', { target: { value: 'test param value' } });
expect(handleParamChange).toHaveBeenCalledTimes(1);
expect(handleParamChange).toHaveBeenLastCalledWith(1, 'test param value');
});
});
54 changes: 54 additions & 0 deletions frontend/src/components/NewRunParameters.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Copyright 2018 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as React from 'react';
import { commonCss } from '../Css';
import TextField from '@material-ui/core/TextField';
import { ApiParameter } from '../apis/pipeline';

export interface NewRunParametersProps {
initialParams: ApiParameter[];
titleMessage: string;
handleParamChange: (index: number, value: string) => void;
}

class NewRunParameters extends React.Component<NewRunParametersProps> {
constructor(props: any) {
super(props);
}

public render(): JSX.Element | null {
const { handleParamChange, initialParams, titleMessage } = this.props;

return (
<div>
<div className={commonCss.header}>Run parameters</div>
<div>{titleMessage}</div>
{!!initialParams.length && (
<div>
{initialParams.map((param, i) =>
<TextField id={`newRunPipelineParam${i}`} key={i} variant='outlined'
label={param.name} value={param.value || ''}
onChange={(ev) => handleParamChange(i, ev.target.value || '')}
style={{ maxWidth: 600 }} className={commonCss.textField}/>)}
</div>
)}
</div>
);
}
}

export default NewRunParameters;
1 change: 1 addition & 0 deletions frontend/src/components/Router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const css = stylesheet({

export enum QUERY_PARAMS {
cloneFromRun = 'cloneFromRun',
cloneFromRecurringRun = 'cloneFromRecurringRun',
experimentId = 'experimentId',
isRecurring = 'recurring',
firstRunInExperiment = 'firstRunInExperiment',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`NewRunParameters does not display any text fields if there are parameters 1`] = `
<div>
<div
className="header"
>
Run parameters
</div>
<div>
This pipeline has no parameters
</div>
</div>
`;

exports[`NewRunParameters shows parameters 1`] = `
<div>
<div
className="header"
>
Run parameters
</div>
<div>
Specify parameters required by the pipeline
</div>
<div>
<TextField
className="textField"
id="newRunPipelineParam0"
key="0"
label="testParam"
onChange={[Function]}
required={false}
select={false}
style={
Object {
"maxWidth": 600,
}
}
value="testVal"
variant="outlined"
/>
</div>
</div>
`;
24 changes: 22 additions & 2 deletions frontend/src/lib/Buttons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,17 @@ export default class Buttons {
};
}

public cloneRecurringRun(getSelectedIds: () => string[], useCurrentResource: boolean): ToolbarActionConfig {
return {
action: () => this._cloneRun(getSelectedIds(), true),
disabled: !useCurrentResource,
disabledTitle: useCurrentResource ? undefined : 'Select a recurring run to clone',
id: 'cloneBtn',
title: 'Clone recurring run',
tooltip: 'Create a copy from this run\s initial state',
};
}

public collapseSections(action: () => void): ToolbarActionConfig {
return {
action,
Expand Down Expand Up @@ -221,10 +232,19 @@ export default class Buttons {
};
}

private _cloneRun(selectedIds: string[]): void {
private _cloneRun(selectedIds: string[], isRecurring?: boolean): void {
if (selectedIds.length === 1) {
const runId = selectedIds[0];
const searchString = this._urlParser.build({ [QUERY_PARAMS.cloneFromRun]: runId || '' });
let searchTerms;
if (isRecurring) {
searchTerms = {
[QUERY_PARAMS.cloneFromRecurringRun]: runId || '',
[QUERY_PARAMS.isRecurring]: '1'
};
} else {
searchTerms = { [QUERY_PARAMS.cloneFromRun]: runId || '' };
}
const searchString = this._urlParser.build(searchTerms);
this._props.history.push(RoutePage.NEW_RUN + searchString);
}
}
Expand Down
26 changes: 25 additions & 1 deletion frontend/src/lib/RunUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@
*/

import { ApiJob } from '../apis/job';
import { ApiRun, ApiResourceType, ApiResourceReference } from '../apis/run';
import { ApiRun, ApiResourceType, ApiResourceReference, ApiRunDetail, ApiPipelineRuntime } from '../apis/run';
import { orderBy } from 'lodash';
import { ApiParameter } from 'src/apis/pipeline';
import { Workflow } from 'third_party/argo-ui/argo_template';
import WorkflowParser from './WorkflowParser';
import { logger } from './Utils';

export interface MetricMetadata {
count: number;
Expand All @@ -25,6 +29,24 @@ export interface MetricMetadata {
name: string;
}

function getParametersFromRun(run: ApiRunDetail): ApiParameter[] {
return getParametersFromRuntime(run.pipeline_runtime);
}

function getParametersFromRuntime(runtime?: ApiPipelineRuntime): ApiParameter[] {
if (!runtime) {
return [];
}

try {
const workflow = JSON.parse(runtime.workflow_manifest!) as Workflow;
return WorkflowParser.getParameters(workflow);
} catch (err) {
logger.error('Failed to parse runtime workflow manifest', err);
return [];
}
}

function getPipelineId(run?: ApiRun | ApiJob): string | null {
return (run && run.pipeline_spec && run.pipeline_spec.pipeline_id) || null;
}
Expand Down Expand Up @@ -106,6 +128,8 @@ export default {
getAllExperimentReferences,
getFirstExperimentReference,
getFirstExperimentReferenceId,
getParametersFromRun,
getParametersFromRuntime,
getPipelineId,
getPipelineSpec,
getRecurringRunId,
Expand Down
Loading

0 comments on commit 66883b0

Please sign in to comment.