From 6c0dcc6c5987d3145001e3452ca1429a46ca646a Mon Sep 17 00:00:00 2001 From: "Yuan (Bob) Gong" Date: Fri, 21 Feb 2020 12:52:33 +0800 Subject: [PATCH] [UI] Scheduled workflow catchup=false option (#3131) * Update codegen instruction * Regenerate api * [UI] scheduled workflow catchup option * Show catchup in recurring run details page * Add help button to introduce swf catchup=false * Update snapshots and fix tests --- frontend/README.md | 5 +- frontend/src/apis/job/api.ts | 22 +- frontend/src/atoms/HelpButton.tsx | 40 ++++ frontend/src/components/Trigger.test.tsx | 199 ++++++++++-------- frontend/src/components/Trigger.tsx | 81 +++++-- .../__snapshots__/Trigger.test.tsx.snap | 145 +++++++++++++ frontend/src/lib/Apis.ts | 2 +- frontend/src/lib/RunUtils.ts | 2 +- frontend/src/pages/GettingStarted.test.tsx | 2 +- frontend/src/pages/NewRun.test.tsx | 1 + frontend/src/pages/NewRun.tsx | 50 ++--- .../src/pages/RecurringRunDetails.test.tsx | 27 ++- frontend/src/pages/RecurringRunDetails.tsx | 1 + .../RecurringRunDetails.test.tsx.snap | 8 + 14 files changed, 424 insertions(+), 161 deletions(-) create mode 100644 frontend/src/atoms/HelpButton.tsx diff --git a/frontend/README.md b/frontend/README.md index 8bdaeb6e8a4..41d080f23e3 100644 --- a/frontend/README.md +++ b/frontend/README.md @@ -2,12 +2,13 @@ ## Develop -You need `npm`, install dependencies using `npm install`. +You need `npm`, install dependencies using `npm ci` (`npm ci` makes sure your +installed dependencies have the exact same version as others). If you made any changes to protos (see backend/README), you'll need to regenerate the Typescript client library from swagger. We use swagger-codegen-cli@2.4.7, which you can get -[here](http://central.maven.org/maven2/io/swagger/swagger-codegen-cli/2.4.7/swagger-codegen-cli-2.4.7.jar). +[here](https://repo1.maven.org/maven2/io/swagger/swagger-codegen-cli/2.4.7/). Make sure the jar file is somewhere on your path with the name swagger-codegen-cli.jar, then run `npm run apis`. diff --git a/frontend/src/apis/job/api.ts b/frontend/src/apis/job/api.ts index 8f0ffb5cc8a..7036625cdd2 100644 --- a/frontend/src/apis/job/api.ts +++ b/frontend/src/apis/job/api.ts @@ -191,6 +191,12 @@ export interface ApiJob { * @memberof ApiJob */ enabled?: boolean; + /** + * Optional input field. Whether the job should catch up if behind schedule. If true, the job will only schedule the latest interval if behind schedule. If false, the job will catch up on each past interval. + * @type {boolean} + * @memberof ApiJob + */ + no_catchup?: boolean; } /** @@ -558,7 +564,7 @@ export const JobServiceApiFetchParamCreator = function(configuration?: Configura }, /** * - * @summary Disable a job. + * @summary Stops a job and all its associated runs. The job is not deleted. * @param {string} id The ID of the job to be disabled * @param {*} [options] Override http request option. * @throws {RequiredError} @@ -606,7 +612,7 @@ export const JobServiceApiFetchParamCreator = function(configuration?: Configura }, /** * - * @summary Enable a job. + * @summary Restarts a job that was previously stopped. All runs associated with the job will continue. * @param {string} id The ID of the job to be enabled * @param {*} [options] Override http request option. * @throws {RequiredError} @@ -839,7 +845,7 @@ export const JobServiceApiFp = function(configuration?: Configuration) { }, /** * - * @summary Disable a job. + * @summary Stops a job and all its associated runs. The job is not deleted. * @param {string} id The ID of the job to be disabled * @param {*} [options] Override http request option. * @throws {RequiredError} @@ -861,7 +867,7 @@ export const JobServiceApiFp = function(configuration?: Configuration) { }, /** * - * @summary Enable a job. + * @summary Restarts a job that was previously stopped. All runs associated with the job will continue. * @param {string} id The ID of the job to be enabled * @param {*} [options] Override http request option. * @throws {RequiredError} @@ -981,7 +987,7 @@ export const JobServiceApiFactory = function( }, /** * - * @summary Disable a job. + * @summary Stops a job and all its associated runs. The job is not deleted. * @param {string} id The ID of the job to be disabled * @param {*} [options] Override http request option. * @throws {RequiredError} @@ -991,7 +997,7 @@ export const JobServiceApiFactory = function( }, /** * - * @summary Enable a job. + * @summary Restarts a job that was previously stopped. All runs associated with the job will continue. * @param {string} id The ID of the job to be enabled * @param {*} [options] Override http request option. * @throws {RequiredError} @@ -1082,7 +1088,7 @@ export class JobServiceApi extends BaseAPI { /** * - * @summary Disable a job. + * @summary Stops a job and all its associated runs. The job is not deleted. * @param {string} id The ID of the job to be disabled * @param {*} [options] Override http request option. * @throws {RequiredError} @@ -1094,7 +1100,7 @@ export class JobServiceApi extends BaseAPI { /** * - * @summary Enable a job. + * @summary Restarts a job that was previously stopped. All runs associated with the job will continue. * @param {string} id The ID of the job to be enabled * @param {*} [options] Override http request option. * @throws {RequiredError} diff --git a/frontend/src/atoms/HelpButton.tsx b/frontend/src/atoms/HelpButton.tsx new file mode 100644 index 00000000000..e834a7c36d5 --- /dev/null +++ b/frontend/src/atoms/HelpButton.tsx @@ -0,0 +1,40 @@ +import Card from '@material-ui/core/Card'; +import CardContent from '@material-ui/core/CardContent'; +import IconButton from '@material-ui/core/IconButton'; +import { withStyles } from '@material-ui/core/styles'; +import Tooltip from '@material-ui/core/Tooltip'; +import HelpIcon from '@material-ui/icons/Help'; +import React, { ReactNode } from 'react'; +import { color, fontsize } from '../Css'; + +const NostyleTooltip = withStyles({ + tooltip: { + backgroundColor: 'transparent', + border: '0 none', + color: color.secondaryText, + fontSize: fontsize.base, + maxWidth: 220, + }, +})(Tooltip); + +interface HelpButtonProps { + helpText?: ReactNode; +} +export const HelpButton: React.FC = ({ helpText }) => { + return ( + + {helpText} + + } + interactive={true} + leaveDelay={400} + placement='top' + > + + + + + ); +}; diff --git a/frontend/src/components/Trigger.test.tsx b/frontend/src/components/Trigger.test.tsx index 3d10c5b3382..b72ab6e2a6b 100644 --- a/frontend/src/components/Trigger.test.tsx +++ b/frontend/src/components/Trigger.test.tsx @@ -19,6 +19,17 @@ import Trigger from './Trigger'; import { shallow } from 'enzyme'; import { TriggerType, PeriodicInterval } from '../lib/TriggerUtils'; +const PARAMS_DEFAULT = { + catchup: true, + maxConcurrentRuns: '10', +}; +const PERIODIC_DEFAULT = { + end_time: undefined, + interval_second: '60', + start_time: undefined, +}; +const CRON_DEFAULT = { cron: '0 * * * * ?', end_time: undefined, start_time: undefined }; + describe('Trigger', () => { // tslint:disable-next-line:variable-name const RealDate = Date; @@ -82,12 +93,12 @@ describe('Trigger', () => { (tree.instance() as Trigger).handleChange('type')({ target: { value: TriggerType.INTERVALED }, }); - expect(spy).toHaveBeenLastCalledWith( - { - periodic_schedule: { end_time: undefined, interval_second: '60', start_time: undefined }, + expect(spy).toHaveBeenLastCalledWith({ + ...PARAMS_DEFAULT, + trigger: { + periodic_schedule: PERIODIC_DEFAULT, }, - '10', - ); + }); }); it('builds trigger with a start time if the checkbox is checked', () => { @@ -99,10 +110,12 @@ describe('Trigger', () => { (tree.instance() as Trigger).handleChange('hasStartDate')({ target: { type: 'checkbox', checked: true }, }); - expect(spy).toHaveBeenLastCalledWith( - { periodic_schedule: { end_time: undefined, interval_second: '60', start_time: testDate } }, - '10', - ); + expect(spy).toHaveBeenLastCalledWith({ + ...PARAMS_DEFAULT, + trigger: { + periodic_schedule: { ...PERIODIC_DEFAULT, start_time: testDate }, + }, + }); }); it('builds trigger with the entered start date/time', () => { @@ -116,16 +129,15 @@ describe('Trigger', () => { }); (tree.instance() as Trigger).handleChange('startDate')({ target: { value: '2018-11-23' } }); (tree.instance() as Trigger).handleChange('endTime')({ target: { value: '08:35' } }); - expect(spy).toHaveBeenLastCalledWith( - { + expect(spy).toHaveBeenLastCalledWith({ + ...PARAMS_DEFAULT, + trigger: { periodic_schedule: { - end_time: undefined, - interval_second: '60', + ...PERIODIC_DEFAULT, start_time: new Date(2018, 10, 23, 8, 35), }, }, - '10', - ); + }); }); it('builds trigger without the entered start date if no time is entered', () => { @@ -139,16 +151,12 @@ describe('Trigger', () => { }); (tree.instance() as Trigger).handleChange('startDate')({ target: { value: '2018-11-23' } }); (tree.instance() as Trigger).handleChange('startTime')({ target: { value: '' } }); - expect(spy).toHaveBeenLastCalledWith( - { - periodic_schedule: { - end_time: undefined, - interval_second: '60', - start_time: undefined, - }, + expect(spy).toHaveBeenLastCalledWith({ + ...PARAMS_DEFAULT, + trigger: { + periodic_schedule: PERIODIC_DEFAULT, }, - '10', - ); + }); }); it('builds trigger without the entered start time if no date is entered', () => { @@ -162,16 +170,12 @@ describe('Trigger', () => { }); (tree.instance() as Trigger).handleChange('startDate')({ target: { value: '' } }); (tree.instance() as Trigger).handleChange('startTime')({ target: { value: '11:33' } }); - expect(spy).toHaveBeenLastCalledWith( - { - periodic_schedule: { - end_time: undefined, - interval_second: '60', - start_time: undefined, - }, + expect(spy).toHaveBeenLastCalledWith({ + ...PARAMS_DEFAULT, + trigger: { + periodic_schedule: PERIODIC_DEFAULT, }, - '10', - ); + }); }); it('builds trigger with a date if both start and end checkboxes are checked', () => { @@ -186,10 +190,12 @@ describe('Trigger', () => { (tree.instance() as Trigger).handleChange('hasEndDate')({ target: { type: 'checkbox', checked: true }, }); - expect(spy).toHaveBeenLastCalledWith( - { periodic_schedule: { end_time: testDate, interval_second: '60', start_time: testDate } }, - '10', - ); + expect(spy).toHaveBeenLastCalledWith({ + ...PARAMS_DEFAULT, + trigger: { + periodic_schedule: { ...PERIODIC_DEFAULT, end_time: testDate, start_time: testDate }, + }, + }); }); it('resets trigger to no start date if it is added then removed', () => { @@ -204,12 +210,12 @@ describe('Trigger', () => { (tree.instance() as Trigger).handleChange('hasStartDate')({ target: { type: 'checkbox', checked: false }, }); - expect(spy).toHaveBeenLastCalledWith( - { - periodic_schedule: { end_time: undefined, interval_second: '60', start_time: undefined }, + expect(spy).toHaveBeenLastCalledWith({ + ...PARAMS_DEFAULT, + trigger: { + periodic_schedule: PERIODIC_DEFAULT, }, - '10', - ); + }); }); it('builds trigger with a weekly interval', () => { @@ -221,16 +227,15 @@ describe('Trigger', () => { (tree.instance() as Trigger).handleChange('intervalCategory')({ target: { value: PeriodicInterval.WEEK }, }); - expect(spy).toHaveBeenLastCalledWith( - { + expect(spy).toHaveBeenLastCalledWith({ + ...PARAMS_DEFAULT, + trigger: { periodic_schedule: { - end_time: undefined, + ...PERIODIC_DEFAULT, interval_second: (7 * 24 * 60 * 60).toString(), - start_time: undefined, }, }, - '10', - ); + }); }); it('builds trigger with an every-three-months interval', () => { @@ -243,16 +248,15 @@ describe('Trigger', () => { target: { value: PeriodicInterval.MONTH }, }); (tree.instance() as Trigger).handleChange('intervalValue')({ target: { value: 3 } }); - expect(spy).toHaveBeenLastCalledWith( - { + expect(spy).toHaveBeenLastCalledWith({ + ...PARAMS_DEFAULT, + trigger: { periodic_schedule: { - end_time: undefined, + ...PERIODIC_DEFAULT, interval_second: (3 * 30 * 24 * 60 * 60).toString(), - start_time: undefined, }, }, - '10', - ); + }); }); it('builds trigger with the specified max concurrency setting', () => { @@ -262,16 +266,31 @@ describe('Trigger', () => { target: { value: TriggerType.INTERVALED }, }); (tree.instance() as Trigger).handleChange('maxConcurrentRuns')({ target: { value: '3' } }); - expect(spy).toHaveBeenLastCalledWith( - { - periodic_schedule: { - end_time: undefined, - interval_second: '60', - start_time: undefined, - }, + expect(spy).toHaveBeenLastCalledWith({ + ...PARAMS_DEFAULT, + maxConcurrentRuns: '3', + trigger: { + periodic_schedule: PERIODIC_DEFAULT, + }, + }); + }); + + it('builds trigger with the specified catchup setting', () => { + const spy = jest.fn(); + const tree = shallow(); + (tree.instance() as Trigger).handleChange('type')({ + target: { value: TriggerType.INTERVALED }, + }); + (tree.instance() as Trigger).handleChange('catchup')({ + target: { type: 'checkbox', checked: false }, + }); + expect(spy).toHaveBeenLastCalledWith({ + ...PARAMS_DEFAULT, + catchup: false, + trigger: { + periodic_schedule: PERIODIC_DEFAULT, }, - '3', - ); + }); }); }); @@ -280,10 +299,12 @@ describe('Trigger', () => { const spy = jest.fn(); const tree = shallow(); (tree.instance() as Trigger).handleChange('type')({ target: { value: TriggerType.CRON } }); - expect(spy).toHaveBeenLastCalledWith( - { cron_schedule: { cron: '0 * * * * ?', end_time: undefined, start_time: undefined } }, - '10', - ); + expect(spy).toHaveBeenLastCalledWith({ + ...PARAMS_DEFAULT, + trigger: { + cron_schedule: CRON_DEFAULT, + }, + }); }); it('builds a 1-minute cron trigger with specified start date', () => { @@ -294,10 +315,12 @@ describe('Trigger', () => { target: { type: 'checkbox', checked: true }, }); (tree.instance() as Trigger).handleChange('startDate')({ target: { value: '2018-03-23' } }); - expect(spy).toHaveBeenLastCalledWith( - { cron_schedule: { cron: '0 * * * * ?', end_time: undefined, start_time: testDate } }, - '10', - ); + expect(spy).toHaveBeenLastCalledWith({ + ...PARAMS_DEFAULT, + trigger: { + cron_schedule: { ...CRON_DEFAULT, start_time: testDate }, + }, + }); }); it('builds a daily cron trigger with specified end date/time', () => { @@ -310,10 +333,12 @@ describe('Trigger', () => { (tree.instance() as Trigger).handleChange('intervalCategory')({ target: { value: PeriodicInterval.DAY }, }); - expect(spy).toHaveBeenLastCalledWith( - { cron_schedule: { cron: '0 0 0 * * ?', end_time: testDate, start_time: undefined } }, - '10', - ); + expect(spy).toHaveBeenLastCalledWith({ + ...PARAMS_DEFAULT, + trigger: { + cron_schedule: { ...CRON_DEFAULT, end_time: testDate, cron: '0 0 0 * * ?' }, + }, + }); }); it('builds a weekly cron trigger that runs every Monday, Friday, and Saturday', () => { @@ -327,10 +352,12 @@ describe('Trigger', () => { (tree.instance() as any)._toggleDay(1); (tree.instance() as any)._toggleDay(5); (tree.instance() as any)._toggleDay(6); - expect(spy).toHaveBeenLastCalledWith( - { cron_schedule: { cron: '0 0 0 ? * 1,5,6', end_time: undefined, start_time: undefined } }, - '10', - ); + expect(spy).toHaveBeenLastCalledWith({ + ...PARAMS_DEFAULT, + trigger: { + cron_schedule: { ...CRON_DEFAULT, cron: '0 0 0 ? * 1,5,6' }, + }, + }); }); it('builds a cron with the manually specified cron string, even if days are toggled', () => { @@ -350,16 +377,12 @@ describe('Trigger', () => { (tree.instance() as Trigger).handleChange('cron')({ target: { value: 'oops this will break!' }, }); - expect(spy).toHaveBeenLastCalledWith( - { - cron_schedule: { - cron: 'oops this will break!', - end_time: undefined, - start_time: undefined, - }, + expect(spy).toHaveBeenLastCalledWith({ + ...PARAMS_DEFAULT, + trigger: { + cron_schedule: { ...CRON_DEFAULT, cron: 'oops this will break!' }, }, - '10', - ); + }); }); }); }); diff --git a/frontend/src/components/Trigger.tsx b/frontend/src/components/Trigger.tsx index 485e502a5d9..132a0f9aea8 100644 --- a/frontend/src/components/Trigger.tsx +++ b/frontend/src/components/Trigger.tsx @@ -14,28 +14,33 @@ * limitations under the License. */ -import * as React from 'react'; import Button from '@material-ui/core/Button'; import Checkbox from '@material-ui/core/Checkbox'; import FormControlLabel from '@material-ui/core/FormControlLabel'; -import Input from '../atoms/Input'; import MenuItem from '@material-ui/core/MenuItem'; +import * as React from 'react'; +import { stylesheet } from 'typestyle'; +import { ApiTrigger } from '../apis/job'; +import { HelpButton } from '../atoms/HelpButton'; +import Input from '../atoms/Input'; import Separator from '../atoms/Separator'; import { commonCss } from '../Css'; -import { dateToPickerFormat } from '../lib/TriggerUtils'; import { - PeriodicInterval, - TriggerType, - triggers, buildCron, - pickersToDate, buildTrigger, + dateToPickerFormat, + PeriodicInterval, + pickersToDate, + triggers, + TriggerType, } from '../lib/TriggerUtils'; -import { ApiTrigger } from '../apis/job'; -import { stylesheet } from 'typestyle'; interface TriggerProps { - onChange?: (trigger?: ApiTrigger, maxConcurrentRuns?: string) => void; + onChange?: (config: { + trigger?: ApiTrigger; + maxConcurrentRuns?: string; + catchup: boolean; + }) => void; } interface TriggerState { @@ -52,6 +57,7 @@ interface TriggerState { startDate: string; startTime: string; type: TriggerType; + catchup: boolean; } const css = stylesheet({ @@ -61,9 +67,7 @@ const css = stylesheet({ }); export default class Trigger extends React.Component { - constructor(props: any) { - super(props); - + public state = (() => { const now = new Date(); const inAWeek = new Date( now.getFullYear(), @@ -75,7 +79,8 @@ export default class Trigger extends React.Component const [startDate, startTime] = dateToPickerFormat(now); const [endDate, endTime] = dateToPickerFormat(inAWeek); - this.state = { + return { + catchup: true, cron: '', editCron: false, endDate, @@ -90,7 +95,7 @@ export default class Trigger extends React.Component startTime, type: TriggerType.INTERVALED, }; - } + })(); public componentDidMount(): void { // TODO: This is called here because NewRun only updates its Trigger in state when onChange is @@ -114,6 +119,7 @@ export default class Trigger extends React.Component startDate, startTime, type, + catchup, } = this.state; return ( @@ -209,6 +215,36 @@ export default class Trigger extends React.Component variant='outlined' /> + + + } + label='Catchup' + /> + +

+ Whether the recurring run should catch up if behind schedule. Defaults to true. +

+

+ For example, if the recurring run is paused for a while and re-enabled + afterwards. If catchup=true, the scheduler will catch up on (backfill) each + missed interval. Otherwise, it only schedules the latest interval. +

+

+ Usually, if your pipeline handles backfill internally, you should turn catchup + off to avoid duplicate work. +

+ + } + /> + Run every @@ -322,11 +358,11 @@ export default class Trigger extends React.Component { [name]: value, } as any, - this._updateTrigger.bind(this), + this._updateTrigger, ); }; - private _updateTrigger(): void { + private _updateTrigger = () => { const { hasStartDate, hasEndDate, @@ -340,6 +376,7 @@ export default class Trigger extends React.Component type, cron, selectedDays, + catchup, } = this.state; const startDateTime = pickersToDate(hasStartDate, startDate, startTime); @@ -363,11 +400,15 @@ export default class Trigger extends React.Component ); if (this.props.onChange) { - this.props.onChange(trigger, trigger ? this.state.maxConcurrentRuns : undefined); + this.props.onChange({ + catchup, + maxConcurrentRuns: trigger ? this.state.maxConcurrentRuns : undefined, + trigger, + }); } }, ); - } + }; private _isAllDaysChecked(): boolean { return this.state.selectedDays.every(d => !!d); @@ -397,7 +438,7 @@ export default class Trigger extends React.Component cron, selectedDays: newDays, }, - this._updateTrigger.bind(this), + this._updateTrigger, ); } } diff --git a/frontend/src/components/__snapshots__/Trigger.test.tsx.snap b/frontend/src/components/__snapshots__/Trigger.test.tsx.snap index 44768588860..d2927fa2b9f 100644 --- a/frontend/src/components/__snapshots__/Trigger.test.tsx.snap +++ b/frontend/src/components/__snapshots__/Trigger.test.tsx.snap @@ -145,6 +145,35 @@ exports[`Trigger enables a single day on click 1`] = ` width={120} /> + + + } + label="Catchup" + /> + +

+ Whether the recurring run should catch up if behind schedule. Defaults to true. +

+

+ For example, if the recurring run is paused for a while and re-enabled afterwards. If catchup=true, the scheduler will catch up on (backfill) each missed interval. Otherwise, it only schedules the latest interval. +

+

+ Usually, if your pipeline handles backfill internally, you should turn catchup off to avoid duplicate work. +

+ + } + /> + @@ -457,6 +486,35 @@ exports[`Trigger renders all week days enabled 1`] = ` width={120} /> + + + } + label="Catchup" + /> + +

