Skip to content

Commit

Permalink
fix(dataset-import): support empty strings for extra fields (#24663)
Browse files Browse the repository at this point in the history
(cherry picked from commit 65fb8e1)
  • Loading branch information
Vitor-Avila authored and michael-s-molina committed Jul 19, 2023
1 parent c6ecbc8 commit c94bee4
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 1 deletion.
6 changes: 5 additions & 1 deletion superset/datasets/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,11 @@ 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"])
try:
extra = data["extra"]
data["extra"] = json.loads(extra) if extra.strip() else None
except ValueError:
data["extra"] = None

return data

Expand Down
63 changes: 63 additions & 0 deletions tests/unit_tests/datasets/commands/importers/v1/import_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 == None


@patch("superset.datasets.commands.importers.v1.utils.request")
def test_import_column_allowed_data_url(
request: Mock,
Expand Down

0 comments on commit c94bee4

Please sign in to comment.