From 2f9f169de9784cda67fa1e566f3c17211a2bd799 Mon Sep 17 00:00:00 2001 From: Milan Bariya <52292922+MilanBariya@users.noreply.github.com> Date: Thu, 1 Dec 2022 16:26:43 +0530 Subject: [PATCH] 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 --- .../v007__create_db_connection_info.sql | 3 +++ .../v007__create_db_connection_info.sql | 3 +++ .../source/database/database_service.py | 9 ++++--- .../databaseServiceMetadataPipeline.json | 4 +-- .../AddIngestion/AddIngestion.component.tsx | 27 +++++++++---------- .../AddIngestion/Steps/ConfigureIngestion.tsx | 14 +++++----- .../AddIngestion/addIngestion.interface.ts | 4 +-- .../pages/LogsViewer/mocks/LogsViewer.mock.ts | 2 +- 8 files changed, 37 insertions(+), 29 deletions(-) create mode 100644 bootstrap/sql/com.mysql.cj.jdbc.Driver/v007__create_db_connection_info.sql create mode 100644 bootstrap/sql/org.postgresql.Driver/v007__create_db_connection_info.sql diff --git a/bootstrap/sql/com.mysql.cj.jdbc.Driver/v007__create_db_connection_info.sql b/bootstrap/sql/com.mysql.cj.jdbc.Driver/v007__create_db_connection_info.sql new file mode 100644 index 00000000000..873602c9e31 --- /dev/null +++ b/bootstrap/sql/com.mysql.cj.jdbc.Driver/v007__create_db_connection_info.sql @@ -0,0 +1,3 @@ +-- Remove markDeletedTablesFromFilterOnly +UPDATE ingestion_pipeline_entity +SET json = JSON_REMOVE(json ,'$.sourceConfig.config.markDeletedTablesFromFilterOnly'); \ No newline at end of file diff --git a/bootstrap/sql/org.postgresql.Driver/v007__create_db_connection_info.sql b/bootstrap/sql/org.postgresql.Driver/v007__create_db_connection_info.sql new file mode 100644 index 00000000000..1ad4a9690dc --- /dev/null +++ b/bootstrap/sql/org.postgresql.Driver/v007__create_db_connection_info.sql @@ -0,0 +1,3 @@ +-- Remove markDeletedTablesFromFilterOnly +UPDATE ingestion_pipeline_entity +SET json = json::jsonb #- '{sourceConfig,config,markDeletedTablesFromFilterOnly}'; \ No newline at end of file diff --git a/ingestion/src/metadata/ingestion/source/database/database_service.py b/ingestion/src/metadata/ingestion/source/database/database_service.py index d9a5fb7a94f..deb3e1e0ade 100644 --- a/ingestion/src/metadata/ingestion/source/database/database_service.py +++ b/ingestion/src/metadata/ingestion/source/database/database_service.py @@ -533,7 +533,12 @@ class DatabaseServiceSource( logger.info( 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() for schema_name in schema_names_list: schema_fqn = fqn.build( @@ -545,5 +550,3 @@ class DatabaseServiceSource( ) yield from self.delete_schema_tables(schema_fqn) - else: - yield from self.fetch_all_schema_and_delete_tables() diff --git a/openmetadata-spec/src/main/resources/json/schema/metadataIngestion/databaseServiceMetadataPipeline.json b/openmetadata-spec/src/main/resources/json/schema/metadataIngestion/databaseServiceMetadataPipeline.json index e820404c6ad..4147dda1939 100644 --- a/openmetadata-spec/src/main/resources/json/schema/metadataIngestion/databaseServiceMetadataPipeline.json +++ b/openmetadata-spec/src/main/resources/json/schema/metadataIngestion/databaseServiceMetadataPipeline.json @@ -23,8 +23,8 @@ "type": "boolean", "default": true }, - "markDeletedTablesFromFilterOnly": { - "description": "Optional configuration to mark deleted tables only to the filtered schema", + "markAllDeletedTables": { + "description": "Optional configuration to mark all deleted tables to the filtered schema", "type": "boolean", "default": false }, diff --git a/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/AddIngestion.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/AddIngestion.component.tsx index d71aa5d96d2..050a4b4c4fe 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/AddIngestion.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/AddIngestion.component.tsx @@ -170,15 +170,14 @@ const AddIngestion = ({ ) : undefined ); - const [markDeletedTablesFromFilterOnly, setMarkDeletedTablesFromFilterOnly] = - useState( - isDatabaseService - ? Boolean( - (data?.sourceConfig.config as ConfigClass) - ?.markDeletedTablesFromFilterOnly ?? false - ) - : undefined - ); + const [markAllDeletedTables, setMarkAllDeletedTables] = useState( + isDatabaseService + ? Boolean( + (data?.sourceConfig.config as ConfigClass)?.markAllDeletedTables ?? + false + ) + : undefined + ); const [includeView, setIncludeView] = useState( Boolean((data?.sourceConfig.config as ConfigClass)?.includeViews) ); @@ -466,7 +465,7 @@ const AddIngestion = ({ showTableFilter ), markDeletedTables, - markDeletedTablesFromFilterOnly, + markAllDeletedTables, ...DatabaseConfigData, type: ConfigType.DatabaseMetadata, }; @@ -734,10 +733,10 @@ const AddIngestion = ({ handleIncludeView={() => setIncludeView((pre) => !pre)} handleIngestSampleData={() => setIngestSampleData((pre) => !pre)} handleIngestionName={(val) => setIngestionName(val)} - handleMarkDeletedTables={() => setMarkDeletedTables((pre) => !pre)} - handleMarkDeletedTablesFromFilterOnly={() => - setMarkDeletedTablesFromFilterOnly((pre) => !pre) + handleMarkAllDeletedTables={() => + setMarkAllDeletedTables((pre) => !pre) } + handleMarkDeletedTables={() => setMarkDeletedTables((pre) => !pre)} handleProfileSample={(val) => setProfileSample(val)} handleQueryLogDuration={(val) => setQueryLogDuration(val)} handleResultLimit={setResultLimit} @@ -749,8 +748,8 @@ const AddIngestion = ({ includeView={includeView} ingestSampleData={ingestSampleData} ingestionName={ingestionName} + markAllDeletedTables={markAllDeletedTables} markDeletedTables={markDeletedTables} - markDeletedTablesFromFilterOnly={markDeletedTablesFromFilterOnly} mlModelFilterPattern={mlModelFilterPattern} pipelineFilterPattern={pipelineFilterPattern} pipelineType={pipelineType} diff --git a/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/Steps/ConfigureIngestion.tsx b/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/Steps/ConfigureIngestion.tsx index 343fa51c6dd..2f11daa69d5 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/Steps/ConfigureIngestion.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/Steps/ConfigureIngestion.tsx @@ -42,7 +42,7 @@ const ConfigureIngestion = ({ includeView, includeTags, markDeletedTables, - markDeletedTablesFromFilterOnly, + markAllDeletedTables, serviceCategory, pipelineType, showDatabaseFilter, @@ -70,7 +70,7 @@ const ConfigureIngestion = ({ handleIncludeView, handleIncludeTags, handleMarkDeletedTables, - handleMarkDeletedTablesFromFilterOnly, + handleMarkAllDeletedTables, handleIngestSampleData, handleDatasetServiceName, handleQueryLogDuration, @@ -234,15 +234,15 @@ const ConfigureIngestion = ({ {getSeparator('')} )} - {!isNil(markDeletedTablesFromFilterOnly) && ( + {!isNil(markAllDeletedTables) && (
- + { - if (handleMarkDeletedTablesFromFilterOnly) { - handleMarkDeletedTablesFromFilterOnly(); + if (handleMarkAllDeletedTables) { + handleMarkAllDeletedTables(); } }} testId="mark-deleted-filter-only" diff --git a/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/addIngestion.interface.ts b/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/addIngestion.interface.ts index 5db955a303f..497f2288d08 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/addIngestion.interface.ts +++ b/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/addIngestion.interface.ts @@ -72,7 +72,7 @@ export interface ConfigureIngestionProps { includeView: boolean; includeTags: boolean; markDeletedTables?: boolean; - markDeletedTablesFromFilterOnly?: boolean; + markAllDeletedTables?: boolean; enableDebugLog: boolean; profileSample?: number; ingestSampleData: boolean; @@ -98,7 +98,7 @@ export interface ConfigureIngestionProps { handleIncludeView: () => void; handleIncludeTags: () => void; handleMarkDeletedTables?: () => void; - handleMarkDeletedTablesFromFilterOnly?: () => void; + handleMarkAllDeletedTables?: () => void; handleEnableDebugLog: () => void; handleIngestSampleData: () => void; getIncludeValue: (value: string[], type: FilterPatternEnum) => void; diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/LogsViewer/mocks/LogsViewer.mock.ts b/openmetadata-ui/src/main/resources/ui/src/pages/LogsViewer/mocks/LogsViewer.mock.ts index d6cfb4c23e0..033c13b413f 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/LogsViewer/mocks/LogsViewer.mock.ts +++ b/openmetadata-ui/src/main/resources/ui/src/pages/LogsViewer/mocks/LogsViewer.mock.ts @@ -23,7 +23,7 @@ export const mockIngestionPipeline = { config: { type: 'DatabaseMetadata', markDeletedTables: true, - markDeletedTablesFromFilterOnly: false, + markAllDeletedTables: false, includeTables: true, includeViews: true, includeTags: true,