+ Whether the recurring run should catch up if behind schedule. Defaults to true. +

+

+ For example, if the recurring run is paused for a while and re-enabled afterwards. If catchup=true, the scheduler will catch up on (backfill) each missed interval. Otherwise, it only schedules the latest interval. +

+

+ Usually, if your pipeline handles backfill internally, you should turn catchup off to avoid duplicate work. +

+ + } + /> + @@ -769,6 +827,35 @@ exports[`Trigger renders periodic schedule controls for initial render 1`] = ` width={120} /> + + + } + label="Catchup" + /> + +

+ Whether the recurring run should catch up if behind schedule. Defaults to true. +

+

+ For example, if the recurring run is paused for a while and re-enabled afterwards. If catchup=true, the scheduler will catch up on (backfill) each missed interval. Otherwise, it only schedules the latest interval. +

+

+ Usually, if your pipeline handles backfill internally, you should turn catchup off to avoid duplicate work. +

+ + } + /> + @@ -979,6 +1066,35 @@ exports[`Trigger renders periodic schedule controls if the trigger type is CRON width={120} /> + + + } + label="Catchup" + /> + +

+ Whether the recurring run should catch up if behind schedule. Defaults to true. +

+

+ For example, if the recurring run is paused for a while and re-enabled afterwards. If catchup=true, the scheduler will catch up on (backfill) each missed interval. Otherwise, it only schedules the latest interval. +

