From d6ae3a59190471162dee049149edfcc96d78dcab Mon Sep 17 00:00:00 2001 From: Mayuri N Date: Tue, 21 Jan 2025 20:50:48 +0530 Subject: [PATCH 1/5] fix(ingest): fix reporting for missing secure view lineage Secure Views in databases created from Snowflake Shares will always be missing on view definitions and that need not be reported as warning. We would report such views as info in pipeline run. --- .../source/snowflake/snowflake_schema_gen.py | 12 ++++- .../snowflake/test_snowflake_failures.py | 52 +++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/metadata-ingestion/src/datahub/ingestion/source/snowflake/snowflake_schema_gen.py b/metadata-ingestion/src/datahub/ingestion/source/snowflake/snowflake_schema_gen.py index a2d69d9e552916..99810788335e24 100644 --- a/metadata-ingestion/src/datahub/ingestion/source/snowflake/snowflake_schema_gen.py +++ b/metadata-ingestion/src/datahub/ingestion/source/snowflake/snowflake_schema_gen.py @@ -491,15 +491,23 @@ def fetch_secure_view_definition( try: view_definitions = self.data_dictionary.get_secure_view_definitions() return view_definitions[db_name][schema_name][table_name] + except KeyError: + # Received secure view definitions but the view is not present in results + self.structured_reporter.info( + "Secure view definition not found. Lineage will be missing for the view.", + context=f"{db_name}.{schema_name}.{table_name}", + ) + return None except Exception as e: if isinstance(e, SnowflakePermissionError): error_msg = ( "Failed to get secure views definitions. Please check permissions." ) else: - error_msg = "Failed to get secure views definitions" + error_msg = "Failed to get secure views definitions." self.structured_reporter.warning( - error_msg, + error_msg + " Lineage will be missing for the view.", + context=f"{db_name}.{schema_name}.{table_name}", exc=e, ) return None diff --git a/metadata-ingestion/tests/integration/snowflake/test_snowflake_failures.py b/metadata-ingestion/tests/integration/snowflake/test_snowflake_failures.py index de6e996a52642b..bc8e84631ed33a 100644 --- a/metadata-ingestion/tests/integration/snowflake/test_snowflake_failures.py +++ b/metadata-ingestion/tests/integration/snowflake/test_snowflake_failures.py @@ -260,3 +260,55 @@ def test_snowflake_missing_snowflake_operations_permission_causes_pipeline_failu assert "usage-permission-error" in [ failure.message for failure in pipeline.source.get_report().failures ] + + +@freeze_time(FROZEN_TIME) +def test_snowflake_missing_snowflake_secure_view_definitions_raises_pipeline_info( + pytestconfig, + snowflake_pipeline_config, +): + with mock.patch("snowflake.connector.connect") as mock_connect: + sf_connection = mock.MagicMock() + sf_cursor = mock.MagicMock() + mock_connect.return_value = sf_connection + sf_connection.cursor.return_value = sf_cursor + + # Error in getting access history date range + sf_cursor.execute.side_effect = query_permission_response_override( + default_query_results, + [snowflake_query.SnowflakeQuery.get_secure_view_definitions()], + [], + ) + pipeline = Pipeline(snowflake_pipeline_config) + pipeline.run() + + pipeline.raise_from_status(raise_warnings=True) + assert ( + "Secure view definition not found. Lineage will be missing for the view." + in [info.message for info in pipeline.source.get_report().infos] + ) + + +@freeze_time(FROZEN_TIME) +def test_snowflake_failed_secure_view_definitions_query_raises_pipeline_warning( + pytestconfig, + snowflake_pipeline_config, +): + with mock.patch("snowflake.connector.connect") as mock_connect: + sf_connection = mock.MagicMock() + sf_cursor = mock.MagicMock() + mock_connect.return_value = sf_connection + sf_connection.cursor.return_value = sf_cursor + + # Error in getting access history date range + sf_cursor.execute.side_effect = query_permission_error_override( + default_query_results, + [snowflake_query.SnowflakeQuery.get_secure_view_definitions()], + "Database 'SNOWFLAKE' does not exist or not authorized.", + ) + pipeline = Pipeline(snowflake_pipeline_config) + pipeline.run() + assert ( + "Failed to get secure views definitions. Please check permissions. " + "Lineage will be missing for the view." + ) in [info.message for info in pipeline.source.get_report().warnings] From 37af41ef4f30c65c4eff0585d47c0b783ba0ae2d Mon Sep 17 00:00:00 2001 From: Mayuri Nehate <33225191+mayurinehate@users.noreply.github.com> Date: Wed, 22 Jan 2025 14:26:37 +0530 Subject: [PATCH 2/5] fix comments --- .../tests/integration/snowflake/test_snowflake_failures.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/metadata-ingestion/tests/integration/snowflake/test_snowflake_failures.py b/metadata-ingestion/tests/integration/snowflake/test_snowflake_failures.py index bc8e84631ed33a..93e6339c7ba956 100644 --- a/metadata-ingestion/tests/integration/snowflake/test_snowflake_failures.py +++ b/metadata-ingestion/tests/integration/snowflake/test_snowflake_failures.py @@ -273,7 +273,7 @@ def test_snowflake_missing_snowflake_secure_view_definitions_raises_pipeline_inf mock_connect.return_value = sf_connection sf_connection.cursor.return_value = sf_cursor - # Error in getting access history date range + # Empty secure view definitions sf_cursor.execute.side_effect = query_permission_response_override( default_query_results, [snowflake_query.SnowflakeQuery.get_secure_view_definitions()], @@ -300,7 +300,7 @@ def test_snowflake_failed_secure_view_definitions_query_raises_pipeline_warning( mock_connect.return_value = sf_connection sf_connection.cursor.return_value = sf_cursor - # Error in getting access history date range + # Error in getting secure view definitions sf_cursor.execute.side_effect = query_permission_error_override( default_query_results, [snowflake_query.SnowflakeQuery.get_secure_view_definitions()], From 22f201df82f9543c962255ad3a041b2cd1999d6d Mon Sep 17 00:00:00 2001 From: Mayuri Nehate <33225191+mayurinehate@users.noreply.github.com> Date: Thu, 23 Jan 2025 23:49:26 +0530 Subject: [PATCH 3/5] Update metadata-ingestion/src/datahub/ingestion/source/snowflake/snowflake_schema_gen.py Co-authored-by: Harshal Sheth --- .../datahub/ingestion/source/snowflake/snowflake_schema_gen.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metadata-ingestion/src/datahub/ingestion/source/snowflake/snowflake_schema_gen.py b/metadata-ingestion/src/datahub/ingestion/source/snowflake/snowflake_schema_gen.py index 99810788335e24..59f1b3d166a285 100644 --- a/metadata-ingestion/src/datahub/ingestion/source/snowflake/snowflake_schema_gen.py +++ b/metadata-ingestion/src/datahub/ingestion/source/snowflake/snowflake_schema_gen.py @@ -506,7 +506,7 @@ def fetch_secure_view_definition( else: error_msg = "Failed to get secure views definitions." self.structured_reporter.warning( - error_msg + " Lineage will be missing for the view.", + f"{error_msg} Lineage will be missing for the view.", context=f"{db_name}.{schema_name}.{table_name}", exc=e, ) From e9f3835d12524c3fb28b15d0cd05b750f6bd580a Mon Sep 17 00:00:00 2001 From: Mayuri N Date: Fri, 24 Jan 2025 12:04:37 +0530 Subject: [PATCH 4/5] add title to reporting --- .../source/snowflake/snowflake_schema_gen.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/metadata-ingestion/src/datahub/ingestion/source/snowflake/snowflake_schema_gen.py b/metadata-ingestion/src/datahub/ingestion/source/snowflake/snowflake_schema_gen.py index 59f1b3d166a285..04bc51f1ebd3f5 100644 --- a/metadata-ingestion/src/datahub/ingestion/source/snowflake/snowflake_schema_gen.py +++ b/metadata-ingestion/src/datahub/ingestion/source/snowflake/snowflake_schema_gen.py @@ -494,19 +494,21 @@ def fetch_secure_view_definition( except KeyError: # Received secure view definitions but the view is not present in results self.structured_reporter.info( - "Secure view definition not found. Lineage will be missing for the view.", + title="Secure view definition not found", + message="Lineage will be missing for the view.", context=f"{db_name}.{schema_name}.{table_name}", ) return None except Exception as e: - if isinstance(e, SnowflakePermissionError): - error_msg = ( - "Failed to get secure views definitions. Please check permissions." - ) - else: - error_msg = "Failed to get secure views definitions." + action_msg = ( + "Please check permissions." + if isinstance(e, SnowflakePermissionError) + else "" + ) + self.structured_reporter.warning( - f"{error_msg} Lineage will be missing for the view.", + title="Failed to get secure views definitions", + message=f"Lineage will be missing for the view. {action_msg}", context=f"{db_name}.{schema_name}.{table_name}", exc=e, ) From cc402406e696ec492b45fdb89d06959becdf571e Mon Sep 17 00:00:00 2001 From: Mayuri N Date: Fri, 24 Jan 2025 13:44:05 +0530 Subject: [PATCH 5/5] update test --- .../snowflake/test_snowflake_failures.py | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/metadata-ingestion/tests/integration/snowflake/test_snowflake_failures.py b/metadata-ingestion/tests/integration/snowflake/test_snowflake_failures.py index 93e6339c7ba956..4cb6cec4906efa 100644 --- a/metadata-ingestion/tests/integration/snowflake/test_snowflake_failures.py +++ b/metadata-ingestion/tests/integration/snowflake/test_snowflake_failures.py @@ -283,10 +283,13 @@ def test_snowflake_missing_snowflake_secure_view_definitions_raises_pipeline_inf pipeline.run() pipeline.raise_from_status(raise_warnings=True) - assert ( - "Secure view definition not found. Lineage will be missing for the view." - in [info.message for info in pipeline.source.get_report().infos] - ) + assert pipeline.source.get_report().infos.as_obj() == [ + { + "title": "Secure view definition not found", + "message": "Lineage will be missing for the view.", + "context": ["TEST_DB.TEST_SCHEMA.VIEW_1"], + } + ] @freeze_time(FROZEN_TIME) @@ -308,7 +311,12 @@ def test_snowflake_failed_secure_view_definitions_query_raises_pipeline_warning( ) pipeline = Pipeline(snowflake_pipeline_config) pipeline.run() - assert ( - "Failed to get secure views definitions. Please check permissions. " - "Lineage will be missing for the view." - ) in [info.message for info in pipeline.source.get_report().warnings] + assert pipeline.source.get_report().warnings.as_obj() == [ + { + "title": "Failed to get secure views definitions", + "message": "Lineage will be missing for the view. Please check permissions.", + "context": [ + "TEST_DB.TEST_SCHEMA.VIEW_1 : Database 'SNOWFLAKE' does not exist or not authorized." + ], + } + ]