You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2021/12/15 17:34:42 UTC
[superset] 02/02: Revert "fix: import should accept old keys (#17330)"
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch 1.4
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 752062674b554b8a53d90242eb482b82e4936c7e
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Wed Dec 15 09:30:26 2021 -0800
Revert "fix: import should accept old keys (#17330)"
This reverts commit 0b8507fe77d1f96cdd70cdc21c19bb7e94449e4c.
---
superset/databases/commands/export.py | 17 +----------
superset/databases/commands/importers/v1/utils.py | 7 -----
superset/databases/schemas.py | 32 +++-----------------
.../integration_tests/databases/commands_tests.py | 35 ----------------------
4 files changed, 5 insertions(+), 86 deletions(-)
diff --git a/superset/databases/commands/export.py b/superset/databases/commands/export.py
index 134bda5..786eb56 100644
--- a/superset/databases/commands/export.py
+++ b/superset/databases/commands/export.py
@@ -65,25 +65,10 @@ class ExportDatabasesCommand(ExportModelsCommand):
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
diff --git a/superset/databases/commands/importers/v1/utils.py b/superset/databases/commands/importers/v1/utils.py
index 6704ccd..6e016d0 100644
--- a/superset/databases/commands/importers/v1/utils.py
+++ b/superset/databases/commands/importers/v1/utils.py
@@ -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"])
diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py
index 6599e8d..763e7db 100644
--- a/superset/databases/schemas.py
+++ b/superset/databases/schemas.py
@@ -558,19 +558,11 @@ class ImportV1DatabaseExtraSchema(Schema):
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(
@@ -587,22 +579,6 @@ class ImportV1DatabaseExtraSchema(Schema):
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)
diff --git a/tests/integration_tests/databases/commands_tests.py b/tests/integration_tests/databases/commands_tests.py
index 389b1ac..449dd4c 100644
--- a/tests/integration_tests/databases/commands_tests.py
+++ b/tests/integration_tests/databases/commands_tests.py
@@ -338,41 +338,6 @@ class TestImportDatabasesCommand(SupersetTestCase):
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()