+

+ Usually, if your pipeline handles backfill internally, you should turn catchup off to avoid duplicate work. +

+ + } + /> + @@ -1212,6 +1328,35 @@ exports[`Trigger renders week days if the trigger type is CRON and interval is w width={120} /> + + + } + label="Catchup" + /> + +

+ Whether the recurring run should catch up if behind schedule. Defaults to true. +

+

+ For example, if the recurring run is paused for a while and re-enabled afterwards. If catchup=true, the scheduler will catch up on (backfill) each missed interval. Otherwise, it only schedules the latest interval. +

+

+ Usually, if your pipeline handles backfill internally, you should turn catchup off to avoid duplicate work. +

+ + } + /> + diff --git a/frontend/src/lib/Apis.ts b/frontend/src/lib/Apis.ts index c37713b9d63..cacde425632 100644 --- a/frontend/src/lib/Apis.ts +++ b/frontend/src/lib/Apis.ts @@ -13,7 +13,7 @@ // limitations under the License. import * as portableFetch from 'portable-fetch'; -import { HTMLViewerConfig } from 'src/components/viewers/HTMLViewer'; +import { HTMLViewerConfig } from '../components/viewers/HTMLViewer'; import { ExperimentServiceApi, FetchAPI } from '../apis/experiment'; import { JobServiceApi } from '../apis/job'; import { ApiPipeline, PipelineServiceApi, ApiPipelineVersion } from '../apis/pipeline'; diff --git a/frontend/src/lib/RunUtils.ts b/frontend/src/lib/RunUtils.ts index b4183a6228e..675d3cb45d0 100644 --- a/frontend/src/lib/RunUtils.ts +++ b/frontend/src/lib/RunUtils.ts @@ -15,7 +15,7 @@ */ import { orderBy } from 'lodash'; -import { ApiParameter, ApiPipelineVersion } from 'src/apis/pipeline'; +import { ApiParameter, ApiPipelineVersion } from '../apis/pipeline'; import { Workflow } from 'third_party/argo-ui/argo_template'; import { ApiJob } from '../apis/job'; import { diff --git a/frontend/src/pages/GettingStarted.test.tsx b/frontend/src/pages/GettingStarted.test.tsx index bfbf1084481..5d8c624519a 100644 --- a/frontend/src/pages/GettingStarted.test.tsx +++ b/frontend/src/pages/GettingStarted.test.tsx @@ -4,7 +4,7 @@ import TestUtils, { formatHTML } from '../TestUtils'; import { render } from '@testing-library/react'; import { PageProps } from './Page'; import { Apis } from '../lib/Apis'; -import { ApiListPipelinesResponse } from 'src/apis/pipeline/api'; +import { ApiListPipelinesResponse } from '../apis/pipeline/api'; import snapshotDiff from 'snapshot-diff'; const PATH_BACKEND_CONFIG = '../../../backend/src/apiserver/config/sample_config.json'; diff --git a/frontend/src/pages/NewRun.test.tsx b/frontend/src/pages/NewRun.test.tsx index 2239625d809..564af541493 100644 --- a/frontend/src/pages/NewRun.test.tsx +++ b/frontend/src/pages/NewRun.test.tsx @@ -1649,6 +1649,7 @@ describe('NewRun', () => { enabled: true, max_concurrency: '10', name: 'test run name', + no_catchup: false, pipeline_spec: { parameters: MOCK_PIPELINE.parameters, }, diff --git a/frontend/src/pages/NewRun.tsx b/frontend/src/pages/NewRun.tsx index 86482a20838..74388b83d3e 100644 --- a/frontend/src/pages/NewRun.tsx +++ b/frontend/src/pages/NewRun.tsx @@ -68,6 +68,7 @@ interface NewRunState { isFirstRunInExperiment: boolean; isRecurringRun: boolean; maxConcurrentRuns?: string; + catchup: boolean; parameters: ApiParameter[]; pipeline?: ApiPipeline; pipelineVersion?: ApiPipelineVersion; @@ -112,6 +113,27 @@ class NewRun extends Page<{}, NewRunState> { static contextType = NamespaceContext; context!: React.ContextType; + public state: NewRunState = { + catchup: true, + description: '', + errorMessage: '', + experimentName: '', + experimentSelectorOpen: false, + isBeingStarted: false, + isClone: false, + isFirstRunInExperiment: false, + isRecurringRun: false, + parameters: [], + pipelineName: '', + pipelineSelectorOpen: false, + pipelineVersionName: '', + pipelineVersionSelectorOpen: false, + runName: '', + uploadDialogOpen: false, + usePipelineFromRunLabel: 'Using pipeline from cloned run', + useWorkflowFromRun: false, + }; + private pipelineSelectorColumns = [ { customRenderer: NameWithTooltip, @@ -142,30 +164,6 @@ class NewRun extends Page<{}, NewRunState> { { label: 'Created at', flex: 1, sortKey: ExperimentSortKeys.CREATED_AT }, ]; - constructor(props: any) { - super(props); - - this.state = { - description: '', - errorMessage: '', - experimentName: '', - experimentSelectorOpen: false, - isBeingStarted: false, - isClone: false, - isFirstRunInExperiment: false, - isRecurringRun: false, - parameters: [], - pipelineName: '', - pipelineSelectorOpen: false, - pipelineVersionName: '', - pipelineVersionSelectorOpen: false, - runName: '', - uploadDialogOpen: false, - usePipelineFromRunLabel: 'Using pipeline from cloned run', - useWorkflowFromRun: false, - }; - } - public getInitialToolbarState(): ToolbarProps { return { actions: {}, @@ -507,9 +505,10 @@ class NewRun extends Page<{}, NewRunState> {
Choose a method by which new runs will be triggered
+ onChange={({ trigger, maxConcurrentRuns, catchup }) => this.setStateSafe( { + catchup, maxConcurrentRuns, trigger, }, @@ -1036,6 +1035,7 @@ class NewRun extends Page<{}, NewRunState> { newRun = Object.assign(newRun, { enabled: true, max_concurrency: this.state.maxConcurrentRuns || '1', + no_catchup: !this.state.catchup, trigger: this.state.trigger, }); } diff --git a/frontend/src/pages/RecurringRunDetails.test.tsx b/frontend/src/pages/RecurringRunDetails.test.tsx index 4c3761894a6..f0e834bab50 100644 --- a/frontend/src/pages/RecurringRunDetails.test.tsx +++ b/frontend/src/pages/RecurringRunDetails.test.tsx @@ -66,6 +66,7 @@ describe('RecurringRunDetails', () => { enabled: true, id: 'test-job-id', max_concurrency: '50', + no_catchup: true, name: 'test job', pipeline_spec: { parameters: [{ name: 'param1', value: 'value1' }], @@ -80,21 +81,12 @@ describe('RecurringRunDetails', () => { }, } as ApiJob; - historyPushSpy.mockClear(); - updateBannerSpy.mockClear(); - updateDialogSpy.mockClear(); - updateSnackbarSpy.mockClear(); - updateToolbarSpy.mockClear(); + jest.clearAllMocks(); getJobSpy.mockImplementation(() => fullTestJob); - getJobSpy.mockClear(); - deleteJobSpy.mockClear(); deleteJobSpy.mockImplementation(); - enableJobSpy.mockClear(); enableJobSpy.mockImplementation(); - disableJobSpy.mockClear(); disableJobSpy.mockImplementation(); getExperimentSpy.mockImplementation(); - getExperimentSpy.mockClear(); }); afterEach(() => tree.unmount()); @@ -106,13 +98,18 @@ describe('RecurringRunDetails', () => { }); it('renders a recurring run with cron schedule', async () => { - fullTestJob.trigger = { - cron_schedule: { - cron: '* * * 0 0 !', - end_time: new Date(2018, 10, 9, 8, 7, 6), - start_time: new Date(2018, 9, 8, 7, 6), + const cronTestJob = { + ...fullTestJob, + no_catchup: undefined, // in api requests, it's undefined when false + trigger: { + cron_schedule: { + cron: '* * * 0 0 !', + end_time: new Date(2018, 10, 9, 8, 7, 6), + start_time: new Date(2018, 9, 8, 7, 6), + }, }, }; + getJobSpy.mockImplementation(() => cronTestJob); tree = shallow(); await TestUtils.flushPromises(); expect(tree).toMatchSnapshot(); diff --git a/frontend/src/pages/RecurringRunDetails.tsx b/frontend/src/pages/RecurringRunDetails.tsx index cfe1e95b21a..d6e005f41f9 100644 --- a/frontend/src/pages/RecurringRunDetails.tsx +++ b/frontend/src/pages/RecurringRunDetails.tsx @@ -85,6 +85,7 @@ class RecurringRunDetails extends Page<{}, RecurringRunConfigState> { if (run.max_concurrency) { triggerDetails.push(['Max. concurrent runs', run.max_concurrency]); } + triggerDetails.push(['Catchup', `${!run.no_catchup}`]); if (run.trigger.cron_schedule && run.trigger.cron_schedule.start_time) { triggerDetails.push([ 'Start time', diff --git a/frontend/src/pages/__snapshots__/RecurringRunDetails.test.tsx.snap b/frontend/src/pages/__snapshots__/RecurringRunDetails.test.tsx.snap index 25043260e63..c8ad38cc0b9 100644 --- a/frontend/src/pages/__snapshots__/RecurringRunDetails.test.tsx.snap +++ b/frontend/src/pages/__snapshots__/RecurringRunDetails.test.tsx.snap @@ -37,6 +37,10 @@ exports[`RecurringRunDetails renders a recurring run with cron schedule 1`] = ` "Max. concurrent runs", "50", ], + Array [ + "Catchup", + "true", + ], Array [ "Start time", "10/8/2018, 7:06:00 AM", @@ -101,6 +105,10 @@ exports[`RecurringRunDetails renders a recurring run with periodic schedule 1`] "Max. concurrent runs", "50", ], + Array [ + "Catchup", + "false", + ], Array [ "Start time", "10/8/2018, 7:06:00 AM",