From e6fc2af93a523376538c9b3a51ea452c32a23c5d Mon Sep 17 00:00:00 2001 From: Dave Bauman Date: Thu, 11 Jan 2024 13:42:25 -0500 Subject: [PATCH 1/2] feat: Cron schedule support --- package.json | 1 + .../webapp/__tests__/lib/utils/cron.test.ts | 78 +++++++- .../DataDocSchedule/DataDocScheduleForm.tsx | 1 + .../components/DescribeCron/DescribeCron.tsx | 19 ++ .../webapp/components/Task/TaskEditor.tsx | 99 ++-------- querybook/webapp/lib/utils/cron.ts | 55 +++++- .../ui/ReccurenceEditor/RecurrenceEditor.tsx | 170 +++++++++++------- yarn.lock | 5 + 8 files changed, 276 insertions(+), 152 deletions(-) create mode 100644 querybook/webapp/components/DescribeCron/DescribeCron.tsx diff --git a/package.json b/package.json index c13a7ae23..63f8ebdcc 100644 --- a/package.json +++ b/package.json @@ -46,6 +46,7 @@ "clean-webpack-plugin": "3.0.0", "clsx": "^1.2.1", "codemirror": "^5.64.0", + "cronstrue": "2.47.0", "core-decorators": "0.20.0", "core-js": "^3.19.1", "cron-parser": "^4.7.0", diff --git a/querybook/webapp/__tests__/lib/utils/cron.test.ts b/querybook/webapp/__tests__/lib/utils/cron.test.ts index 665a43612..babca9959 100644 --- a/querybook/webapp/__tests__/lib/utils/cron.test.ts +++ b/querybook/webapp/__tests__/lib/utils/cron.test.ts @@ -1,4 +1,8 @@ -import { cronToRecurrence, recurrenceToCron } from 'lib/utils/cron'; +import { + cronToRecurrence, + recurrenceToCron, + validateCronForRecurrrence, +} from 'lib/utils/cron'; test('basic recurrence to cron', () => { expect( @@ -67,6 +71,7 @@ test('basic cron to recurrence', () => { recurrence: 'daily', on: {}, + cron: '1 1 * * *', }); }); @@ -77,6 +82,7 @@ test('yearly cron to recurrence', () => { recurrence: 'yearly', on: { dayMonth: [1, 2, 3], month: [3, 6, 9, 12] }, + cron: '1 1 1,2,3 3,6,9,12 *', }); }); @@ -87,6 +93,7 @@ test('monthly cron to recurrence', () => { recurrence: 'monthly', on: { dayMonth: [1, 2, 3] }, + cron: '1 1 1,2,3 * *', }); }); @@ -97,6 +104,7 @@ test('weekly cron to recurrence', () => { recurrence: 'weekly', on: { dayWeek: [1, 2, 3] }, + cron: '1 1 * * 1,2,3', }); }); @@ -107,5 +115,73 @@ test('hourly cron to recurrence', () => { recurrence: 'hourly', on: {}, + cron: '1 * * * *', }); }); + +test('every 5 minutes cron to recurrence', () => { + expect(cronToRecurrence('*/5 * * * *')).toStrictEqual({ + hour: 0, + minute: 0, + + recurrence: 'cron', + on: {}, + cron: '*/5 * * * *', + }); +}); + +test('at 22:00 on every day-of-week from Monday through Friday cron to recurrence', () => { + expect(cronToRecurrence('0 22 * * 1-5')).toStrictEqual({ + hour: 0, + minute: 0, + + recurrence: 'cron', + on: {}, + cron: '0 22 * * 1-5', + }); +}); + +test('at minute 0 past hour 0 and 12 on day-of-month 1 and 15 in January and July cron to recurrence', () => { + expect(cronToRecurrence('0 0,12 1,15 1,7 *')).toStrictEqual({ + hour: 0, + minute: 0, + + recurrence: 'cron', + on: {}, + cron: '0 0,12 1,15 1,7 *', + }); +}); + +test('validate hourly cron', () => { + expect(validateCronForRecurrrence('0 * * * *')).toBe(true); +}); +test('validate daily cron', () => { + expect(validateCronForRecurrrence('1 1 * * *')).toBe(true); +}); +test('validate weekly cron', () => { + expect(validateCronForRecurrrence('0 0 * * 1,3,5')).toBe(true); +}); +test('validate monthly cron', () => { + expect(validateCronForRecurrrence('0 0 1,15 1,7 *')).toBe(true); +}); +test('validate cron with step', () => { + expect(validateCronForRecurrrence('*/30 * * * *')).toBe(false); +}); +test('validate cron with range', () => { + expect(validateCronForRecurrrence('0 22 * * 1-5')).toBe(false); +}); +test('validate cron with minute and hour lists', () => { + expect(validateCronForRecurrrence('30 0,1,2 * * *')).toBe(false); +}); +test('validate cron with minute list', () => { + expect(validateCronForRecurrrence('0,15,30,45 * * * *')).toBe(false); +}); +test('validate cron with monthday and weekday', () => { + expect(validateCronForRecurrrence('0 0 1,15 * 1,7')).toBe(false); +}); +test('validate cron with missing minute', () => { + expect(validateCronForRecurrrence('* 0 * * *')).toBe(false); +}); +test('validate cron with missing minute and hour', () => { + expect(validateCronForRecurrrence('* * * * *')).toBe(false); +}); diff --git a/querybook/webapp/components/DataDocSchedule/DataDocScheduleForm.tsx b/querybook/webapp/components/DataDocSchedule/DataDocScheduleForm.tsx index fc8b65364..99cd3058f 100644 --- a/querybook/webapp/components/DataDocSchedule/DataDocScheduleForm.tsx +++ b/querybook/webapp/components/DataDocSchedule/DataDocScheduleForm.tsx @@ -276,6 +276,7 @@ export const DataDocScheduleForm: React.FunctionComponent< setFieldValue('recurrence', val) } diff --git a/querybook/webapp/components/DescribeCron/DescribeCron.tsx b/querybook/webapp/components/DescribeCron/DescribeCron.tsx new file mode 100644 index 000000000..6f294ba6d --- /dev/null +++ b/querybook/webapp/components/DescribeCron/DescribeCron.tsx @@ -0,0 +1,19 @@ +import cronstrue from 'cronstrue'; +import React, { useMemo } from 'react'; + +export const DescribeCron: React.FunctionComponent<{ cron?: string }> = ({ + cron, +}) => { + const description = useMemo(() => { + try { + if (cron) { + return cronstrue.toString(cron); + } + } catch (e) { + return null; + } + return null; + }, [cron]); + + return <>{description}; +}; diff --git a/querybook/webapp/components/Task/TaskEditor.tsx b/querybook/webapp/components/Task/TaskEditor.tsx index 406168922..6ce79529f 100644 --- a/querybook/webapp/components/Task/TaskEditor.tsx +++ b/querybook/webapp/components/Task/TaskEditor.tsx @@ -13,7 +13,6 @@ import { recurrenceOnYup, recurrenceToCron, recurrenceTypes, - validateCronForRecurrrence, } from 'lib/utils/cron'; import { TaskScheduleResource } from 'resource/taskSchedule'; import { AsyncButton } from 'ui/AsyncButton/AsyncButton'; @@ -26,7 +25,6 @@ import { RecurrenceEditor } from 'ui/ReccurenceEditor/RecurrenceEditor'; import { Tabs } from 'ui/Tabs/Tabs'; import { TimeFromNow } from 'ui/Timer/TimeFromNow'; import { Title } from 'ui/Title/Title'; -import { ToggleButton } from 'ui/ToggleButton/ToggleButton'; import './TaskEditor.scss'; @@ -42,20 +40,14 @@ interface IProps { const taskFormSchema = Yup.object().shape({ name: Yup.string().required(), task: Yup.string().required(), - isCron: Yup.boolean(), - recurrence: Yup.object().when('isCron', (isCron, schema) => - !isCron - ? schema.shape({ - hour: Yup.number().min(0).max(23), - minute: Yup.number().min(0).max(59), - recurrence: Yup.string().oneOf(recurrenceTypes), - on: recurrenceOnYup, - }) - : schema - ), - cron: Yup.string().when('isCron', (isCron, schema) => - isCron ? schema.required() : schema - ), + + recurrence: Yup.object().shape({ + hour: Yup.number().min(0).max(23), + minute: Yup.number().min(0).max(59), + recurrence: Yup.string().oneOf(recurrenceTypes), + on: recurrenceOnYup, + cron: Yup.string().optional(), + }), enabled: Yup.boolean().required(), arg: Yup.array().of(Yup.mixed()), kwargs: Yup.array().of(Yup.mixed()), @@ -100,9 +92,7 @@ export const TaskEditor: React.FunctionComponent = ({ const handleTaskEditSubmit = React.useCallback( (editedValues) => { - const editedCron = editedValues.isCron - ? editedValues.cron - : recurrenceToCron(editedValues.recurrence); + const editedCron = recurrenceToCron(editedValues.recurrence); const editedArgs = editedValues.args .filter((arg) => !(arg === '')) .map(stringToTypedVal); @@ -173,9 +163,7 @@ export const TaskEditor: React.FunctionComponent = ({ return { name: task.name || '', task: task.task || '', - isCron: !validateCronForRecurrrence(cron), recurrence, - cron, enabled: task.enabled ?? true, args: task.args || [], kwargs: Object.entries(task.kwargs || {}), @@ -299,8 +287,6 @@ export const TaskEditor: React.FunctionComponent = ({ ); - const canUseRecurrence = validateCronForRecurrrence(values.cron); - return (
@@ -349,63 +335,16 @@ export const TaskEditor: React.FunctionComponent = ({ />
{values.enabled ? ( -
- {values.isCron || !canUseRecurrence ? ( - - ) : ( - - - setFieldValue( - 'recurrence', - val - ) - } - /> - - )} - {canUseRecurrence ? ( -
- { - setFieldValue( - 'isCron', - val - ); - if (val) { - setFieldValue( - 'cron', - recurrenceToCron( - values.recurrence - ) - ); - } else { - setFieldValue( - 'recurrence', - cronToRecurrence( - values.cron - ) - ); - } - }} - title={ - values.isCron - ? 'Use Recurrence Editor' - : 'Use Cron' - } - /> -
- ) : null} -
+ + + setFieldValue('recurrence', val) + } + allowCron={true} + /> + ) : null}
diff --git a/querybook/webapp/lib/utils/cron.ts b/querybook/webapp/lib/utils/cron.ts index c47fca757..4a2c6c2af 100644 --- a/querybook/webapp/lib/utils/cron.ts +++ b/querybook/webapp/lib/utils/cron.ts @@ -7,6 +7,8 @@ export interface IRecurrence { recurrence: RecurrenceType; on: IRecurrenceOn; + + cron?: string; } export interface IRecurrenceOn { @@ -21,6 +23,7 @@ export const recurrenceTypes = [ 'weekly', 'monthly', 'yearly', + 'cron', ]; export type RecurrenceType = typeof recurrenceTypes[number]; @@ -30,7 +33,11 @@ export function cronToRecurrence(cron: string): IRecurrence { let recurrencePolicy: RecurrenceType = 'daily'; let on = {}; - if (dayMonth !== '*' && month !== '*') { + + // If cron is not supported via the Recurrence editor, then we default to cron type + if (!validateCronForRecurrrence(cron)) { + recurrencePolicy = 'cron'; + } else if (dayMonth !== '*' && month !== '*') { recurrencePolicy = 'yearly'; on = { month: month.split(',').map((d) => Number(d)), @@ -46,18 +53,34 @@ export function cronToRecurrence(cron: string): IRecurrence { recurrencePolicy = 'hourly'; } + if (recurrencePolicy === 'cron') { + return { + recurrence: 'cron', + cron, + hour: 0, + minute: 0, + on, + }; + } + const recurrence: IRecurrence = { hour: recurrencePolicy === 'hourly' ? 0 : Number(hour), minute: Number(minute), recurrence: recurrencePolicy, on, + + cron, }; return recurrence; } export function recurrenceToCron(recurrence: IRecurrence): string { + if (recurrence.recurrence === 'cron') { + return recurrence.cron; + } + const { minute } = recurrence; let hour = recurrence.hour.toString(); @@ -136,24 +159,46 @@ export function getRecurrenceLocalTimeString( .format(format); } +/** + * Determines whether a cron string is supported by the recurrence editor. + * + * @param cron Cron string to validate + * @returns true if cron string is supported by recurrence editor, false otherwise + */ export function validateCronForRecurrrence(cron: string) { if (cron.includes('/')) { return false; } + // Recurrence does not support ranges + if (cron.includes('-')) { + return false; + } + const cronValArr = cron.split(' '); if (cronValArr.length < 5) { return false; } - const [minute, hour, month, monthDay, weekDay] = cronValArr.map( + const [minuteList, hourList] = cronValArr.map((s) => s.includes(',')); + + // Recurrence does not support lists for minute and hour + if (minuteList || hourList) { + return false; + } + + const [minute, hour, monthDay, month, weekDay] = cronValArr.map( (s) => s !== '*' ); - // Minute and hour must be provided - if (!(minute && hour)) { + + const isHourly = minute && !hour && !monthDay && !month && !weekDay; + + // Minute and hour must be provided unless hourly + if (!(minute && hour) && !isHourly) { return false; } - // Recurrence don't current support having both monthday and weekday + + // Recurrence doesn't current support having both monthday and weekday if ((monthDay || month) && weekDay) { return false; } diff --git a/querybook/webapp/ui/ReccurenceEditor/RecurrenceEditor.tsx b/querybook/webapp/ui/ReccurenceEditor/RecurrenceEditor.tsx index fc038f583..a66611b82 100644 --- a/querybook/webapp/ui/ReccurenceEditor/RecurrenceEditor.tsx +++ b/querybook/webapp/ui/ReccurenceEditor/RecurrenceEditor.tsx @@ -3,6 +3,7 @@ import moment from 'moment'; import * as React from 'react'; import Select from 'react-select'; +import { DescribeCron } from 'components/DescribeCron/DescribeCron'; import { getMonthdayOptions, getRecurrenceLocalTimeString, @@ -10,6 +11,7 @@ import { getYearlyMonthOptions, IRecurrence, IRecurrenceOn, + recurrenceToCron, RecurrenceType, recurrenceTypes, } from 'lib/utils/cron'; @@ -26,12 +28,14 @@ const recurrenceReactSelectStyle = makeReactSelectStyle(true); interface IProps { recurrence: IRecurrence; recurrenceError?: FormikErrors; + allowCron?: boolean; setRecurrence: (val: IRecurrence) => void; } export const RecurrenceEditor: React.FunctionComponent = ({ recurrence, recurrenceError, + allowCron = false, setRecurrence, }) => { const localTime = React.useMemo( @@ -39,39 +43,10 @@ export const RecurrenceEditor: React.FunctionComponent = ({ [recurrence] ); - const isHourly = recurrence.recurrence === 'hourly'; - - const hourSecondField = ( - -
- { - const newRecurrence = { - ...recurrence, - hour: value.hour(), - minute: value.minute(), - }; - setRecurrence(newRecurrence); - }} - /> -
- {recurrence.recurrence === 'hourly' - ? `Every hour at minute ${recurrence.minute} ` - : `Local Time: ${localTime}`} -
-
- + // Filter out cron if not allowed + const filteredRecurrenceTypes = React.useMemo( + () => recurrenceTypes.filter((type) => allowCron || type !== 'cron'), + [allowCron] ); const recurrenceTypeField = ( @@ -79,13 +54,17 @@ export const RecurrenceEditor: React.FunctionComponent = ({ {({ field }) => ( { const newRecurrence = { ...recurrence, recurrence: key, }; + if (key === 'cron') { + newRecurrence.cron = + recurrenceToCron(recurrence); + } if (field.value !== key) { newRecurrence.on = {}; } @@ -98,50 +77,109 @@ export const RecurrenceEditor: React.FunctionComponent = ({ ); + let hourSecondField: React.ReactNode; let datePickerField: React.ReactNode; - if (recurrence.recurrence === 'yearly') { + + if (recurrence.recurrence === 'cron') { datePickerField = ( - <> + + ( + <> + +
+ +
+ + )} + /> + + ); + } else { + const isHourly = recurrence.recurrence === 'hourly'; + + hourSecondField = ( + +
+ { + const newRecurrence = { + ...recurrence, + hour: value.hour(), + minute: value.minute(), + }; + setRecurrence(newRecurrence); + }} + /> +
+ {recurrence.recurrence === 'hourly' + ? `Every hour at minute ${recurrence.minute} ` + : `Local Time: ${localTime}`} +
+
+ + ); + + if (recurrence.recurrence === 'yearly') { + datePickerField = ( + <> + + + + ); + } else if (recurrence.recurrence === 'monthly') { + datePickerField = ( + ); + } else if (recurrence.recurrence === 'weekly') { + datePickerField = ( - - ); - } else if (recurrence.recurrence === 'monthly') { - datePickerField = ( - - ); - } else if (recurrence.recurrence === 'weekly') { - datePickerField = ( - - ); + ); + } } return ( diff --git a/yarn.lock b/yarn.lock index de1d62c58..ffb726722 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8611,6 +8611,11 @@ cron-parser@^4.7.0: dependencies: luxon "^3.1.0" +cronstrue@2.47.0: + version "2.47.0" + resolved "https://registry.yarnpkg.com/cronstrue/-/cronstrue-2.47.0.tgz#3cf447c2a3a070d5bad2cedd6383e7cc16f5a308" + integrity sha512-fnFwJy7zslTEz6h7O7BkwgHNBvuuPmkhAYKqPDxK5mBQLz2nG08T9afbnjm+yrvcc/wxrd+otaVSnoXT9ulUOw== + cross-fetch@^3.0.4: version "3.1.4" resolved "https://registry.yarnpkg.com/cross-fetch/-/cross-fetch-3.1.4.tgz#9723f3a3a247bf8b89039f3a380a9244e8fa2f39" From 66f888a275651751ffee53fc07728cd03428fb32 Mon Sep 17 00:00:00 2001 From: Dave Bauman Date: Mon, 4 Mar 2024 09:08:33 -0500 Subject: [PATCH 2/2] fix: PR comments --- .../components/DescribeCron/DescribeCron.tsx | 19 --------- querybook/webapp/lib/utils/cron.ts | 40 ++++++++++--------- .../ui/ReccurenceEditor/RecurrenceEditor.tsx | 23 ++--------- .../RecurrenceEditorCronPicker.tsx | 37 +++++++++++++++++ 4 files changed, 62 insertions(+), 57 deletions(-) delete mode 100644 querybook/webapp/components/DescribeCron/DescribeCron.tsx create mode 100644 querybook/webapp/ui/ReccurenceEditor/RecurrenceEditorCronPicker.tsx diff --git a/querybook/webapp/components/DescribeCron/DescribeCron.tsx b/querybook/webapp/components/DescribeCron/DescribeCron.tsx deleted file mode 100644 index 6f294ba6d..000000000 --- a/querybook/webapp/components/DescribeCron/DescribeCron.tsx +++ /dev/null @@ -1,19 +0,0 @@ -import cronstrue from 'cronstrue'; -import React, { useMemo } from 'react'; - -export const DescribeCron: React.FunctionComponent<{ cron?: string }> = ({ - cron, -}) => { - const description = useMemo(() => { - try { - if (cron) { - return cronstrue.toString(cron); - } - } catch (e) { - return null; - } - return null; - }, [cron]); - - return <>{description}; -}; diff --git a/querybook/webapp/lib/utils/cron.ts b/querybook/webapp/lib/utils/cron.ts index 4a2c6c2af..f773e862f 100644 --- a/querybook/webapp/lib/utils/cron.ts +++ b/querybook/webapp/lib/utils/cron.ts @@ -81,25 +81,29 @@ export function recurrenceToCron(recurrence: IRecurrence): string { return recurrence.cron; } - const { minute } = recurrence; - - let hour = recurrence.hour.toString(); - let dayMonth = '*'; - let month = '*'; - let dayWeek = '*'; - - if (recurrence.recurrence === 'yearly') { - month = recurrence.on.month.join(','); - dayMonth = recurrence.on.dayMonth.join(','); - } else if (recurrence.recurrence === 'monthly') { - dayMonth = recurrence.on.dayMonth.join(','); - } else if (recurrence.recurrence === 'weekly') { - dayWeek = recurrence.on.dayWeek.join(','); - } else if (recurrence.recurrence === 'hourly') { - hour = '*'; - } + try { + const { minute } = recurrence; + + let hour = recurrence.hour.toString(); + let dayMonth = '*'; + let month = '*'; + let dayWeek = '*'; + + if (recurrence.recurrence === 'yearly') { + month = recurrence.on.month.join(','); + dayMonth = recurrence.on.dayMonth.join(','); + } else if (recurrence.recurrence === 'monthly') { + dayMonth = recurrence.on.dayMonth.join(','); + } else if (recurrence.recurrence === 'weekly') { + dayWeek = recurrence.on.dayWeek.join(','); + } else if (recurrence.recurrence === 'hourly') { + hour = '*'; + } - return `${minute} ${hour} ${dayMonth} ${month} ${dayWeek}`; + return `${minute} ${hour} ${dayMonth} ${month} ${dayWeek}`; + } catch (error) { + return ''; + } } export const WEEKDAYS = [ diff --git a/querybook/webapp/ui/ReccurenceEditor/RecurrenceEditor.tsx b/querybook/webapp/ui/ReccurenceEditor/RecurrenceEditor.tsx index a66611b82..bb0280924 100644 --- a/querybook/webapp/ui/ReccurenceEditor/RecurrenceEditor.tsx +++ b/querybook/webapp/ui/ReccurenceEditor/RecurrenceEditor.tsx @@ -3,7 +3,6 @@ import moment from 'moment'; import * as React from 'react'; import Select from 'react-select'; -import { DescribeCron } from 'components/DescribeCron/DescribeCron'; import { getMonthdayOptions, getRecurrenceLocalTimeString, @@ -21,6 +20,8 @@ import { overlayRoot } from 'ui/Overlay/Overlay'; import { Tabs } from 'ui/Tabs/Tabs'; import { TimePicker } from 'ui/TimePicker/TimePicker'; +import { RecurrenceEditorCronPicker } from './RecurrenceEditorCronPicker'; + import './RecurrenceEditor.scss'; const recurrenceReactSelectStyle = makeReactSelectStyle(true); @@ -81,25 +82,7 @@ export const RecurrenceEditor: React.FunctionComponent = ({ let datePickerField: React.ReactNode; if (recurrence.recurrence === 'cron') { - datePickerField = ( - - ( - <> - -
- -
- - )} - /> - - ); + datePickerField = ; } else { const isHourly = recurrence.recurrence === 'hourly'; diff --git a/querybook/webapp/ui/ReccurenceEditor/RecurrenceEditorCronPicker.tsx b/querybook/webapp/ui/ReccurenceEditor/RecurrenceEditorCronPicker.tsx new file mode 100644 index 000000000..4b81bbbc5 --- /dev/null +++ b/querybook/webapp/ui/ReccurenceEditor/RecurrenceEditorCronPicker.tsx @@ -0,0 +1,37 @@ +import cronstrue from 'cronstrue'; +import { Field } from 'formik'; +import React from 'react'; + +import { FormField } from 'ui/Form/FormField'; + +export const RecurrenceEditorCronPicker: React.FunctionComponent<{ + cron?: string; +}> = ({ cron }) => { + const cronDescription = React.useMemo(() => { + try { + return cronstrue.toString(cron); + } catch (e) { + return null; + } + }, [cron]); + + return ( + + ( + <> + +
+ {cronDescription} +
+ + )} + /> + + ); +};