From daa89ef309adec7d23b20fb2b90031ffedf95de9 Mon Sep 17 00:00:00 2001 From: Vitor Avila <96086495+Vitor-Avila@users.noreply.github.com> Date: Tue, 11 Jul 2023 10:35:38 -0300 Subject: [PATCH 1/2] fix(dataset-import): support empty strings for extra fields * fix(dataset-import):support empty strings for extra fields * Adding unit test * black update --- superset/datasets/schemas.py | 9 ++- .../commands/importers/v1/import_test.py | 63 +++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py index 2b65d674ed41f..764384672326d 100644 --- a/superset/datasets/schemas.py +++ b/superset/datasets/schemas.py @@ -204,8 +204,13 @@ def fix_extra(self, data: dict[str, Any], **kwargs: Any) -> dict[str, Any]: """ Fix for extra initially being exported as a string. """ - if isinstance(data.get("extra"), str): - data["extra"] = json.loads(data["extra"]) + if "extra" in data: + extra = data["extra"] + if isinstance(extra, str): + if extra.strip(): + data["extra"] = json.loads(extra) + else: + data["extra"] = {} return data diff --git a/tests/unit_tests/datasets/commands/importers/v1/import_test.py b/tests/unit_tests/datasets/commands/importers/v1/import_test.py index 9e8690e6e35fc..aaa136965e3b4 100644 --- a/tests/unit_tests/datasets/commands/importers/v1/import_test.py +++ b/tests/unit_tests/datasets/commands/importers/v1/import_test.py @@ -359,6 +359,69 @@ def test_import_column_extra_is_string(mocker: MockFixture, session: Session) -> assert sqla_table.extra == '{"warning_markdown": "*WARNING*"}' +def test_import_dataset_extra_empty_string( + mocker: MockFixture, session: Session +) -> None: + """ + Test importing a dataset when the extra field is an empty string. + """ + from superset import security_manager + from superset.connectors.sqla.models import SqlaTable + from superset.datasets.commands.importers.v1.utils import import_dataset + from superset.datasets.schemas import ImportV1DatasetSchema + from superset.models.core import Database + + mocker.patch.object(security_manager, "can_access", return_value=True) + + engine = session.get_bind() + SqlaTable.metadata.create_all(engine) # pylint: disable=no-member + + database = Database(database_name="my_database", sqlalchemy_uri="sqlite://") + session.add(database) + session.flush() + + dataset_uuid = uuid.uuid4() + yaml_config: dict[str, Any] = { + "version": "1.0.0", + "table_name": "my_table", + "main_dttm_col": "ds", + "schema": "my_schema", + "sql": None, + "params": { + "remote_id": 64, + "database_name": "examples", + "import_time": 1606677834, + }, + "extra": " ", + "uuid": dataset_uuid, + "metrics": [ + { + "metric_name": "cnt", + "expression": "COUNT(*)", + } + ], + "columns": [ + { + "column_name": "profit", + "is_dttm": False, + "is_active": True, + "type": "INTEGER", + "groupby": False, + "filterable": False, + "expression": "revenue-expenses", + } + ], + "database_uuid": database.uuid, + } + + schema = ImportV1DatasetSchema() + dataset_config = schema.load(yaml_config) + dataset_config["database_id"] = database.id + sqla_table = import_dataset(session, dataset_config) + + assert sqla_table.extra == "{}" + + @patch("superset.datasets.commands.importers.v1.utils.request") def test_import_column_allowed_data_url( request: Mock, From 02a2626d4bea5123da49e01538e4f2fbf345ca67 Mon Sep 17 00:00:00 2001 From: Vitor Avila <96086495+Vitor-Avila@users.noreply.github.com> Date: Thu, 13 Jul 2023 00:57:11 -0300 Subject: [PATCH 2/2] Import fix (#2) * fix(dataset-import):support empty strings for extra fields * Adding unit tests * black update * Simplifying logic * Updating tests --- superset/datasets/schemas.py | 13 ++++++------- .../datasets/commands/importers/v1/import_test.py | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py index 764384672326d..dcac648148ed3 100644 --- a/superset/datasets/schemas.py +++ b/superset/datasets/schemas.py @@ -204,13 +204,12 @@ def fix_extra(self, data: dict[str, Any], **kwargs: Any) -> dict[str, Any]: """ Fix for extra initially being exported as a string. """ - if "extra" in data: - extra = data["extra"] - if isinstance(extra, str): - if extra.strip(): - data["extra"] = json.loads(extra) - else: - data["extra"] = {} + if isinstance(data.get("extra"), str): + try: + extra = data["extra"] + data["extra"] = json.loads(extra) if extra.strip() else None + except ValueError: + data["extra"] = None return data diff --git a/tests/unit_tests/datasets/commands/importers/v1/import_test.py b/tests/unit_tests/datasets/commands/importers/v1/import_test.py index aaa136965e3b4..e8e8c8e7c59fc 100644 --- a/tests/unit_tests/datasets/commands/importers/v1/import_test.py +++ b/tests/unit_tests/datasets/commands/importers/v1/import_test.py @@ -419,7 +419,7 @@ def test_import_dataset_extra_empty_string( dataset_config["database_id"] = database.id sqla_table = import_dataset(session, dataset_config) - assert sqla_table.extra == "{}" + assert sqla_table.extra == None @patch("superset.datasets.commands.importers.v1.utils.request")