Fix: Safer default Mark Deleted Tables (#9065)

* Fix: Safer default Mark Deleted Tables

* Fix: Change Based On Comments

* Fix: Change Based On Comments

* Add: Migration Files

Co-authored-by: Sachin Chaurasiya <sachinchaurasiyachotey87@gmail.com>
This commit is contained in:
Milan Bariya 2022-12-01 16:26:43 +05:30 committed by GitHub
parent 11edcef279
commit 2f9f169de9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 37 additions and 29 deletions

View File

@ -0,0 +1,3 @@
-- Remove markDeletedTablesFromFilterOnly
UPDATE ingestion_pipeline_entity
SET json = JSON_REMOVE(json ,'$.sourceConfig.config.markDeletedTablesFromFilterOnly');

View File

@ -0,0 +1,3 @@
-- Remove markDeletedTablesFromFilterOnly
UPDATE ingestion_pipeline_entity
SET json = json::jsonb #- '{sourceConfig,config,markDeletedTablesFromFilterOnly}';

View File

@ -533,7 +533,12 @@ class DatabaseServiceSource(
logger.info( logger.info(
f"Mark Deleted Tables set to True. Processing database [{self.context.database.name.__root__}]" f"Mark Deleted Tables set to True. Processing database [{self.context.database.name.__root__}]"
) )
if self.source_config.markDeletedTablesFromFilterOnly: # If markAllDeletedTables is True, all tables Which are not in FilterPattern will be deleted
if self.source_config.markAllDeletedTables:
yield from self.fetch_all_schema_and_delete_tables()
# If markAllDeletedTables is False (Default), Only delete tables which are deleted from the datasource
else:
schema_names_list = self.get_database_schema_names() schema_names_list = self.get_database_schema_names()
for schema_name in schema_names_list: for schema_name in schema_names_list:
schema_fqn = fqn.build( schema_fqn = fqn.build(
@ -545,5 +550,3 @@ class DatabaseServiceSource(
) )
yield from self.delete_schema_tables(schema_fqn) yield from self.delete_schema_tables(schema_fqn)
else:
yield from self.fetch_all_schema_and_delete_tables()

View File

@ -23,8 +23,8 @@
"type": "boolean", "type": "boolean",
"default": true "default": true
}, },
"markDeletedTablesFromFilterOnly": { "markAllDeletedTables": {
"description": "Optional configuration to mark deleted tables only to the filtered schema", "description": "Optional configuration to mark all deleted tables to the filtered schema",
"type": "boolean", "type": "boolean",
"default": false "default": false
}, },

View File

@ -170,15 +170,14 @@ const AddIngestion = ({
) )
: undefined : undefined
); );
const [markDeletedTablesFromFilterOnly, setMarkDeletedTablesFromFilterOnly] = const [markAllDeletedTables, setMarkAllDeletedTables] = useState(
useState( isDatabaseService
isDatabaseService ? Boolean(
? Boolean( (data?.sourceConfig.config as ConfigClass)?.markAllDeletedTables ??
(data?.sourceConfig.config as ConfigClass) false
?.markDeletedTablesFromFilterOnly ?? false )
) : undefined
: undefined );
);
const [includeView, setIncludeView] = useState( const [includeView, setIncludeView] = useState(
Boolean((data?.sourceConfig.config as ConfigClass)?.includeViews) Boolean((data?.sourceConfig.config as ConfigClass)?.includeViews)
); );
@ -466,7 +465,7 @@ const AddIngestion = ({
showTableFilter showTableFilter
), ),
markDeletedTables, markDeletedTables,
markDeletedTablesFromFilterOnly, markAllDeletedTables,
...DatabaseConfigData, ...DatabaseConfigData,
type: ConfigType.DatabaseMetadata, type: ConfigType.DatabaseMetadata,
}; };
@ -734,10 +733,10 @@ const AddIngestion = ({
handleIncludeView={() => setIncludeView((pre) => !pre)} handleIncludeView={() => setIncludeView((pre) => !pre)}
handleIngestSampleData={() => setIngestSampleData((pre) => !pre)} handleIngestSampleData={() => setIngestSampleData((pre) => !pre)}
handleIngestionName={(val) => setIngestionName(val)} handleIngestionName={(val) => setIngestionName(val)}
handleMarkDeletedTables={() => setMarkDeletedTables((pre) => !pre)} handleMarkAllDeletedTables={() =>
handleMarkDeletedTablesFromFilterOnly={() => setMarkAllDeletedTables((pre) => !pre)
setMarkDeletedTablesFromFilterOnly((pre) => !pre)
} }
handleMarkDeletedTables={() => setMarkDeletedTables((pre) => !pre)}
handleProfileSample={(val) => setProfileSample(val)} handleProfileSample={(val) => setProfileSample(val)}
handleQueryLogDuration={(val) => setQueryLogDuration(val)} handleQueryLogDuration={(val) => setQueryLogDuration(val)}
handleResultLimit={setResultLimit} handleResultLimit={setResultLimit}
@ -749,8 +748,8 @@ const AddIngestion = ({
includeView={includeView} includeView={includeView}
ingestSampleData={ingestSampleData} ingestSampleData={ingestSampleData}
ingestionName={ingestionName} ingestionName={ingestionName}
markAllDeletedTables={markAllDeletedTables}
markDeletedTables={markDeletedTables} markDeletedTables={markDeletedTables}
markDeletedTablesFromFilterOnly={markDeletedTablesFromFilterOnly}
mlModelFilterPattern={mlModelFilterPattern} mlModelFilterPattern={mlModelFilterPattern}
pipelineFilterPattern={pipelineFilterPattern} pipelineFilterPattern={pipelineFilterPattern}
pipelineType={pipelineType} pipelineType={pipelineType}

View File

@ -42,7 +42,7 @@ const ConfigureIngestion = ({
includeView, includeView,
includeTags, includeTags,
markDeletedTables, markDeletedTables,
markDeletedTablesFromFilterOnly, markAllDeletedTables,
serviceCategory, serviceCategory,
pipelineType, pipelineType,
showDatabaseFilter, showDatabaseFilter,
@ -70,7 +70,7 @@ const ConfigureIngestion = ({
handleIncludeView, handleIncludeView,
handleIncludeTags, handleIncludeTags,
handleMarkDeletedTables, handleMarkDeletedTables,
handleMarkDeletedTablesFromFilterOnly, handleMarkAllDeletedTables,
handleIngestSampleData, handleIngestSampleData,
handleDatasetServiceName, handleDatasetServiceName,
handleQueryLogDuration, handleQueryLogDuration,
@ -234,15 +234,15 @@ const ConfigureIngestion = ({
{getSeparator('')} {getSeparator('')}
</Field> </Field>
)} )}
{!isNil(markDeletedTablesFromFilterOnly) && ( {!isNil(markAllDeletedTables) && (
<Field> <Field>
<div className="tw-flex tw-gap-1"> <div className="tw-flex tw-gap-1">
<label>Mark Deleted Tables from Filter Only</label> <label>Mark All Deleted Tables</label>
<ToggleSwitchV1 <ToggleSwitchV1
checked={markDeletedTablesFromFilterOnly} checked={markAllDeletedTables}
handleCheck={() => { handleCheck={() => {
if (handleMarkDeletedTablesFromFilterOnly) { if (handleMarkAllDeletedTables) {
handleMarkDeletedTablesFromFilterOnly(); handleMarkAllDeletedTables();
} }
}} }}
testId="mark-deleted-filter-only" testId="mark-deleted-filter-only"

View File

@ -72,7 +72,7 @@ export interface ConfigureIngestionProps {
includeView: boolean; includeView: boolean;
includeTags: boolean; includeTags: boolean;
markDeletedTables?: boolean; markDeletedTables?: boolean;
markDeletedTablesFromFilterOnly?: boolean; markAllDeletedTables?: boolean;
enableDebugLog: boolean; enableDebugLog: boolean;
profileSample?: number; profileSample?: number;
ingestSampleData: boolean; ingestSampleData: boolean;
@ -98,7 +98,7 @@ export interface ConfigureIngestionProps {
handleIncludeView: () => void; handleIncludeView: () => void;
handleIncludeTags: () => void; handleIncludeTags: () => void;
handleMarkDeletedTables?: () => void; handleMarkDeletedTables?: () => void;
handleMarkDeletedTablesFromFilterOnly?: () => void; handleMarkAllDeletedTables?: () => void;
handleEnableDebugLog: () => void; handleEnableDebugLog: () => void;
handleIngestSampleData: () => void; handleIngestSampleData: () => void;
getIncludeValue: (value: string[], type: FilterPatternEnum) => void; getIncludeValue: (value: string[], type: FilterPatternEnum) => void;

View File

@ -23,7 +23,7 @@ export const mockIngestionPipeline = {
config: { config: {
type: 'DatabaseMetadata', type: 'DatabaseMetadata',
markDeletedTables: true, markDeletedTables: true,
markDeletedTablesFromFilterOnly: false, markAllDeletedTables: false,
includeTables: true, includeTables: true,
includeViews: true, includeViews: true,
includeTags: true, includeTags: true,