297443 delete table data for preparation datasets#229
297443 delete table data for preparation datasets#229ChristosKalaitzis merged 7 commits intoTestEnvfrom
Conversation
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
just deleting empty new lines as a clean up practice
| @Override | ||
| public boolean checkFolderExist(S3PathResolver s3PathResolver, String path) { | ||
| if (StringUtils.isNotBlank(s3PathResolver.getPreparationCode())) path = PreparationPathRegistry.resolve(path, s3PathResolver.getPreparationCode()); | ||
|
|
There was a problem hiding this comment.
This exact line will be repeated through out this development. Later, I explain PreparationPathRegistry. I am actually looking if there is a preparationCode (not null or empty) and if exists I proceed to change the path from a normal one to a preparation one. Example:
- From:
S3_IMPORT_TABLE_NAME_FOLDER_PATH = "%s/%s/%s/current/import/%s" - To:
S3_PREPARATION_IMPORT_TABLE_NAME_FOLDER_PATH = "%s/%s/%s/preparation/%s/import/%s
There was a problem hiding this comment.
This is a registry for storing the relation between normal paths and the relative preparation paths for S3.
Currently is a dynamically created map that end with the following being stored:
| normal S3 path | relative preparation path |
|---|---|
S3_TABLE_NAME_FOLDER_PATH |
S3_PREPARATION_TABLE_NAME_FOLDER_PATH |
S3_TABLE_AS_FOLDER_QUERY_PATH |
S3_PREPARATION_TABLE_AS_FOLDER_QUERY_PATH |
S3_VALIDATION_TABLE_PATH |
S3_PREPARATION_VALIDATION_TABLE_PATH |
S3_PROVIDER_IMPORT_PATH |
S3_PREPARATION_PROVIDER_IMPORT_PATH |
S3_IMPORT_FILE_PATH |
S3_PREPARATION_IMPORT_FILE_PATH |
S3_IMPORT_TABLE_NAME_FOLDER_PATH |
S3_PREPARATION_IMPORT_TABLE_NAME_FOLDER_PATH |
S3_ATTACHMENTS_TABLE_PATH |
S3_PREPARATION_ATTACHMENTS_TABLE_PATH |
S3_IMPORT_CSV_FILE_QUERY_PATH |
S3_PREPARATION_IMPORT_CSV_FILE_QUERY_PATH |
S3_PROVIDER_PATH |
S3_PREPARATION_PROVIDER_PATH |
If we want this registry to work for a new path we have to add after S3_ the word PREPARATION_ and will be automatically detected and added to the final map.
| * resolution of preparation-specific paths when a preparation code | ||
| * is provided. | ||
| * </p> | ||
| */ |
There was a problem hiding this comment.
just added a JavaDoc to be able to recognize the role of each of those classes better.
| public static final String S3_PREPARATION_TABLE_AS_FOLDER_QUERY_PATH = ".\"%s\".\"%s\".\"%s\".\"preparation\".\"%s\".\"%s\""; | ||
|
|
||
| /** The Constant S3_PPREPARATION_VALIDATION_TABLE_PATH: {@value}. */ | ||
| /** The Constant S3_PREPARATION_VALIDATION_TABLE_PATH: {@value}. */ |
There was a problem hiding this comment.
removed a P that was a syntax mistake
| false, dataflowId, providerId, Collections.singletonList(datasetId)); | ||
|
|
||
| jobId = jobControllerZuul.addDeleteDataJob(datasetId, null, dataflowId, providerId, | ||
| jobId = jobControllerZuul.addDeleteDataJob(datasetId, null, dataflowId, providerId, null, |
There was a problem hiding this comment.
This null will probably change when I do the development of delete dataset data, currently I have only developed everything about delete table data
| userNotificationContentVO.setDatasetId(datasetId); | ||
| userNotificationContentVO.setProviderId(providerId); | ||
| notificationControllerZuul.createUserNotificationPrivate("DELETE_TABLE_DATA_INIT", | ||
| userNotificationContentVO); |
There was a problem hiding this comment.
moved this part to a private method for clarity of controller actions
| throw new ResponseStatusException(HttpStatus.FORBIDDEN, | ||
| String.format(EEAErrorMessage.DATASET_NOT_BELONG_DATAFLOW, datasetId, dataflowId)); | ||
| } | ||
| validateDatasetBelongsToDataflow(datasetId, resolvedDataflowId, expectedDataflowId); |
There was a problem hiding this comment.
moved this part to a private method for clarity of controller actions
| if (JobStatusEnum.REFUSED.equals(jobStatus)) { | ||
| throw new ResponseStatusException(HttpStatus.LOCKED, EEAErrorMessage.DELETING_TABLE_DATA_REFUSED); | ||
| } | ||
| executeDeleteProcess(datasetId, tableSchemaId, preparationCode, resolvedDataflowId, providerId, jobId); |
There was a problem hiding this comment.
moved the part below to a private method for clarity of controller actions
| } | ||
| LOG.info("Successfully deleted table data for dataflowId {}, datasetId {} and tableSchemaId {}", dataflowId, datasetId, tableSchemaId); | ||
|
|
||
| Map<String, Object> result = new HashMap<>(); |
There was a problem hiding this comment.
moved the part below to a private method for clarity of controller actions
| //if table is big data remove first data from s3 | ||
| if (BooleanUtils.isTrue(isBigDataFlow)) { | ||
| bigDataDatasetService.deleteTableData(datasetId, dataSetMetabaseVO.getDataflowId(), dataSetMetabaseVO.getDataProviderId(), tableSchemaId, null, false); | ||
| bigDataDatasetService.deleteTableData(datasetId, dataSetMetabaseVO.getDataflowId(), dataSetMetabaseVO.getDataProviderId(), null, tableSchemaId, null, false); |
There was a problem hiding this comment.
This will remain null because it is only meant for DESIGN datasets where the preparation feature is not present
| public void deletePreparationDatasetById(@PathVariable("id") Long preparationId) { | ||
| public void deletePreparationDatasetById( | ||
| @PathVariable("id") Long preparationId, | ||
| @RequestParam("dataflowId") Long dataflowId |
There was a problem hiding this comment.
TODO: update front-end accordingly to include dataflowId parameter
| String preparationCode = notificationVO.getPreparationCode(); | ||
| String preparationDatasetMessagePart = (StringUtils.isNotBlank(preparationCode)) | ||
| ? " for preparation dataset " + preparationCode | ||
| : ""; |
There was a problem hiding this comment.
This is a work-around to have a conditional message at notification.
I have this variable to show a notification, and when there is no preparationCode I set this to empty string, so everytime it shows correctly.
| "deleteTabHeaderConfirm": "Delete table reconfirmation", | ||
| "deleteTable": "Delete table data", | ||
| "deleteTableCompletedEvent": "The data of table <strong title='{:tableSchemaName}'>{:shortTableSchemaName}</strong> in dataset <a href='{:navigateTo}' title='{:datasetName}'>{:shortDatasetName}</a> was deleted successfully.", | ||
| "deleteTableCompletedEvent": "The data of table <strong title='{:tableSchemaName}'>{:shortTableSchemaName}</strong> in dataset <a href='{:navigateTo}' title='{:datasetName}'>{:shortDatasetName}</a>{:preparationDatasetMessagePart} was deleted successfully.", |
There was a problem hiding this comment.
This is the notification message that I am using the empty string or for preparation dataset SECTION_A string
| } | ||
| } | ||
| } | ||
| if (isDatasetInActiveReleaseOrValidationJob(datasetIds)) return JobStatusEnum.REFUSED; |
There was a problem hiding this comment.
extracted to private method so it can be used in both cases and easier to read
ChristosKalaitzis
left a comment
ChristosKalaitzis
left a comment
There was a problem hiding this comment.
We have to update the front-end with a new variable of dataflowId in a previously designed controller.
There was a problem hiding this comment.
In deleteDatasetData
for (TableSchemaIdNameVO entry : tableSchemas) {
TableSchemaVO tableSchemaVO = datasetSchemaService.getTableSchemaVO(entry.getIdTableSchema(), datasetSchemaId);
if (tableSchemaVO != null && BooleanUtils.isTrue(tableSchemaVO.getDataAreManuallyEditable())
&& BooleanUtils.isTrue(datasetTableService.icebergTableIsCreated(datasetId, tableSchemaVO.getIdTableSchema()))) {
throw new Exception("Can not delete table data because iceberg table is created");
}
}
these checks need to be skipped if preparation code is not empty. This is because we don't have enabled editing yet with preparation datasets so we only check parent dataset. So if we are in a prep dataset, we can skip this check and add a TODO comment for when enable/disable editing feature is implemented for prep datasets.
Same for deleteTableData
skip following check if preparation code is not empty
if (tableSchemaVO != null && BooleanUtils.isTrue(tableSchemaVO.getDataAreManuallyEditable())
&& BooleanUtils.isTrue(datasetTableService.icebergTableIsCreated(datasetId, tableSchemaId))) {
throw new Exception("Can not delete table data because iceberg table is created");
}
There was a problem hiding this comment.
I've corrected this at my recent commit. Thank you for your findings.
fkouret
left a comment
fkouret
left a comment
There was a problem hiding this comment.
For testing in Test env for all
4 participants
No description provided.