You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2021/05/27 21:48:06 UTC
[superset] branch master updated: fix: show error on invalid import
(#14851)
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 2313e3e fix: show error on invalid import (#14851)
2313e3e is described below
commit 2313e3ef4f328cac2a61b39fa05b0a0ea3560704
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Thu May 27 14:46:41 2021 -0700
fix: show error on invalid import (#14851)
* fix: show error on invalid import
* Add unit test
* Remove unused imports
* Fix tests
---
superset/charts/api.py | 17 +++---
superset/commands/importers/exceptions.py | 5 ++
superset/dashboards/api.py | 18 +++----
superset/databases/api.py | 18 +++----
superset/datasets/api.py | 18 +++----
superset/queries/saved_queries/api.py | 17 +++---
tests/charts/api_tests.py | 39 ++++++++++++--
tests/dashboards/api_tests.py | 86 +++++++++++++++++++++++++++++--
tests/databases/api_tests.py | 66 +++++++++++++++++++++---
tests/datasets/api_tests.py | 57 ++++++++++++++++++--
10 files changed, 262 insertions(+), 79 deletions(-)
diff --git a/superset/charts/api.py b/superset/charts/api.py
index 602f082..bb92b3d 100644
--- a/superset/charts/api.py
+++ b/superset/charts/api.py
@@ -63,7 +63,7 @@ from superset.charts.schemas import (
screenshot_query_schema,
thumbnail_query_schema,
)
-from superset.commands.exceptions import CommandInvalidError
+from superset.commands.importers.exceptions import NoValidFilesFoundError
from superset.commands.importers.v1.utils import get_contents_from_bundle
from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
from superset.exceptions import QueryObjectValidationError
@@ -977,7 +977,6 @@ class ChartRestApi(BaseSupersetModelRestApi):
@expose("/import/", methods=["POST"])
@protect()
- @safe
@statsd_metrics
@event_logger.log_this_with_context(
action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.import_",
@@ -1029,6 +1028,9 @@ class ChartRestApi(BaseSupersetModelRestApi):
with ZipFile(upload) as bundle:
contents = get_contents_from_bundle(bundle)
+ if not contents:
+ raise NoValidFilesFoundError()
+
passwords = (
json.loads(request.form["passwords"])
if "passwords" in request.form
@@ -1039,12 +1041,5 @@ class ChartRestApi(BaseSupersetModelRestApi):
command = ImportChartsCommand(
contents, passwords=passwords, overwrite=overwrite
)
- try:
- command.run()
- return self.response(200, message="OK")
- except CommandInvalidError as exc:
- logger.warning("Import chart failed")
- return self.response_422(message=exc.normalized_messages())
- except Exception as exc: # pylint: disable=broad-except
- logger.exception("Import chart failed")
- return self.response_500(message=str(exc))
+ command.run()
+ return self.response(200, message="OK")
diff --git a/superset/commands/importers/exceptions.py b/superset/commands/importers/exceptions.py
index e79cc1c..c1beb8e 100644
--- a/superset/commands/importers/exceptions.py
+++ b/superset/commands/importers/exceptions.py
@@ -21,3 +21,8 @@ from superset.commands.exceptions import CommandException
class IncorrectVersionError(CommandException):
status = 422
message = "Import has incorrect version"
+
+
+class NoValidFilesFoundError(CommandException):
+ status = 400
+ message = "No valid import files were found"
diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py
index 14e92ed..632f7f2 100644
--- a/superset/dashboards/api.py
+++ b/superset/dashboards/api.py
@@ -32,7 +32,7 @@ from werkzeug.wsgi import FileWrapper
from superset import is_feature_enabled, thumbnail_cache
from superset.charts.schemas import ChartEntityResponseSchema
-from superset.commands.exceptions import CommandInvalidError
+from superset.commands.importers.exceptions import NoValidFilesFoundError
from superset.commands.importers.v1.utils import get_contents_from_bundle
from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
from superset.dashboards.commands.bulk_delete import BulkDeleteDashboardCommand
@@ -43,7 +43,6 @@ from superset.dashboards.commands.exceptions import (
DashboardCreateFailedError,
DashboardDeleteFailedError,
DashboardForbiddenError,
- DashboardImportError,
DashboardInvalidError,
DashboardNotFoundError,
DashboardUpdateFailedError,
@@ -888,7 +887,6 @@ class DashboardRestApi(BaseSupersetModelRestApi):
@expose("/import/", methods=["POST"])
@protect()
- @safe
@statsd_metrics
@event_logger.log_this_with_context(
action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.import_",
@@ -944,6 +942,9 @@ class DashboardRestApi(BaseSupersetModelRestApi):
upload.seek(0)
contents = {upload.filename: upload.read()}
+ if not contents:
+ raise NoValidFilesFoundError()
+
passwords = (
json.loads(request.form["passwords"])
if "passwords" in request.form
@@ -954,12 +955,5 @@ class DashboardRestApi(BaseSupersetModelRestApi):
command = ImportDashboardsCommand(
contents, passwords=passwords, overwrite=overwrite
)
- try:
- command.run()
- return self.response(200, message="OK")
- except CommandInvalidError as exc:
- logger.warning("Import dashboard failed")
- return self.response_422(message=exc.normalized_messages())
- except DashboardImportError as exc:
- logger.exception("Import dashboard failed")
- return self.response_500(message=str(exc))
+ command.run()
+ return self.response(200, message="OK")
diff --git a/superset/databases/api.py b/superset/databases/api.py
index 75a2187..d64238b 100644
--- a/superset/databases/api.py
+++ b/superset/databases/api.py
@@ -28,7 +28,7 @@ from marshmallow import ValidationError
from sqlalchemy.exc import NoSuchTableError, OperationalError, SQLAlchemyError
from superset import app, event_logger
-from superset.commands.exceptions import CommandInvalidError
+from superset.commands.importers.exceptions import NoValidFilesFoundError
from superset.commands.importers.v1.utils import get_contents_from_bundle
from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
from superset.databases.commands.create import CreateDatabaseCommand
@@ -38,7 +38,6 @@ from superset.databases.commands.exceptions import (
DatabaseCreateFailedError,
DatabaseDeleteDatasetsExistFailedError,
DatabaseDeleteFailedError,
- DatabaseImportError,
DatabaseInvalidError,
DatabaseNotFoundError,
DatabaseUpdateFailedError,
@@ -749,7 +748,6 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
@expose("/import/", methods=["POST"])
@protect()
- @safe
@statsd_metrics
@event_logger.log_this_with_context(
action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.import_",
@@ -801,6 +799,9 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
with ZipFile(upload) as bundle:
contents = get_contents_from_bundle(bundle)
+ if not contents:
+ raise NoValidFilesFoundError()
+
passwords = (
json.loads(request.form["passwords"])
if "passwords" in request.form
@@ -811,15 +812,8 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
command = ImportDatabasesCommand(
contents, passwords=passwords, overwrite=overwrite
)
- try:
- command.run()
- return self.response(200, message="OK")
- except CommandInvalidError as exc:
- logger.warning("Import database failed")
- return self.response_422(message=exc.normalized_messages())
- except DatabaseImportError as exc:
- logger.error("Import database failed", exc_info=True)
- return self.response_500(message=str(exc))
+ command.run()
+ return self.response(200, message="OK")
@expose("/<int:pk>/function_names/", methods=["GET"])
@protect()
diff --git a/superset/datasets/api.py b/superset/datasets/api.py
index 278fc0b..312369c 100644
--- a/superset/datasets/api.py
+++ b/superset/datasets/api.py
@@ -29,7 +29,7 @@ from flask_babel import ngettext
from marshmallow import ValidationError
from superset import event_logger, is_feature_enabled
-from superset.commands.exceptions import CommandInvalidError
+from superset.commands.importers.exceptions import NoValidFilesFoundError
from superset.commands.importers.v1.utils import get_contents_from_bundle
from superset.connectors.sqla.models import SqlaTable
from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
@@ -42,7 +42,6 @@ from superset.datasets.commands.exceptions import (
DatasetCreateFailedError,
DatasetDeleteFailedError,
DatasetForbiddenError,
- DatasetImportError,
DatasetInvalidError,
DatasetNotFoundError,
DatasetRefreshFailedError,
@@ -655,7 +654,6 @@ class DatasetRestApi(BaseSupersetModelRestApi):
@expose("/import/", methods=["POST"])
@protect()
- @safe
@statsd_metrics
@event_logger.log_this_with_context(
action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.import_",
@@ -711,6 +709,9 @@ class DatasetRestApi(BaseSupersetModelRestApi):
upload.seek(0)
contents = {upload.filename: upload.read()}
+ if not contents:
+ raise NoValidFilesFoundError()
+
passwords = (
json.loads(request.form["passwords"])
if "passwords" in request.form
@@ -721,12 +722,5 @@ class DatasetRestApi(BaseSupersetModelRestApi):
command = ImportDatasetsCommand(
contents, passwords=passwords, overwrite=overwrite
)
- try:
- command.run()
- return self.response(200, message="OK")
- except CommandInvalidError as exc:
- logger.warning("Import dataset failed")
- return self.response_422(message=exc.normalized_messages())
- except DatasetImportError as exc:
- logger.error("Import dataset failed", exc_info=True)
- return self.response_500(message=str(exc))
+ command.run()
+ return self.response(200, message="OK")
diff --git a/superset/queries/saved_queries/api.py b/superset/queries/saved_queries/api.py
index ee39f09..97b123e 100644
--- a/superset/queries/saved_queries/api.py
+++ b/superset/queries/saved_queries/api.py
@@ -26,7 +26,7 @@ from flask_appbuilder.api import expose, protect, rison, safe
from flask_appbuilder.models.sqla.interface import SQLAInterface
from flask_babel import ngettext
-from superset.commands.exceptions import CommandInvalidError
+from superset.commands.importers.exceptions import NoValidFilesFoundError
from superset.commands.importers.v1.utils import get_contents_from_bundle
from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
from superset.databases.filters import DatabaseFilter
@@ -263,7 +263,6 @@ class SavedQueryRestApi(BaseSupersetModelRestApi):
@expose("/import/", methods=["POST"])
@protect()
- @safe
@statsd_metrics
@event_logger.log_this_with_context(
action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.import_",
@@ -315,6 +314,9 @@ class SavedQueryRestApi(BaseSupersetModelRestApi):
with ZipFile(upload) as bundle:
contents = get_contents_from_bundle(bundle)
+ if not contents:
+ raise NoValidFilesFoundError()
+
passwords = (
json.loads(request.form["passwords"])
if "passwords" in request.form
@@ -325,12 +327,5 @@ class SavedQueryRestApi(BaseSupersetModelRestApi):
command = ImportSavedQueriesCommand(
contents, passwords=passwords, overwrite=overwrite
)
- try:
- command.run()
- return self.response(200, message="OK")
- except CommandInvalidError as exc:
- logger.warning("Import Saved Query failed")
- return self.response_422(message=exc.normalized_messages())
- except Exception as exc: # pylint: disable=broad-except
- logger.exception("Import Saved Query failed")
- return self.response_500(message=str(exc))
+ command.run()
+ return self.response(200, message="OK")
diff --git a/tests/charts/api_tests.py b/tests/charts/api_tests.py
index 33927bb..b43abfb 100644
--- a/tests/charts/api_tests.py
+++ b/tests/charts/api_tests.py
@@ -1633,9 +1633,22 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin):
assert rv.status_code == 422
assert response == {
- "message": {
- "charts/imported_chart.yaml": "Chart already exists and `overwrite=true` was not passed"
- }
+ "errors": [
+ {
+ "message": "Error importing chart",
+ "error_type": "GENERIC_COMMAND_ERROR",
+ "level": "warning",
+ "extra": {
+ "charts/imported_chart.yaml": "Chart already exists and `overwrite=true` was not passed",
+ "issue_codes": [
+ {
+ "code": 1010,
+ "message": "Issue 1010 - Superset encountered an error while running a command.",
+ }
+ ],
+ },
+ }
+ ]
}
# import with overwrite flag
@@ -1691,7 +1704,25 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin):
assert rv.status_code == 422
assert response == {
- "message": {"metadata.yaml": {"type": ["Must be equal to Slice."]}}
+ "errors": [
+ {
+ "message": "Error importing chart",
+ "error_type": "GENERIC_COMMAND_ERROR",
+ "level": "warning",
+ "extra": {
+ "metadata.yaml": {"type": ["Must be equal to Slice."]},
+ "issue_codes": [
+ {
+ "code": 1010,
+ "message": (
+ "Issue 1010 - Superset encountered an "
+ "error while running a command."
+ ),
+ }
+ ],
+ },
+ }
+ ]
}
@pytest.mark.usefixtures(
diff --git a/tests/dashboards/api_tests.py b/tests/dashboards/api_tests.py
index 02be01e..7bec82b 100644
--- a/tests/dashboards/api_tests.py
+++ b/tests/dashboards/api_tests.py
@@ -626,6 +626,14 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi
buf.seek(0)
return buf
+ def create_invalid_dashboard_import(self):
+ buf = BytesIO()
+ with ZipFile(buf, "w") as bundle:
+ with bundle.open("sql/dump.sql", "w") as fp:
+ fp.write("CREATE TABLE foo (bar INT)".encode())
+ buf.seek(0)
+ return buf
+
def test_delete_dashboard(self):
"""
Dashboard API: Test delete
@@ -1392,6 +1400,42 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi
db.session.delete(database)
db.session.commit()
+ def test_import_dashboard_invalid_file(self):
+ """
+ Dashboard API: Test import invalid dashboard file
+ """
+ self.login(username="admin")
+ uri = "api/v1/dashboard/import/"
+
+ buf = self.create_invalid_dashboard_import()
+ form_data = {
+ "formData": (buf, "dashboard_export.zip"),
+ }
+ rv = self.client.post(uri, data=form_data, content_type="multipart/form-data")
+ response = json.loads(rv.data.decode("utf-8"))
+
+ assert rv.status_code == 400
+ assert response == {
+ "errors": [
+ {
+ "message": "No valid import files were found",
+ "error_type": "GENERIC_COMMAND_ERROR",
+ "level": "warning",
+ "extra": {
+ "issue_codes": [
+ {
+ "code": 1010,
+ "message": (
+ "Issue 1010 - Superset encountered an "
+ "error while running a command."
+ ),
+ }
+ ]
+ },
+ }
+ ]
+ }
+
def test_import_dashboard_v0_export(self):
num_dashboards = db.session.query(Dashboard).count()
@@ -1449,9 +1493,25 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi
assert rv.status_code == 422
assert response == {
- "message": {
- "dashboards/imported_dashboard.yaml": "Dashboard already exists and `overwrite=true` was not passed"
- }
+ "errors": [
+ {
+ "message": "Error importing dashboard",
+ "error_type": "GENERIC_COMMAND_ERROR",
+ "level": "warning",
+ "extra": {
+ "dashboards/imported_dashboard.yaml": "Dashboard already exists and `overwrite=true` was not passed",
+ "issue_codes": [
+ {
+ "code": 1010,
+ "message": (
+ "Issue 1010 - Superset encountered an "
+ "error while running a command."
+ ),
+ }
+ ],
+ },
+ }
+ ]
}
# import with overwrite flag
@@ -1515,7 +1575,25 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi
assert rv.status_code == 422
assert response == {
- "message": {"metadata.yaml": {"type": ["Must be equal to Dashboard."]}}
+ "errors": [
+ {
+ "message": "Error importing dashboard",
+ "error_type": "GENERIC_COMMAND_ERROR",
+ "level": "warning",
+ "extra": {
+ "metadata.yaml": {"type": ["Must be equal to Dashboard."]},
+ "issue_codes": [
+ {
+ "code": 1010,
+ "message": (
+ "Issue 1010 - Superset encountered "
+ "an error while running a command."
+ ),
+ }
+ ],
+ },
+ }
+ ]
}
def test_get_all_related_roles(self):
diff --git a/tests/databases/api_tests.py b/tests/databases/api_tests.py
index 8f17fdc..09711fd 100644
--- a/tests/databases/api_tests.py
+++ b/tests/databases/api_tests.py
@@ -1208,9 +1208,25 @@ class TestDatabaseApi(SupersetTestCase):
assert rv.status_code == 422
assert response == {
- "message": {
- "databases/imported_database.yaml": "Database already exists and `overwrite=true` was not passed"
- }
+ "errors": [
+ {
+ "message": "Error importing database",
+ "error_type": "GENERIC_COMMAND_ERROR",
+ "level": "warning",
+ "extra": {
+ "databases/imported_database.yaml": "Database already exists and `overwrite=true` was not passed",
+ "issue_codes": [
+ {
+ "code": 1010,
+ "message": (
+ "Issue 1010 - Superset encountered an "
+ "error while running a command."
+ ),
+ }
+ ],
+ },
+ }
+ ]
}
# import with overwrite flag
@@ -1263,7 +1279,25 @@ class TestDatabaseApi(SupersetTestCase):
assert rv.status_code == 422
assert response == {
- "message": {"metadata.yaml": {"type": ["Must be equal to Database."]}}
+ "errors": [
+ {
+ "message": "Error importing database",
+ "error_type": "GENERIC_COMMAND_ERROR",
+ "level": "warning",
+ "extra": {
+ "metadata.yaml": {"type": ["Must be equal to Database."]},
+ "issue_codes": [
+ {
+ "code": 1010,
+ "message": (
+ "Issue 1010 - Superset encountered an "
+ "error while running a command."
+ ),
+ }
+ ],
+ },
+ }
+ ]
}
def test_import_database_masked_password(self):
@@ -1300,11 +1334,27 @@ class TestDatabaseApi(SupersetTestCase):
assert rv.status_code == 422
assert response == {
- "message": {
- "databases/imported_database.yaml": {
- "_schema": ["Must provide a password for the database"]
+ "errors": [
+ {
+ "message": "Error importing database",
+ "error_type": "GENERIC_COMMAND_ERROR",
+ "level": "warning",
+ "extra": {
+ "databases/imported_database.yaml": {
+ "_schema": ["Must provide a password for the database"]
+ },
+ "issue_codes": [
+ {
+ "code": 1010,
+ "message": (
+ "Issue 1010 - Superset encountered an "
+ "error while running a command."
+ ),
+ }
+ ],
+ },
}
- }
+ ]
}
def test_import_database_masked_password_provided(self):
diff --git a/tests/datasets/api_tests.py b/tests/datasets/api_tests.py
index b82ef75..ea0e172 100644
--- a/tests/datasets/api_tests.py
+++ b/tests/datasets/api_tests.py
@@ -1543,9 +1543,22 @@ class TestDatasetApi(SupersetTestCase):
assert rv.status_code == 422
assert response == {
- "message": {
- "datasets/imported_dataset.yaml": "Dataset already exists and `overwrite=true` was not passed"
- }
+ "errors": [
+ {
+ "message": "Error importing dataset",
+ "error_type": "GENERIC_COMMAND_ERROR",
+ "level": "warning",
+ "extra": {
+ "datasets/imported_dataset.yaml": "Dataset already exists and `overwrite=true` was not passed",
+ "issue_codes": [
+ {
+ "code": 1010,
+ "message": "Issue 1010 - Superset encountered an error while running a command.",
+ }
+ ],
+ },
+ }
+ ]
}
# import with overwrite flag
@@ -1599,7 +1612,25 @@ class TestDatasetApi(SupersetTestCase):
assert rv.status_code == 422
assert response == {
- "message": {"metadata.yaml": {"type": ["Must be equal to SqlaTable."]}}
+ "errors": [
+ {
+ "message": "Error importing dataset",
+ "error_type": "GENERIC_COMMAND_ERROR",
+ "level": "warning",
+ "extra": {
+ "metadata.yaml": {"type": ["Must be equal to SqlaTable."]},
+ "issue_codes": [
+ {
+ "code": 1010,
+ "message": (
+ "Issue 1010 - Superset encountered "
+ "an error while running a command."
+ ),
+ }
+ ],
+ },
+ }
+ ]
}
def test_import_dataset_invalid_v0_validation(self):
@@ -1628,4 +1659,20 @@ class TestDatasetApi(SupersetTestCase):
response = json.loads(rv.data.decode("utf-8"))
assert rv.status_code == 422
- assert response == {"message": "Could not process entity"}
+ assert response == {
+ "errors": [
+ {
+ "message": "Could not find a valid command to import file",
+ "error_type": "GENERIC_COMMAND_ERROR",
+ "level": "warning",
+ "extra": {
+ "issue_codes": [
+ {
+ "code": 1010,
+ "message": "Issue 1010 - Superset encountered an error while running a command.",
+ }
+ ]
+ },
+ }
+ ]
+ }