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

format cron from quartz to unix in applications #15038

Merged
merged 11 commits into from
Feb 9, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* limitations under the License.
*/

import { Button, Col, Row, Space } from 'antd';
import { Button, Col, Form, Row, Space } from 'antd';
import { t } from 'i18next';
import React, { useEffect, useState } from 'react';
import CronEditor from '../../common/CronEditor/CronEditor';
Expand All @@ -38,12 +38,14 @@ const TestSuiteScheduler: React.FC<TestSuiteSchedulerProps> = ({
return (
<Row gutter={[16, 32]}>
<Col span={24}>
<CronEditor
includePeriodOptions={includePeriodOptions}
isQuartzCron={isQuartzCron}
value={repeatFrequency}
onChange={(value: string) => setRepeatFrequency(value)}
/>
<Form data-testid="schedule-container">
<CronEditor
includePeriodOptions={includePeriodOptions}
isQuartzCron={isQuartzCron}
Ashish8689 marked this conversation as resolved.
Show resolved Hide resolved
value={repeatFrequency}
onChange={(value: string) => setRepeatFrequency(value)}
/>
</Form>
</Col>
<Col span={24}>
<Space className="w-full justify-end" size={16}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ import {
AppScheduleClass,
AppType,
} from '../../../generated/entity/applications/app';
import { PipelineType } from '../../../generated/entity/services/ingestionPipelines/ingestionPipeline';
import { getIngestionPipelineByFqn } from '../../../rest/ingestionPipelineAPI';
import { getIngestionFrequency } from '../../../utils/CommonUtils';
import TestSuiteScheduler from '../../AddDataQualityTest/components/TestSuiteScheduler';
import Loader from '../../Loader/Loader';
import AppRunsHistory from '../AppRunsHistory/AppRunsHistory.component';
Expand Down Expand Up @@ -74,10 +72,6 @@ const AppSchedule = ({

return cronstrue.toString(cronExp, {
throwExceptionOnParseError: false,
// Quartz cron format accepts 1-7 or SUN-SAT so need to increment index by 1
// Ref: https://www.quartz-scheduler.org/api/2.1.7/org/quartz/CronExpression.html
dayOfWeekStartIndexZero: false,
monthStartIndexZero: false,
});
}

Expand Down Expand Up @@ -225,13 +219,14 @@ const AppSchedule = ({
open={showModal}
title={t('label.update-entity', { entity: t('label.schedule') })}>
<TestSuiteScheduler
isQuartzCron
buttonProps={{
cancelText: t('label.cancel'),
okText: t('label.save'),
}}
includePeriodOptions={initialOptions}
initialData={getIngestionFrequency(PipelineType.Application)}
initialData={
(appData.appSchedule as AppScheduleClass).cronExpression ?? ''
}
onCancel={onDialogCancel}
onSubmit={onDialogSave}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,11 @@ import {
ScheduleTimeline,
} from '../../generated/entity/applications/createAppRequest';
import { AppMarketPlaceDefinition } from '../../generated/entity/applications/marketplace/appMarketPlaceDefinition';
import { PipelineType } from '../../generated/entity/services/ingestionPipelines/ingestionPipeline';
import { useFqn } from '../../hooks/useFqn';
import { installApplication } from '../../rest/applicationAPI';
import { getMarketPlaceApplicationByFqn } from '../../rest/applicationMarketPlaceAPI';
import {
getEntityMissingError,
getIngestionFrequency,
} from '../../utils/CommonUtils';
import { getEntityMissingError } from '../../utils/CommonUtils';
import { getCronInitialValue } from '../../utils/CronUtils';
import { formatFormDataForSubmit } from '../../utils/JSONSchemaFormUtils';
import {
getMarketPlaceAppDetailsPath,
Expand All @@ -72,6 +69,24 @@ const AppInstall = () => {
[appData]
);

const { initialOptions, initialValue } = useMemo(() => {
let initialOptions;

if (appData?.name === 'DataInsightsReportApplication') {
initialOptions = ['Week'];
} else if (appData?.appType === AppType.External) {
initialOptions = ['Day'];
}

return {
initialOptions,
initialValue: getCronInitialValue(
appData?.appType ?? AppType.Internal,
appData?.name ?? ''
),
};
}, [appData?.name, appData?.appType]);

const fetchAppDetails = useCallback(async () => {
setIsLoading(true);
try {
Expand Down Expand Up @@ -126,16 +141,6 @@ const AppInstall = () => {
setActiveServiceStep(3);
};

const initialOptions = useMemo(() => {
if (appData?.name === 'DataInsightsReportApplication') {
return ['Week'];
} else if (appData?.appType === AppType.External) {
return ['Day'];
}

return undefined;
}, [appData?.name, appData?.appType]);

const RenderSelectedTab = useCallback(() => {
if (!appData || !jsonSchema) {
return <></>;
Expand Down Expand Up @@ -180,9 +185,8 @@ const AppInstall = () => {
<div className="w-500 p-md border rounded-4">
<Typography.Title level={5}>{t('label.schedule')}</Typography.Title>
<TestSuiteScheduler
isQuartzCron
includePeriodOptions={initialOptions}
initialData={getIngestionFrequency(PipelineType.Application)}
initialData={initialValue}
onCancel={() =>
setActiveServiceStep(appData.allowConfiguration ? 2 : 1)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
* limitations under the License.
*/

import { getQuartzCronExpression } from './CronUtils';
import { AppType } from '../generated/entity/applications/app';
import { getCronInitialValue, getQuartzCronExpression } from './CronUtils';

describe('getQuartzCronExpression function', () => {
it('should generate cron expression for every minute', () => {
Expand Down Expand Up @@ -94,3 +95,32 @@ describe('getQuartzCronExpression function', () => {
expect(result).toEqual('0 30 10 ? * 4');
});
});

describe('getCronInitialValue function', () => {
it('should generate hour cron expression if appType is internal and appName is not DataInsightsReportApplication', () => {
const result = getCronInitialValue(
AppType.Internal,
'SearchIndexingApplication'
);

expect(result).toEqual('0 * * * *');
});

it('should generate week cron expression if appName is DataInsightsReportApplication', () => {
const result = getCronInitialValue(
AppType.Internal,
'DataInsightsReportApplication'
);

expect(result).toEqual('0 0 * * 0');
});

it('should generate day cron expression if appType is external', () => {
const result = getCronInitialValue(
AppType.External,
'DataInsightsApplication'
);

expect(result).toEqual('0 0 * * *');
});
});
18 changes: 18 additions & 0 deletions openmetadata-ui/src/main/resources/ui/src/utils/CronUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
StateValue,
ToDisplay,
} from '../components/common/CronEditor/CronEditor.interface';
import { AppType } from '../generated/entity/applications/app';

export const getQuartzCronExpression = (state: StateValue) => {
const {
Expand Down Expand Up @@ -172,3 +173,20 @@ export const getStateValue = (valueStr: string) => {

return stateVal;
};

export const getCronInitialValue = (appType: AppType, appName: string) => {
const value = {
min: 0,
hour: 0,
};

let initialValue = getHourCron(value);

if (appName === 'DataInsightsReportApplication') {
initialValue = getWeekCron({ ...value, dow: 0 });
} else if (appType === AppType.External) {
initialValue = getDayCron(value);
}

return initialValue;
};
Loading