Skip to content

Commit

Permalink
make checkpoint mandatory
Browse files Browse the repository at this point in the history
Signed-off-by: Shenoy Pratik <[email protected]>
  • Loading branch information
ps48 committed Oct 23, 2023
1 parent 937ff30 commit 3702e2b
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -734,7 +734,7 @@ Array [
class="euiFormLabel euiFormRow__label"
for="some_html_id"
>
Checkpoint location - optional
Checkpoint location
</label>
</div>
<div
Expand Down Expand Up @@ -1986,7 +1986,7 @@ Array [
className="euiFormLabel euiFormRow__label"
htmlFor="some_html_id"
>
Checkpoint location - optional
Checkpoint location
</label>
</div>
<div
Expand Down
29 changes: 11 additions & 18 deletions public/components/acceleration/create/__tests__/utils.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -155,46 +155,39 @@ describe('validateIndexName', () => {
});

describe('validateCheckpointLocation', () => {
it('should return an array with an error message when creating a materialized view without a checkpoint location', () => {
const materializedError = validateCheckpointLocation('materialized', undefined);
expect(materializedError).toEqual([
'Checkpoint location is mandatory for materialized view creation',
]);
it('should return an array with an error message when using auto refresh without a checkpoint location', () => {
const materializedError = validateCheckpointLocation('auto', undefined);
expect(materializedError).toEqual(['Checkpoint location is mandatory for auto refresh']);
});

it('should return an array with an error message when creating a materialized view without a checkpoint location', () => {
const materializedError = validateCheckpointLocation('materialized', '');
expect(materializedError).toEqual([
'Checkpoint location is mandatory for materialized view creation',
]);
it('should return an array with an error message when using auto refresh without a checkpoint location', () => {
const materializedError = validateCheckpointLocation('auto', '');
expect(materializedError).toEqual(['Checkpoint location is mandatory for auto refresh']);
});

it('should return an array with an error message when the checkpoint location is not a valid S3 URL', () => {
const invalidCheckpoint = validateCheckpointLocation('skipping', 'not_a_valid_s3_url');
const invalidCheckpoint = validateCheckpointLocation('auto', 'not_a_valid_s3_url');
expect(invalidCheckpoint).toEqual(['Enter a valid checkpoint location']);
});

it('should return an empty array when the checkpoint location is a valid S3 URL', () => {
const validCheckpoint = validateCheckpointLocation(
'covering',
'interval',
's3://valid-s3-bucket/path/to/checkpoint'
);
expect(validCheckpoint).toEqual([]);
});

it('should return an empty array when the checkpoint location is a valid S3A URL', () => {
const validCheckpoint = validateCheckpointLocation(
'skipping',
'auto',
's3a://valid-s3-bucket/path/to/checkpoint'
);
expect(validCheckpoint).toEqual([]);
});

it('should return an empty array when creating a materialized view with a valid checkpoint location', () => {
const validMaterializedCheckpoint = validateCheckpointLocation(
'materialized',
's3://valid-s3-bucket/path/to/checkpoint'
);
it('should return an empty array when using manual refresh with no checkpoint location', () => {
const validMaterializedCheckpoint = validateCheckpointLocation('manual', '');
expect(validMaterializedCheckpoint).toEqual([]);
});

Expand Down
9 changes: 5 additions & 4 deletions public/components/acceleration/create/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
} from '../../../../common/constants';
import {
AccelerationIndexType,
AccelerationRefreshType,
CreateAccelerationForm,
FormErrorsType,
SkippingIndexRowType,
Expand Down Expand Up @@ -56,11 +57,11 @@ export const validateIndexName = (value: string) => {
};

export const validateCheckpointLocation = (
accelerationIndexType: AccelerationIndexType,
refreshType: AccelerationRefreshType,
checkpointLocation: string | undefined
) => {
if (accelerationIndexType === 'materialized' && !checkpointLocation) {
return ['Checkpoint location is mandatory for materialized view creation'];
if (refreshType !== 'manual' && !checkpointLocation) {
return ['Checkpoint location is mandatory for auto refresh'];
}

if (checkpointLocation && !ACCELERATION_S3_URL_REGEX.test(checkpointLocation))
Expand Down Expand Up @@ -135,7 +136,7 @@ export const formValidator = (accelerationformData: CreateAccelerationForm) => {
refreshType,
refreshIntervalOptions.refreshWindow
),
checkpointLocationError: validateCheckpointLocation(accelerationIndexType, checkpointLocation),
checkpointLocationError: validateCheckpointLocation(refreshType, checkpointLocation),
indexNameError: validateIndexName(accelerationIndexName),
skippingIndexError: validateSkippingIndexData(accelerationIndexType, skippingIndexQueryData),
coveringIndexError: validateCoveringIndexData(accelerationIndexType, coveringIndexQueryData),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ Array [
className="euiFormLabel euiFormRow__label"
htmlFor="some_html_id"
>
Checkpoint location - optional
Checkpoint location
</label>
</div>
<div
Expand Down Expand Up @@ -771,7 +771,7 @@ Array [
className="euiFormLabel euiFormRow__label"
htmlFor="some_html_id"
>
Checkpoint location - optional
Checkpoint location
</label>
</div>
<div
Expand Down Expand Up @@ -1179,7 +1179,7 @@ Array [
className="euiFormLabel euiFormRow__label"
htmlFor="some_html_id"
>
Checkpoint location - optional
Checkpoint location
</label>
</div>
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,11 +228,7 @@ export const IndexSettingOptions = ({
)}
{refreshTypeSelected !== manualRefreshId && (
<EuiFormRow
label={
accelerationFormData.accelerationIndexType === 'materialized'
? 'Checkpoint location'
: 'Checkpoint location - optional'
}
label="Checkpoint location"
helpText="The HDFS compatible file system location path for incremental refresh job checkpoint."
isInvalid={hasError(accelerationFormData.formErrors, 'checkpointLocationError')}
error={accelerationFormData.formErrors.checkpointLocationError}
Expand All @@ -247,7 +243,7 @@ export const IndexSettingOptions = ({
setAccelerationFormData(
producer((accData) => {
accData.formErrors.checkpointLocationError = validateCheckpointLocation(
accData.accelerationIndexType,
accData.refreshType,
e.target.value
);
})
Expand Down

0 comments on commit 3702e2b

Please sign in to comment.