Skip to content

Commit

Permalink
Revert "fix: import should accept old keys (#17330)"
Browse files Browse the repository at this point in the history
This reverts commit 0b8507f.
  • Loading branch information
eschutho committed Dec 15, 2021
1 parent cda6250 commit 7520626
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 86 deletions.
17 changes: 1 addition & 16 deletions superset/databases/commands/export.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,25 +65,10 @@ def _export(model: Database) -> Iterator[Tuple[str, str]]:
include_defaults=True,
export_uuids=True,
)

# https://github.com/apache/superset/pull/16756 renamed ``allow_csv_upload``
# to ``allow_file_upload`, but we can't change the V1 schema
replacements = {"allow_file_upload": "allow_csv_upload"}
# this preserves key order, which is important
payload = {replacements.get(key, key): value for key, value in payload.items()}

# TODO (betodealmeida): move this logic to export_to_dict once this
# becomes the default export endpoint
if payload.get("extra"):
extra = payload["extra"] = parse_extra(payload["extra"])

# ``schemas_allowed_for_csv_upload`` was also renamed to
# ``schemas_allowed_for_file_upload``, we need to change to preserve the
# V1 schema
if "schemas_allowed_for_file_upload" in extra:
extra["schemas_allowed_for_csv_upload"] = extra.pop(
"schemas_allowed_for_file_upload"
)
payload["extra"] = parse_extra(payload["extra"])

payload["version"] = EXPORT_VERSION

Expand Down
7 changes: 0 additions & 7 deletions superset/databases/commands/importers/v1/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,6 @@ def import_database(
return existing
config["id"] = existing.id

# https://github.com/apache/superset/pull/16756 renamed ``csv`` to ``file``.
config["allow_file_upload"] = config.pop("allow_csv_upload")
if "schemas_allowed_for_csv_upload" in config["extra"]:
config["extra"]["schemas_allowed_for_file_upload"] = config["extra"].pop(
"schemas_allowed_for_csv_upload"
)

# TODO (betodealmeida): move this logic to import_from_dict
config["extra"] = json.dumps(config["extra"])

Expand Down
32 changes: 4 additions & 28 deletions superset/databases/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -558,19 +558,11 @@ def fix_schemas_allowed_for_csv_upload(
self, data: Dict[str, Any], **kwargs: Any
) -> Dict[str, Any]:
"""
Fixes for ``schemas_allowed_for_csv_upload``.
"""
# Fix for https://github.com/apache/superset/pull/16756, which temporarily
# changed the V1 schema. We need to support exports made after that PR and
# before this PR.
if "schemas_allowed_for_file_upload" in data:
data["schemas_allowed_for_csv_upload"] = data.pop(
"schemas_allowed_for_file_upload"
)
Fix ``schemas_allowed_for_csv_upload`` being a string.
# Fix ``schemas_allowed_for_csv_upload`` being a string.
# Due to a bug in the database modal, some databases might have been
# saved and exported with a string for ``schemas_allowed_for_csv_upload``.
Due to a bug in the database modal, some databases might have been
saved and exported with a string for ``schemas_allowed_for_csv_upload``.
"""
schemas_allowed_for_csv_upload = data.get("schemas_allowed_for_csv_upload")
if isinstance(schemas_allowed_for_csv_upload, str):
data["schemas_allowed_for_csv_upload"] = json.loads(
Expand All @@ -587,22 +579,6 @@ def fix_schemas_allowed_for_csv_upload(


class ImportV1DatabaseSchema(Schema):
# pylint: disable=no-self-use, unused-argument
@pre_load
def fix_allow_csv_upload(
self, data: Dict[str, Any], **kwargs: Any
) -> Dict[str, Any]:
"""
Fix for ``allow_csv_upload`` .
"""
# Fix for https://github.com/apache/superset/pull/16756, which temporarily
# changed the V1 schema. We need to support exports made after that PR and
# before this PR.
if "allow_file_upload" in data:
data["allow_csv_upload"] = data.pop("allow_file_upload")

return data

database_name = fields.String(required=True)
sqlalchemy_uri = fields.String(required=True)
password = fields.String(allow_none=True)
Expand Down
35 changes: 0 additions & 35 deletions tests/integration_tests/databases/commands_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,41 +338,6 @@ def test_import_v1_database(self):
db.session.delete(database)
db.session.commit()

def test_import_v1_database_broken_csv_fields(self):
"""
Test that a database can be imported with broken schema.
https://github.com/apache/superset/pull/16756 renamed some fields, changing
the V1 schema. This test ensures that we can import databases that were
exported with the broken schema.
"""
broken_config = database_config.copy()
broken_config["allow_file_upload"] = broken_config.pop("allow_csv_upload")
broken_config["extra"] = {"schemas_allowed_for_file_upload": ["upload"]}

contents = {
"metadata.yaml": yaml.safe_dump(database_metadata_config),
"databases/imported_database.yaml": yaml.safe_dump(broken_config),
}
command = ImportDatabasesCommand(contents)
command.run()

database = (
db.session.query(Database).filter_by(uuid=database_config["uuid"]).one()
)
assert database.allow_file_upload
assert database.allow_ctas
assert database.allow_cvas
assert not database.allow_run_async
assert database.cache_timeout is None
assert database.database_name == "imported_database"
assert database.expose_in_sqllab
assert database.extra == '{"schemas_allowed_for_file_upload": ["upload"]}'
assert database.sqlalchemy_uri == "sqlite:///test.db"

db.session.delete(database)
db.session.commit()

def test_import_v1_database_multiple(self):
"""Test that a database can be imported multiple times"""
num_databases = db.session.query(Database).count()
Expand Down

0 comments on commit 7520626

Please sign in to comment.