From 6a404419e608e670e875150ec711a30802f03a41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Thu, 10 Oct 2024 18:04:59 +0200 Subject: [PATCH 01/18] enable audit in Snowflake --- .../0.0.0-dev/src/Snowflake_Data_Link.enso | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Snowflake_Data_Link.enso b/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Snowflake_Data_Link.enso index 192e13100004..34d3223f05d4 100644 --- a/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Snowflake_Data_Link.enso +++ b/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Snowflake_Data_Link.enso @@ -1,6 +1,6 @@ from Standard.Base import all import Standard.Base.Errors.Illegal_Argument.Illegal_Argument -from Standard.Base.Enso_Cloud.Data_Link_Helpers import parse_secure_value +from Standard.Base.Enso_Cloud.Data_Link_Helpers import Data_Link_Source_Metadata, parse_secure_value from Standard.Base.Enso_Cloud.Public_Utils import get_optional_field, get_required_field import Standard.Database.Connection.Connection_Options.Connection_Options @@ -12,15 +12,14 @@ import project.Connection.Snowflake_Details.Snowflake_Details type Snowflake_Data_Link ## PRIVATE A data-link returning a connection to the specified database. - Connection details:Snowflake_Details + Connection details:Snowflake_Details related_asset_id:Text|Nothing ## PRIVATE A data-link returning a query to a specific table within a database. - Table name:Text details:Snowflake_Details + Table name:Text details:Snowflake_Details related_asset_id:Text|Nothing ## PRIVATE parse json source -> Snowflake_Data_Link = - _ = source account = get_required_field "account" json expected_type=Text db_name = get_required_field "database_name" json expected_type=Text schema = get_optional_field "schema" json if_missing="SNOWFLAKE" expected_type=Text @@ -31,6 +30,9 @@ type Snowflake_Data_Link password = get_required_field "password" credentials_json |> parse_secure_value credentials = Credentials.Username_And_Password username password + related_asset_id = case source of + Data_Link_Source_Metadata.Cloud_Asset id -> id + _ -> Nothing details = Snowflake_Details.Snowflake account=account database=db_name schema=schema warehouse=warehouse credentials=credentials case get_optional_field "table" json expected_type=Text of Nothing -> @@ -42,9 +44,11 @@ type Snowflake_Data_Link read self (format = Auto_Detect) (on_problems : Problem_Behavior) = _ = on_problems if format != Auto_Detect then Error.throw (Illegal_Argument.Error "Only Auto_Detect can be used with a Snowflake Data Link, as it points to a database.") else - default_options = Connection_Options.Value - connection = self.details.connect default_options + audit_mode = if Enso_User.is_logged_in then "cloud" else "local" + options_vector = [["enso.internal.audit", audit_mode]] + (if self.related_asset_id.is_nothing then [] else [["enso.internal.relatedAssetId", self.related_asset_id]]) + default_options = Connection_Options.Value options_vector + connection = self.details.connect default_options allow_data_links=False case self of - Snowflake_Data_Link.Connection _ -> connection - Snowflake_Data_Link.Table table_name _ -> + Snowflake_Data_Link.Connection _ _ -> connection + Snowflake_Data_Link.Table table_name _ _ -> connection.query table_name From b8830c5b0bdb81913d53a7b98879340d640d0203 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Thu, 10 Oct 2024 18:07:41 +0200 Subject: [PATCH 02/18] tests --- test/Snowflake_Tests/src/Snowflake_Spec.enso | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/Snowflake_Tests/src/Snowflake_Spec.enso b/test/Snowflake_Tests/src/Snowflake_Spec.enso index e62b832eef27..d9c8ac39cada 100644 --- a/test/Snowflake_Tests/src/Snowflake_Spec.enso +++ b/test/Snowflake_Tests/src/Snowflake_Spec.enso @@ -20,6 +20,7 @@ from Standard.Test import all import Standard.Test.Test_Environment import enso_dev.Table_Tests +import enso_dev.Table_Tests.Database.Common.Audit_Spec import enso_dev.Table_Tests.Database.Common.Common_Spec import enso_dev.Table_Tests.Database.Common.IR_Spec import enso_dev.Table_Tests.Database.Transaction_Spec @@ -551,6 +552,9 @@ add_snowflake_specs suite_builder create_connection_fn db_name = Upload_Spec.add_specs suite_builder setup create_connection_fn IR_Spec.add_specs suite_builder setup prefix default_connection.get + # TODO + # Audit_Spec.add_specs suite_builder prefix data_link_file.get database_pending=pending + ## PRIVATE is_snowflake_integer value_type = case value_type of Value_Type.Integer _ -> True From 34aed2e455147083d34d154a176d318fe400ad58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Thu, 10 Oct 2024 18:13:16 +0200 Subject: [PATCH 03/18] share common code --- .../Data_Link/Postgres_Data_Link.enso | 17 ++++++----------- .../src/Internal/DB_Data_Link_Helpers.enso | 13 +++++++++++++ .../0.0.0-dev/src/Snowflake_Data_Link.enso | 17 ++++++----------- 3 files changed, 25 insertions(+), 22 deletions(-) create mode 100644 distribution/lib/Standard/Database/0.0.0-dev/src/Internal/DB_Data_Link_Helpers.enso diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/Connection/Data_Link/Postgres_Data_Link.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/Connection/Data_Link/Postgres_Data_Link.enso index 9c43742a41d1..4cfd028a3341 100644 --- a/distribution/lib/Standard/Database/0.0.0-dev/src/Connection/Data_Link/Postgres_Data_Link.enso +++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Connection/Data_Link/Postgres_Data_Link.enso @@ -9,17 +9,17 @@ from Standard.Base.Enso_Cloud.Public_Utils import get_optional_field, get_requir import project.Connection.Connection_Options.Connection_Options import project.Connection.Credentials.Credentials import project.Connection.Postgres.Postgres +import project.Internal.DB_Data_Link_Helpers ## PRIVATE type Postgres_Data_Link ## PRIVATE A data-link returning a connection to the specified database. - Connection details:Postgres related_asset_id:Text|Nothing + Connection details:Postgres source:Data_Link_Source_Metadata ## PRIVATE A data-link returning a query to a specific table within a database. - Table name:Text details:Postgres related_asset_id:Text|Nothing - + Table name:Text details:Postgres source:Data_Link_Source_Metadata ## PRIVATE parse json source:Data_Link_Source_Metadata -> Postgres_Data_Link = @@ -34,23 +34,18 @@ type Postgres_Data_Link password = get_required_field "password" credentials_json |> parse_secure_value Credentials.Username_And_Password username password - related_asset_id = case source of - Data_Link_Source_Metadata.Cloud_Asset id -> id - _ -> Nothing details = Postgres.Server host=host port=port database=db_name schema=schema credentials=credentials case get_optional_field "table" json expected_type=Text of Nothing -> - Postgres_Data_Link.Connection details related_asset_id + Postgres_Data_Link.Connection details source table_name : Text -> - Postgres_Data_Link.Table table_name details related_asset_id + Postgres_Data_Link.Table table_name details source ## PRIVATE read self (format = Auto_Detect) (on_problems : Problem_Behavior) = _ = on_problems if format != Auto_Detect then Error.throw (Illegal_Argument.Error "Only Auto_Detect can be used with a Postgres Data Link, as it points to a database.") else - audit_mode = if Enso_User.is_logged_in then "cloud" else "local" - options_vector = [["enso.internal.audit", audit_mode]] + (if self.related_asset_id.is_nothing then [] else [["enso.internal.relatedAssetId", self.related_asset_id]]) - default_options = Connection_Options.Value options_vector + default_options = DB_Data_Link_Helpers.data_link_connection_parameters self.source connection = self.details.connect default_options allow_data_links=False case self of Postgres_Data_Link.Connection _ _ -> connection diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/DB_Data_Link_Helpers.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/DB_Data_Link_Helpers.enso new file mode 100644 index 000000000000..f3ee644730cf --- /dev/null +++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/DB_Data_Link_Helpers.enso @@ -0,0 +1,13 @@ +from Standard.Base import all + +from Standard.Base.Enso_Cloud.Data_Link_Helpers import Data_Link_Source_Metadata + +import project.Connection.Connection_Options.Connection_Options + +data_link_connection_parameters (source : Data_Link_Source_Metadata) -> Vector = + related_asset_id = case source of + Data_Link_Source_Metadata.Cloud_Asset id -> id + _ -> Nothing + audit_mode = if Enso_User.is_logged_in then "cloud" else "local" + options_vector = [["enso.internal.audit", audit_mode]] + (if related_asset_id.is_nothing then [] else [["enso.internal.relatedAssetId", related_asset_id]]) + Connection_Options.Value options_vector diff --git a/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Snowflake_Data_Link.enso b/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Snowflake_Data_Link.enso index 34d3223f05d4..b098540dca09 100644 --- a/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Snowflake_Data_Link.enso +++ b/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Snowflake_Data_Link.enso @@ -3,8 +3,8 @@ import Standard.Base.Errors.Illegal_Argument.Illegal_Argument from Standard.Base.Enso_Cloud.Data_Link_Helpers import Data_Link_Source_Metadata, parse_secure_value from Standard.Base.Enso_Cloud.Public_Utils import get_optional_field, get_required_field -import Standard.Database.Connection.Connection_Options.Connection_Options import Standard.Database.Connection.Credentials.Credentials +import Standard.Database.Internal.DB_Data_Link_Helpers import project.Connection.Snowflake_Details.Snowflake_Details @@ -12,11 +12,11 @@ import project.Connection.Snowflake_Details.Snowflake_Details type Snowflake_Data_Link ## PRIVATE A data-link returning a connection to the specified database. - Connection details:Snowflake_Details related_asset_id:Text|Nothing + Connection details:Snowflake_Details source:Data_Link_Source_Metadata ## PRIVATE A data-link returning a query to a specific table within a database. - Table name:Text details:Snowflake_Details related_asset_id:Text|Nothing + Table name:Text details:Snowflake_Details source:Data_Link_Source_Metadata ## PRIVATE parse json source -> Snowflake_Data_Link = @@ -30,23 +30,18 @@ type Snowflake_Data_Link password = get_required_field "password" credentials_json |> parse_secure_value credentials = Credentials.Username_And_Password username password - related_asset_id = case source of - Data_Link_Source_Metadata.Cloud_Asset id -> id - _ -> Nothing details = Snowflake_Details.Snowflake account=account database=db_name schema=schema warehouse=warehouse credentials=credentials case get_optional_field "table" json expected_type=Text of Nothing -> - Snowflake_Data_Link.Connection details + Snowflake_Data_Link.Connection details source table_name : Text -> - Snowflake_Data_Link.Table table_name details + Snowflake_Data_Link.Table table_name details source ## PRIVATE read self (format = Auto_Detect) (on_problems : Problem_Behavior) = _ = on_problems if format != Auto_Detect then Error.throw (Illegal_Argument.Error "Only Auto_Detect can be used with a Snowflake Data Link, as it points to a database.") else - audit_mode = if Enso_User.is_logged_in then "cloud" else "local" - options_vector = [["enso.internal.audit", audit_mode]] + (if self.related_asset_id.is_nothing then [] else [["enso.internal.relatedAssetId", self.related_asset_id]]) - default_options = Connection_Options.Value options_vector + default_options = DB_Data_Link_Helpers.data_link_connection_parameters self.source connection = self.details.connect default_options allow_data_links=False case self of Snowflake_Data_Link.Connection _ _ -> connection From b2d9dee6d7d32ccf7bdfe784d0799785290ff909 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 11 Oct 2024 13:58:57 +0200 Subject: [PATCH 04/18] tests for Snowflake datalink / audit log --- .../data/datalinks/snowflake-db.datalink | 12 ++++++ test/Snowflake_Tests/src/Snowflake_Spec.enso | 40 ++++++++++++------- 2 files changed, 38 insertions(+), 14 deletions(-) create mode 100644 test/Snowflake_Tests/data/datalinks/snowflake-db.datalink diff --git a/test/Snowflake_Tests/data/datalinks/snowflake-db.datalink b/test/Snowflake_Tests/data/datalinks/snowflake-db.datalink new file mode 100644 index 000000000000..530163bdf67a --- /dev/null +++ b/test/Snowflake_Tests/data/datalinks/snowflake-db.datalink @@ -0,0 +1,12 @@ +{ + "type": "Snowflake_Connection", + "libraryName": "Standard.Snowflake", + "account": "ACCOUNT", + "database_name": "DBNAME", + "schema": "SCHEMA", + "warehouse": "WAREHOUSE", + "credentials": { + "username": "USERNAME", + "password": "PASSWORD" + } +} diff --git a/test/Snowflake_Tests/src/Snowflake_Spec.enso b/test/Snowflake_Tests/src/Snowflake_Spec.enso index d9c8ac39cada..a84102b1c8da 100644 --- a/test/Snowflake_Tests/src/Snowflake_Spec.enso +++ b/test/Snowflake_Tests/src/Snowflake_Spec.enso @@ -1,4 +1,5 @@ from Standard.Base import all +import Standard.Base.Enso_Cloud.Data_Link.Data_Link import Standard.Base.Errors.Illegal_Argument.Illegal_Argument import Standard.Base.Errors.Illegal_State.Illegal_State import Standard.Base.Runtime.Ref.Ref @@ -552,9 +553,6 @@ add_snowflake_specs suite_builder create_connection_fn db_name = Upload_Spec.add_specs suite_builder setup create_connection_fn IR_Spec.add_specs suite_builder setup prefix default_connection.get - # TODO - # Audit_Spec.add_specs suite_builder prefix data_link_file.get database_pending=pending - ## PRIVATE is_snowflake_integer value_type = case value_type of Value_Type.Integer _ -> True @@ -571,16 +569,38 @@ supported_replace_params = e4 = [Replace_Params.Value DB_Column Case_Sensitivity.Default False, Replace_Params.Value DB_Column Case_Sensitivity.Sensitive False] Hashset.from_vector <| e0 + e1 + e2 + e3 + e4 +transform_file base_file connection_details = + content = Data_Link.read_raw_config base_file + new_content = content + . replace "ACCOUNT" connection_details.account + . replace "DBNAME" connection_details.database + . replace "SCHEMA" connection_details.schema + . replace "WAREHOUSE" connection_details.warehouse + . replace "USERNAME" connection_details.credentials.username + . replace "PASSWORD" connection_details.credentials.password + temp_file = File.create_temporary_file "snowflake-test-db" ".datalink" + Data_Link.write_raw_config temp_file new_content replace_existing=True . if_not_error temp_file + +type Temporary_Data_Link_File + Value ~get + + make connection_details = Temporary_Data_Link_File.Value <| + transform_file (enso_project.data / "datalinks" / "snowflake-db.datalink") connection_details + add_table_specs suite_builder = - case create_connection_builder of + case get_configured_connection_details of Nothing -> message = "Snowflake test connection is not configured. See README.md for instructions." suite_builder.group "[Snowflake] Database tests" pending=message (_-> Nothing) - connection_builder -> - db_name = get_configured_connection_details.database + connection_details -> + db_name = connection_details.database + connection_builder = _ -> Database.connect connection_details add_snowflake_specs suite_builder connection_builder db_name Transaction_Spec.add_specs suite_builder connection_builder "[Snowflake] " + data_link_file = Temporary_Data_Link_File.make connection_details + Audit_Spec.add_specs suite_builder "[Snowflake] " data_link_file.get database_pending=Nothing + suite_builder.group "[Snowflake] Secrets in connection settings" group_builder-> cloud_setup = Cloud_Tests_Setup.prepare base_details = get_configured_connection_details @@ -627,14 +647,6 @@ get_configured_connection_details = Panic.rethrow <| credentials = Credentials.Username_And_Password user resolved_password Snowflake_Details.Snowflake account_name credentials database schema warehouse -## Returns a function that takes anything and returns a new connection. - The function creates a _new_ connection on each invocation - (this is needed for some tests that need multiple distinct connections). -create_connection_builder = - connection_details = get_configured_connection_details - connection_details.if_not_nothing <| - _ -> Database.connect connection_details - add_specs suite_builder = add_table_specs suite_builder From c9b2fb0e586495215b735d808684cb2c3f00f3a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 11 Oct 2024 14:11:45 +0200 Subject: [PATCH 05/18] update expiration logic based on running with actually expired token --- .../src/Enso_Cloud/Internal/Authentication.enso | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Enso_Cloud/Internal/Authentication.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Enso_Cloud/Internal/Authentication.enso index fd9eb6d5cc72..c58525f3efd7 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Enso_Cloud/Internal/Authentication.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Enso_Cloud/Internal/Authentication.enso @@ -145,8 +145,7 @@ type Refresh_Token_Data HTTP_Error.Status_Error status message _ -> Authentication_Service.log_message level=..Warning "Refresh token request failed with status "+status.to_text+": "+(message.if_nothing "")+"." - # As per OAuth specification, an expired refresh token should result in a 401 status code: https://www.rfc-editor.org/rfc/rfc6750.html#section-3.1 - if status.code == 401 then Panic.throw (Cloud_Session_Expired.Error error) else + if is_refresh_token_expired status message then Panic.throw (Cloud_Session_Expired.Error error) else # Otherwise, we fail with the generic error that gives more details. Panic.throw (Enso_Cloud_Error.Connection_Error error) _ -> Panic.throw (Enso_Cloud_Error.Connection_Error error) @@ -175,6 +174,13 @@ type Refresh_Token_Data it to reduce the chance of it expiring during a request. token_early_refresh_period = Duration.new minutes=2 +## PRIVATE +is_refresh_token_expired status message -> Boolean = + # As per OAuth specification, an expired refresh token should result in a 401 status code: https://www.rfc-editor.org/rfc/rfc6750.html#section-3.1 + # But empirically, it failed with: 400 Bad Request: {"__type":"NotAuthorizedException","message":"Refresh Token has expired"}. + # Let's just handle both just in case + (status.code == 401) || (status.code == 400 && message.contains '"Refresh Token has expired"') + ## PRIVATE A sibling to `get_required_field`. This one raises `Illegal_State` error, because it is dealing with local files and not cloud responses. From e971595ad42a44fbe5d53af8cca590ec7555c446 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 11 Oct 2024 14:38:56 +0200 Subject: [PATCH 06/18] fixes --- .../0.0.0-dev/src/Internal/DB_Data_Link_Helpers.enso | 2 +- test/Snowflake_Tests/src/Snowflake_Spec.enso | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/DB_Data_Link_Helpers.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/DB_Data_Link_Helpers.enso index f3ee644730cf..ed45cee3dc82 100644 --- a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/DB_Data_Link_Helpers.enso +++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/DB_Data_Link_Helpers.enso @@ -4,7 +4,7 @@ from Standard.Base.Enso_Cloud.Data_Link_Helpers import Data_Link_Source_Metadata import project.Connection.Connection_Options.Connection_Options -data_link_connection_parameters (source : Data_Link_Source_Metadata) -> Vector = +data_link_connection_parameters (source : Data_Link_Source_Metadata) -> Connection_Options = related_asset_id = case source of Data_Link_Source_Metadata.Cloud_Asset id -> id _ -> Nothing diff --git a/test/Snowflake_Tests/src/Snowflake_Spec.enso b/test/Snowflake_Tests/src/Snowflake_Spec.enso index a84102b1c8da..fb45b6d7dc6a 100644 --- a/test/Snowflake_Tests/src/Snowflake_Spec.enso +++ b/test/Snowflake_Tests/src/Snowflake_Spec.enso @@ -3,6 +3,7 @@ import Standard.Base.Enso_Cloud.Data_Link.Data_Link import Standard.Base.Errors.Illegal_Argument.Illegal_Argument import Standard.Base.Errors.Illegal_State.Illegal_State import Standard.Base.Runtime.Ref.Ref +from Standard.Base.Enso_Cloud.Data_Link_Helpers import secure_value_to_json from Standard.Table import Table, Value_Type, Aggregate_Column, Bits, expr from Standard.Table.Errors import Invalid_Column_Names, Inexact_Type_Coercion, Duplicate_Output_Column_Names @@ -571,6 +572,10 @@ supported_replace_params = transform_file base_file connection_details = content = Data_Link.read_raw_config base_file + + if connection_details.credentials.password.is_a Enso_Secret then + Panic.throw (Illegal_State.Error "Currently Snowflake tests need a plaintext password in environment variable to run.") + new_content = content . replace "ACCOUNT" connection_details.account . replace "DBNAME" connection_details.database @@ -578,6 +583,7 @@ transform_file base_file connection_details = . replace "WAREHOUSE" connection_details.warehouse . replace "USERNAME" connection_details.credentials.username . replace "PASSWORD" connection_details.credentials.password + temp_file = File.create_temporary_file "snowflake-test-db" ".datalink" Data_Link.write_raw_config temp_file new_content replace_existing=True . if_not_error temp_file From 748f97a5e1a025dd403725b1968b40065c4eca08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 11 Oct 2024 15:06:14 +0200 Subject: [PATCH 07/18] missing arg --- .../0.0.0-dev/src/Connection/Snowflake_Details.enso | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Connection/Snowflake_Details.enso b/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Connection/Snowflake_Details.enso index 5c2d18624a0e..4a738c6e1295 100644 --- a/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Connection/Snowflake_Details.enso +++ b/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Connection/Snowflake_Details.enso @@ -36,8 +36,10 @@ type Snowflake_Details Arguments: - options: Overrides for the connection properties. - connect : Connection_Options -> Snowflake_Connection - connect self options = + connect : Connection_Options -> Boolean -> Snowflake_Connection + connect self options (allow_data_links : Boolean = True) = + # TODO use this once #11294 is done + _ = allow_data_links properties = options.merge self.jdbc_properties ## Cannot use default argument values as gets in an infinite loop if you do. From 660504df4df5a25061e8e594c0c19d591259d209 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 11 Oct 2024 16:01:03 +0200 Subject: [PATCH 08/18] add tests for Snowflake data link schema --- app/dashboard/src/data/__tests__/dataLinkSchema.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/dashboard/src/data/__tests__/dataLinkSchema.test.ts b/app/dashboard/src/data/__tests__/dataLinkSchema.test.ts index e44ef308b83e..6309475912dc 100644 --- a/app/dashboard/src/data/__tests__/dataLinkSchema.test.ts +++ b/app/dashboard/src/data/__tests__/dataLinkSchema.test.ts @@ -34,6 +34,7 @@ const REPO_ROOT = url.fileURLToPath(new URL('../'.repeat(DIR_DEPTH), import.meta const BASE_DATA_LINKS_ROOT = path.resolve(REPO_ROOT, 'test/Base_Tests/data/datalinks/') const S3_DATA_LINKS_ROOT = path.resolve(REPO_ROOT, 'test/AWS_Tests/data/') const TABLE_DATA_LINKS_ROOT = path.resolve(REPO_ROOT, 'test/Table_Tests/data/datalinks/') +const SNOWFLAKE_DATA_LINKS_ROOT = path.resolve(REPO_ROOT, 'test/Snowflake_Tests/data/datalinks/') v.test('correctly validates example HTTP .datalink files with the schema', () => { const schemas = [ @@ -97,3 +98,11 @@ v.test('correctly validates example Database .datalink files with the schema', ( testSchema(json, schema) } }) + +v.test('correctly validates example Snowflake .datalink files with the schema', () => { + const schemas = ['snowflake-db.datalink'] + for (const schema of schemas) { + const json = loadDataLinkFile(path.resolve(SNOWFLAKE_DATA_LINKS_ROOT, schema)) + testSchema(json, schema) + } +}) \ No newline at end of file From 02fe4474c63248bb31b6e2e1bb515c8de1c209f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 11 Oct 2024 16:07:18 +0200 Subject: [PATCH 09/18] prettier --- app/dashboard/src/data/__tests__/dataLinkSchema.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/dashboard/src/data/__tests__/dataLinkSchema.test.ts b/app/dashboard/src/data/__tests__/dataLinkSchema.test.ts index 6309475912dc..39d7ac90888b 100644 --- a/app/dashboard/src/data/__tests__/dataLinkSchema.test.ts +++ b/app/dashboard/src/data/__tests__/dataLinkSchema.test.ts @@ -105,4 +105,4 @@ v.test('correctly validates example Snowflake .datalink files with the schema', const json = loadDataLinkFile(path.resolve(SNOWFLAKE_DATA_LINKS_ROOT, schema)) testSchema(json, schema) } -}) \ No newline at end of file +}) From 2a39e6f84817022c8d62ba02c0abcd8dab6cce0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 11 Oct 2024 16:09:39 +0200 Subject: [PATCH 10/18] missing priv --- .../Database/0.0.0-dev/src/Internal/DB_Data_Link_Helpers.enso | 1 + 1 file changed, 1 insertion(+) diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/DB_Data_Link_Helpers.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/DB_Data_Link_Helpers.enso index ed45cee3dc82..8a760ab55d50 100644 --- a/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/DB_Data_Link_Helpers.enso +++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Internal/DB_Data_Link_Helpers.enso @@ -4,6 +4,7 @@ from Standard.Base.Enso_Cloud.Data_Link_Helpers import Data_Link_Source_Metadata import project.Connection.Connection_Options.Connection_Options +## PRIVATE data_link_connection_parameters (source : Data_Link_Source_Metadata) -> Connection_Options = related_asset_id = case source of Data_Link_Source_Metadata.Cloud_Asset id -> id From 458fefde9a8472dddb902a4e1eda202f9c397722 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 11 Oct 2024 16:14:45 +0200 Subject: [PATCH 11/18] recover unexpected panics when printing GH errors in tests - this was preventing us from seeing failure reasons... --- .../lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso b/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso index 4482f12defd0..5ea58ba164f7 100644 --- a/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso +++ b/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso @@ -97,7 +97,10 @@ report_github_error_message (title : Text) (message : Text) = sanitize_parameter txt = txt . replace '\n' '%0A' . replace ',' '%2C' if is_enabled then location_match = Regex.compile "at ((?:[A-Za-z]:)?[^:]+):([0-9]+):" . match message - location_part = if location_match.is_nothing then "" else + on_location_failing_to_parse caught_panic = + IO.println "Failed to parse file location ["+location_match.to_text+"]: "+caught_panic.to_display_text + "" + location_part = if location_match.is_nothing then "" else Panic.catch Any handler=on_location_failing_to_parse <| repo_root = enso_project.root.parent.parent.absolute.normalize path = File.new (location_match.get 1) relative_path = repo_root.relativize path From 738e797f9c00ea1567f6b46edcdbccab66802016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 11 Oct 2024 17:06:00 +0200 Subject: [PATCH 12/18] fix weird syntax error --- .../Standard/Test/0.0.0-dev/src/Test_Reporter.enso | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso b/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso index 5ea58ba164f7..7edf924d1d1d 100644 --- a/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso +++ b/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso @@ -100,12 +100,13 @@ report_github_error_message (title : Text) (message : Text) = on_location_failing_to_parse caught_panic = IO.println "Failed to parse file location ["+location_match.to_text+"]: "+caught_panic.to_display_text "" - location_part = if location_match.is_nothing then "" else Panic.catch Any handler=on_location_failing_to_parse <| - repo_root = enso_project.root.parent.parent.absolute.normalize - path = File.new (location_match.get 1) - relative_path = repo_root.relativize path - line = location_match.get 2 - ",file="+(sanitize_parameter relative_path.path)+",line="+(sanitize_parameter line)+"" + location_part = if location_match.is_nothing then "" else + Panic.catch Any handler=on_location_failing_to_parse <| + repo_root = enso_project.root.parent.parent.absolute.normalize + path = File.new (location_match.get 1) + relative_path = repo_root.relativize path + line = location_match.get 2 + ",file="+(sanitize_parameter relative_path.path)+",line="+(sanitize_parameter line)+"" IO.println "::error title="+(sanitize_parameter title)+location_part+"::"+(sanitize_message message) ## Prints all the results, optionally writing them to a jUnit XML output. From 11189fcbd77a9399a851c5c2767269f69c0b2559 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 14 Oct 2024 11:17:46 +0200 Subject: [PATCH 13/18] wrap illegal argument exception as a panic --- .../main/java/org/enso/interpreter/runtime/data/EnsoFile.java | 1 + 1 file changed, 1 insertion(+) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoFile.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoFile.java index 25c12c1d45f3..43a16cd0a667 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoFile.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoFile.java @@ -614,6 +614,7 @@ public EnsoObject list() throws IOException { } @Builtin.Method + @Builtin.WrapException(from = IllegalArgumentException.class) @TruffleBoundary public EnsoFile relativize(EnsoFile other) { return new EnsoFile(this.truffleFile.relativize(other.truffleFile)); From 189701430002edff08b33e795041658134f4ee45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 14 Oct 2024 12:14:21 +0200 Subject: [PATCH 14/18] debug --- distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso b/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso index 7edf924d1d1d..081d30d5ac1d 100644 --- a/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso +++ b/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso @@ -98,7 +98,7 @@ report_github_error_message (title : Text) (message : Text) = if is_enabled then location_match = Regex.compile "at ((?:[A-Za-z]:)?[^:]+):([0-9]+):" . match message on_location_failing_to_parse caught_panic = - IO.println "Failed to parse file location ["+location_match.to_text+"]: "+caught_panic.to_display_text + IO.println "Failed to parse file location ["+location_match.to_display_text+"]: "+caught_panic.to_display_text "" location_part = if location_match.is_nothing then "" else Panic.catch Any handler=on_location_failing_to_parse <| From 49bbb2b8d525c7c78b0da57d3eafc21fecd1e758 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 14 Oct 2024 12:51:26 +0200 Subject: [PATCH 15/18] split location parsing to be unit testable, fix bad regex, add support for ranges and columns --- .../Test/0.0.0-dev/src/Test_Reporter.enso | 43 +++++++++++++------ 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso b/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso index 081d30d5ac1d..a8f815936191 100644 --- a/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso +++ b/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso @@ -1,8 +1,10 @@ private from Standard.Base import all +import Standard.Base.Errors.Illegal_Argument.Illegal_Argument import Standard.Base.Runtime.Context import Standard.Base.Runtime.Ref.Ref +from Standard.Base.Logging import all from Standard.Base.Runtime import assert import project.Group.Group @@ -93,21 +95,36 @@ print_single_result (test_result : Test_Result) (config : Suite_Config) = only if running in the GitHub Actions environment. report_github_error_message (title : Text) (message : Text) = is_enabled = Environment.get "GITHUB_ACTIONS" == "true" + if is_enabled then + IO.println (generate_github_error_annotation title message) + +## PRIVATE + Generates a GitHub Actions annotation for a failing test. +generate_github_error_annotation (title : Text) (message : Text) = sanitize_message txt = txt.replace '\n' '%0A' sanitize_parameter txt = txt . replace '\n' '%0A' . replace ',' '%2C' - if is_enabled then - location_match = Regex.compile "at ((?:[A-Za-z]:)?[^:]+):([0-9]+):" . match message - on_location_failing_to_parse caught_panic = - IO.println "Failed to parse file location ["+location_match.to_display_text+"]: "+caught_panic.to_display_text - "" - location_part = if location_match.is_nothing then "" else - Panic.catch Any handler=on_location_failing_to_parse <| - repo_root = enso_project.root.parent.parent.absolute.normalize - path = File.new (location_match.get 1) - relative_path = repo_root.relativize path - line = location_match.get 2 - ",file="+(sanitize_parameter relative_path.path)+",line="+(sanitize_parameter line)+"" - IO.println "::error title="+(sanitize_parameter title)+location_part+"::"+(sanitize_message message) + + location_match = Regex.compile "\(at ((?:[A-Za-z]:)?[^:]+):([0-9\-]+):([0-9\-]+)\)" . match message + split_on_dash start_field_name end_field_name text = + if text.contains "-" . not then start_field_name+"="+text else + parts = text.split "-" + if parts.length != 2 then Panic.throw (Illegal_Argument.Error "Invalid range format: "+text) else + start = parts.at 0 + end = parts.at 1 + start_field_name+"="+start+","+end_field_name+"="+end + on_location_failing_to_parse caught_panic = + Test_Result.log_message "Failed to parse file location ["+location_match.to_display_text+"]: "+caught_panic.to_display_text + "" + location_part = if location_match.is_nothing then "" else + Panic.catch Any handler=on_location_failing_to_parse <| + repo_root = enso_project.root.parent.parent.absolute.normalize + path = File.new (location_match.get 1) + relative_path = repo_root.relativize path + lines_part = split_on_dash "line" "endLine" (location_match.get 2) + columns_part = split_on_dash "col" "endColumn" (location_match.get 3) + ",file="+(sanitize_parameter relative_path.path)+","+lines_part+","+columns_part + + "::error title="+(sanitize_parameter title)+location_part+"::"+(sanitize_message message) ## Prints all the results, optionally writing them to a jUnit XML output. From 7dd4df133da62aaed9c4c34b0b1c5ea5a6a057bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 14 Oct 2024 13:31:35 +0200 Subject: [PATCH 16/18] tests, better error handling --- .../Test/0.0.0-dev/src/Test_Reporter.enso | 8 +++-- .../src/Github_Annotations_Spec.enso | 31 +++++++++++++++++++ test/Base_Internal_Tests/src/Main.enso | 2 ++ 3 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 test/Base_Internal_Tests/src/Github_Annotations_Spec.enso diff --git a/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso b/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso index a8f815936191..0ac6fe7d9cab 100644 --- a/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso +++ b/distribution/lib/Standard/Test/0.0.0-dev/src/Test_Reporter.enso @@ -14,6 +14,7 @@ import project.Suite_Config.Suite_Config import project.Test.Test import project.Test_Result.Test_Result +polyglot java import java.lang.IllegalArgumentException polyglot java import java.lang.StringBuilder polyglot java import java.lang.System as Java_System @@ -113,13 +114,16 @@ generate_github_error_annotation (title : Text) (message : Text) = end = parts.at 1 start_field_name+"="+start+","+end_field_name+"="+end on_location_failing_to_parse caught_panic = - Test_Result.log_message "Failed to parse file location ["+location_match.to_display_text+"]: "+caught_panic.to_display_text + Test_Result.log_message "Failed to parse file location ["+location_match.to_display_text+"]: "+caught_panic.payload.to_display_text "" location_part = if location_match.is_nothing then "" else Panic.catch Any handler=on_location_failing_to_parse <| repo_root = enso_project.root.parent.parent.absolute.normalize path = File.new (location_match.get 1) - relative_path = repo_root.relativize path + ## We try to relativize the path, but if it fails, we just use the original path. + (It may fail eg. if the project root and the test files are on different drives - + very unlikely but not impossible if tests import other libraries located in weird places.) + relative_path = Panic.catch IllegalArgumentException (repo_root.relativize path) _->path lines_part = split_on_dash "line" "endLine" (location_match.get 2) columns_part = split_on_dash "col" "endColumn" (location_match.get 3) ",file="+(sanitize_parameter relative_path.path)+","+lines_part+","+columns_part diff --git a/test/Base_Internal_Tests/src/Github_Annotations_Spec.enso b/test/Base_Internal_Tests/src/Github_Annotations_Spec.enso new file mode 100644 index 000000000000..aba1aa306644 --- /dev/null +++ b/test/Base_Internal_Tests/src/Github_Annotations_Spec.enso @@ -0,0 +1,31 @@ +from Standard.Base import all + +from Standard.Test import all +import Standard.Test.Test_Reporter + +add_specs suite_builder = suite_builder.group "Test Reporter running on GitHub" group_builder-> + file = (enso_project.root / "src" / "test-file.enso") . absolute . normalize + file_relative_to_repo_root = (File.new "test") / "Base_Internal_Tests" / "src" / "test-file.enso" + group_builder.specify "should correctly parse error message" <| + message = "[False, False, False, True] did not equal [False, False, True, True]; first difference at index 2 (at "+file.path+":1:13-110)." + line = Test_Reporter.generate_github_error_annotation 'Test, and\n special characters' message + line.should_equal "::error title=Test%2C and%0A special characters,file="+file_relative_to_repo_root.path+",line=1,col=13,endColumn=110::[False, False, False, True] did not equal [False, False, True, True]; first difference at index 2 (at "+file.path+":1:13-110)." + + group_builder.specify "should be able to parse dashes" <| + message = "test failure (at "+file.path+":1-2:3-4)." + line = Test_Reporter.generate_github_error_annotation "Test" message + line.should_equal "::error title=Test,file="+file_relative_to_repo_root.path+",line=1,endLine=2,col=3,endColumn=4::test failure (at "+file.path+":1-2:3-4)." + + message2 = "test failure (at "+file.path+":1234-5678:91011-121314)." + line2 = Test_Reporter.generate_github_error_annotation "Test" message2 + line2.should_equal "::error title=Test,file="+file_relative_to_repo_root.path+",line=1234,endLine=5678,col=91011,endColumn=121314::test failure (at "+file.path+":1234-5678:91011-121314)." + + group_builder.specify "should be able to parse no dashes" <| + message = "test failure (at "+file.path+":1234:7)." + line = Test_Reporter.generate_github_error_annotation "Test" message + line.should_equal "::error title=Test,file="+file_relative_to_repo_root.path+",line=1234,col=7::test failure (at "+file.path+":1234:7)." + +main filter=Nothing = + suite = Test.build suite_builder-> + add_specs suite_builder + suite.run_with_filter filter diff --git a/test/Base_Internal_Tests/src/Main.enso b/test/Base_Internal_Tests/src/Main.enso index a1fc96bf8faa..b256c798126c 100644 --- a/test/Base_Internal_Tests/src/Main.enso +++ b/test/Base_Internal_Tests/src/Main.enso @@ -6,6 +6,7 @@ import project.Input_Output_Spec import project.Comparator_Spec import project.Decimal_Constructor_Spec import project.Grapheme_Spec +import project.Github_Annotations_Spec main filter=Nothing = suite = Test.build suite_builder-> @@ -13,5 +14,6 @@ main filter=Nothing = Decimal_Constructor_Spec.add_specs suite_builder Grapheme_Spec.add_specs suite_builder Input_Output_Spec.add_specs suite_builder + Github_Annotations_Spec.add_specs suite_builder suite.run_with_filter filter From 36b1dff08a56d9a05936307bbaa6d4fc7005e1dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 14 Oct 2024 13:41:32 +0200 Subject: [PATCH 17/18] fix test: Snowflake DOES support separate NaN --- .../Snowflake/0.0.0-dev/src/Internal/Snowflake_Dialect.enso | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Dialect.enso b/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Dialect.enso index d629e360f262..a7b7f6d0f44e 100644 --- a/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Dialect.enso +++ b/distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Dialect.enso @@ -250,7 +250,7 @@ type Snowflake_Dialect Dialect_Flag.Supports_Float_Decimal_Places -> True Dialect_Flag.Use_Builtin_Bankers -> True Dialect_Flag.Primary_Key_Allows_Nulls -> False - Dialect_Flag.Supports_Separate_NaN -> False + Dialect_Flag.Supports_Separate_NaN -> True Dialect_Flag.Supports_Nested_With_Clause -> True Dialect_Flag.Supports_Case_Sensitive_Columns -> True From c6fb76ba8492f5078a553704924bee0a78decce0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 14 Oct 2024 14:42:32 +0200 Subject: [PATCH 18/18] fix coalesce tests: ensure table has expected ordering --- .../Table_Tests/src/Common_Table_Operations/Coalesce_Spec.enso | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/Table_Tests/src/Common_Table_Operations/Coalesce_Spec.enso b/test/Table_Tests/src/Common_Table_Operations/Coalesce_Spec.enso index a2592d50bae4..6dae9a362c1e 100644 --- a/test/Table_Tests/src/Common_Table_Operations/Coalesce_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Operations/Coalesce_Spec.enso @@ -7,6 +7,7 @@ from Standard.Database.Errors import Integrity_Error from Standard.Test import all from project.Common_Table_Operations.Util import run_default_backend +import project.Common_Table_Operations.Util main filter=Nothing = run_default_backend add_specs filter @@ -15,7 +16,7 @@ add_specs suite_builder setup = add_coalesce_specs suite_builder setup = prefix = setup.prefix - table_builder = setup.table_builder + table_builder = Util.build_sorted_table setup suite_builder.group prefix+"Table.coalesce" group_builder-> group_builder.specify "2 columns" <|