feat(api): bump marshmallow and FAB to version 3 (apache#9964)

* feat(api): bump marshmallow and FAB to version 3

* revert query context tests changes

* obey mypy

* fix tests

* ignore types that collide with marshmallow

* preparing for RC2

* fix tests for marshmallow 3

* typing fixes for marshmallow

* fix tests and black

* fix tests

* bump to RC3 and lint

* Test RC4

* Final 3.0.0

* Address comments, fix tests, better naming, docs

* fix test

* couple of fixes, addressing comments

* bumping marshmallow

requirements.txt

@@ -7,7 +7,7 @@
 aiohttp==3.6.2            # via slackclient
 alembic==1.4.2            # via flask-migrate
 amqp==2.5.2               # via kombu
-apispec[yaml]==1.3.3      # via flask-appbuilder
+apispec[yaml]==3.3.1      # via flask-appbuilder
 async-timeout==3.0.1      # via aiohttp
 attrs==19.3.0             # via aiohttp, jsonschema
 babel==2.8.0              # via flask-babel
@@ -29,7 +29,7 @@ decorator==4.4.2          # via retry
 defusedxml==0.6.0         # via python3-openid
 dnspython==1.16.0         # via email-validator
 email-validator==1.1.0    # via flask-appbuilder
-flask-appbuilder==2.3.4   # via apache-superset (setup.py)
+flask-appbuilder==3.0.0   # via apache-superset (setup.py)
 flask-babel==1.0.0        # via flask-appbuilder
 flask-caching==1.8.0      # via apache-superset (setup.py)
 flask-compress==1.5.0     # via apache-superset (setup.py)
@@ -58,7 +58,7 @@ markdown==3.2.2           # via apache-superset (setup.py)
 markupsafe==1.1.1         # via jinja2, mako, wtforms
 marshmallow-enum==1.5.1   # via flask-appbuilder
 marshmallow-sqlalchemy==0.23.0  # via flask-appbuilder
-marshmallow==2.21.0       # via flask-appbuilder, marshmallow-enum, marshmallow-sqlalchemy
+marshmallow==3.6.1        # via flask-appbuilder, marshmallow-enum, marshmallow-sqlalchemy
 msgpack==1.0.0            # via apache-superset (setup.py)
 multidict==4.7.6          # via aiohttp, yarl
 numpy==1.18.4             # via pandas, pyarrow
@@ -100,4 +100,4 @@ yarl==1.4.2               # via aiohttp
 zipp==3.1.0               # via importlib-metadata
 
 # The following packages are considered to be unsafe in a requirements file:
-# setuptools
+# setuptools

setup.py

@@ -78,7 +78,7 @@ def get_git_sha():
         "cryptography>=2.4.2",
         "dataclasses<0.7",
         "flask>=1.1.0, <2.0.0",
-        "flask-appbuilder>=2.3.4, <2.4.0",
+        "flask-appbuilder>=3.0.0, <4.0.0",
         "flask-caching",
         "flask-compress",
         "flask-talisman",

superset/charts/api.py

@@ -23,6 +23,7 @@
 from flask_appbuilder.api import expose, protect, rison, safe
 from flask_appbuilder.models.sqla.interface import SQLAInterface
 from flask_babel import gettext as _, ngettext
+from marshmallow import ValidationError
 from werkzeug.wrappers import Response as WerkzeugResponse
 from werkzeug.wsgi import FileWrapper
 
@@ -99,6 +100,8 @@ class ChartRestApi(BaseSupersetModelRestApi):
         "params",
         "cache_timeout",
     ]
+    show_select_columns = show_columns + ["table.id"]
+
     list_columns = [
         "id",
         "slice_name",
@@ -121,6 +124,7 @@ class ChartRestApi(BaseSupersetModelRestApi):
         "params",
         "cache_timeout",
     ]
+
     order_columns = [
         "slice_name",
         "viz_type",
@@ -215,13 +219,14 @@ def post(self) -> Response:
         """
         if not request.is_json:
             return self.response_400(message="Request is not JSON")
-        item = self.add_model_schema.load(request.json)
+        try:
+            item = self.add_model_schema.load(request.json)
         # This validates custom Schema with custom validations
-        if item.errors:
-            return self.response_400(message=item.errors)
+        except ValidationError as error:
+            return self.response_400(message=error.messages)
         try:
-            new_model = CreateChartCommand(g.user, item.data).run()
-            return self.response(201, id=new_model.id, result=item.data)
+            new_model = CreateChartCommand(g.user, item).run()
+            return self.response(201, id=new_model.id, result=item)
         except ChartInvalidError as ex:
             return self.response_422(message=ex.normalized_messages())
         except ChartCreateFailedError as ex:
@@ -281,13 +286,14 @@ def put(  # pylint: disable=too-many-return-statements, arguments-differ
         """
         if not request.is_json:
             return self.response_400(message="Request is not JSON")
-        item = self.edit_model_schema.load(request.json)
+        try:
+            item = self.edit_model_schema.load(request.json)
         # This validates custom Schema with custom validations
-        if item.errors:
-            return self.response_400(message=item.errors)
+        except ValidationError as error:
+            return self.response_400(message=error.messages)
         try:
-            changed_model = UpdateChartCommand(g.user, pk, item.data).run()
-            return self.response(200, id=changed_model.id, result=item.data)
+            changed_model = UpdateChartCommand(g.user, pk, item).run()
+            return self.response(200, id=changed_model.id, result=item)
         except ChartNotFoundError:
             return self.response_404()
         except ChartForbiddenError:
@@ -442,8 +448,7 @@ def data(self) -> Response:
               $ref: '#/components/responses/400'
             500:
               $ref: '#/components/responses/500'
-            """
-
+        """
         if request.is_json:
             json_body = request.json
         elif request.form.get("form_data"):
@@ -452,13 +457,13 @@ def data(self) -> Response:
         else:
             return self.response_400(message="Request is not JSON")
         try:
-            query_context, errors = ChartDataQueryContextSchema().load(json_body)
-            if errors:
-                return self.response_400(
-                    message=_("Request is incorrect: %(error)s", error=errors)
-                )
+            query_context = ChartDataQueryContextSchema().load(json_body)
         except KeyError:
             return self.response_400(message="Request is incorrect")
+        except ValidationError as error:
+            return self.response_400(
+                _("Request is incorrect: %(error)s", error=error.messages)
+            )
         try:
             query_context.raise_for_access()
         except SupersetSecurityException:

superset/charts/commands/update.py

@@ -58,7 +58,7 @@ def run(self) -> Model:
         return chart
 
     def validate(self) -> None:
-        exceptions = list()
+        exceptions: List[ValidationError] = list()
         dashboard_ids = self._properties.get("dashboards", [])
         owner_ids: Optional[List[int]] = self._properties.get("owners")
 

superset/charts/schemas.py

@@ -246,8 +246,7 @@ def __init__(self) -> None:
 
 
 class ChartDataPostProcessingOperationOptionsSchema(Schema):
-    def __init__(self, *args: Any, **kwargs: Any) -> None:
-        super().__init__(*args, **kwargs)
+    pass
 
 
 class ChartDataAggregateOptionsSchema(ChartDataPostProcessingOperationOptionsSchema):
@@ -369,7 +368,7 @@ class ChartDataSelectOptionsSchema(ChartDataPostProcessingOperationOptionsSchema
         "referenced here.",
         example=["country", "gender", "age"],
     )
-    exclude = fields.List(
+    exclude = fields.List(  # type: ignore
         fields.String(),
         description="Columns to exclude from selection.",
         example=["my_temp_column"],
@@ -676,6 +675,9 @@ class ChartDataQueryObjectSchema(Schema):
     timeseries_limit = fields.Integer(
         description="Maximum row count for timeseries queries. Default: `0`",
     )
+    timeseries_limit_metric = fields.Integer(
+        description="Metric used to limit timeseries queries by.", allow_none=True,
+    )
     row_limit = fields.Integer(
         description='Maximum row count. Default: `config["ROW_LIMIT"]`',
         validate=[
@@ -744,13 +746,13 @@ class ChartDataQueryContextSchema(Schema):
         validate=validate.OneOf(choices=("json", "csv")),
     )
 
-    # pylint: disable=no-self-use
+    # pylint: disable=no-self-use,unused-argument
     @post_load
-    def make_query_context(self, data: Dict[str, Any]) -> QueryContext:
+    def make_query_context(self, data: Dict[str, Any], **kwargs: Any) -> QueryContext:
         query_context = QueryContext(**data)
         return query_context
 
-    # pylint: enable=no-self-use
+    # pylint: enable=no-self-use,unused-argument
 
 
 class ChartDataResponseResult(Schema):

superset/commands/exceptions.py

@@ -49,7 +49,7 @@ def add_list(self, exceptions: List[ValidationError]) -> None:
     def normalized_messages(self) -> Dict[Any, Any]:
         errors: Dict[Any, Any] = {}
         for exception in self._invalid_exceptions:
-            errors.update(exception.normalized_messages())
+            errors.update(exception.normalized_messages())  # type: ignore
         return errors
 
 
@@ -77,11 +77,11 @@ class OwnersNotFoundValidationError(ValidationError):
     status = 422
 
     def __init__(self) -> None:
-        super().__init__(_("Owners are invalid"), field_names=["owners"])
+        super().__init__([_("Owners are invalid")], field_name="owners")
 
 
 class DatasourceNotFoundValidationError(ValidationError):
     status = 404
 
     def __init__(self) -> None:
-        super().__init__(_("Datasource does not exist"), field_names=["datasource_id"])
+        super().__init__([_("Datasource does not exist")], field_name="datasource_id")

superset/dashboards/api.py

@@ -21,6 +21,7 @@
 from flask_appbuilder.api import expose, protect, rison, safe
 from flask_appbuilder.models.sqla.interface import SQLAInterface
 from flask_babel import ngettext
+from marshmallow import ValidationError
 from werkzeug.wrappers import Response as WerkzeugResponse
 from werkzeug.wsgi import FileWrapper
 
@@ -118,7 +119,7 @@ class DashboardRestApi(BaseSupersetModelRestApi):
         "owners.first_name",
         "owners.last_name",
     ]
-    edit_columns = [
+    add_columns = [
         "dashboard_title",
         "slug",
         "owners",
@@ -127,9 +128,10 @@ class DashboardRestApi(BaseSupersetModelRestApi):
         "json_metadata",
         "published",
     ]
+    edit_columns = add_columns
+
     search_columns = ("dashboard_title", "slug", "owners", "published")
     search_filters = {"dashboard_title": [DashboardTitleOrSlugFilter]}
-    add_columns = edit_columns
     base_order = ("changed_on", "desc")
 
     add_model_schema = DashboardPostSchema()
@@ -197,13 +199,14 @@ def post(self) -> Response:
         """
         if not request.is_json:
             return self.response_400(message="Request is not JSON")
-        item = self.add_model_schema.load(request.json)
+        try:
+            item = self.add_model_schema.load(request.json)
         # This validates custom Schema with custom validations
-        if item.errors:
-            return self.response_400(message=item.errors)
+        except ValidationError as error:
+            return self.response_400(message=error.messages)
         try:
-            new_model = CreateDashboardCommand(g.user, item.data).run()
-            return self.response(201, id=new_model.id, result=item.data)
+            new_model = CreateDashboardCommand(g.user, item).run()
+            return self.response(201, id=new_model.id, result=item)
         except DashboardInvalidError as ex:
             return self.response_422(message=ex.normalized_messages())
         except DashboardCreateFailedError as ex:
@@ -263,13 +266,14 @@ def put(  # pylint: disable=too-many-return-statements, arguments-differ
         """
         if not request.is_json:
             return self.response_400(message="Request is not JSON")
-        item = self.edit_model_schema.load(request.json)
+        try:
+            item = self.edit_model_schema.load(request.json)
         # This validates custom Schema with custom validations
-        if item.errors:
-            return self.response_400(message=item.errors)
+        except ValidationError as error:
+            return self.response_400(message=error.messages)
         try:
-            changed_model = UpdateDashboardCommand(g.user, pk, item.data).run()
-            return self.response(200, id=changed_model.id, result=item.data)
+            changed_model = UpdateDashboardCommand(g.user, pk, item).run()
+            return self.response(200, id=changed_model.id, result=item)
         except DashboardNotFoundError:
             return self.response_404()
         except DashboardForbiddenError:

superset/dashboards/commands/create.py

@@ -50,7 +50,7 @@ def run(self) -> Model:
         return dashboard
 
     def validate(self) -> None:
-        exceptions = list()
+        exceptions: List[ValidationError] = list()
         owner_ids: Optional[List[int]] = self._properties.get("owners")
         slug: str = self._properties.get("slug", "")
 

superset/dashboards/commands/exceptions.py

@@ -33,7 +33,7 @@ class DashboardSlugExistsValidationError(ValidationError):
     """
 
     def __init__(self) -> None:
-        super().__init__(_("Must be unique"), field_names=["slug"])
+        super().__init__([_("Must be unique")], field_name="slug")
 
 
 class DashboardInvalidError(CommandInvalidError):

superset/dashboards/schemas.py

@@ -18,7 +18,7 @@
 import re
 from typing import Any, Dict, Union
 
-from marshmallow import fields, pre_load, Schema
+from marshmallow import fields, post_load, Schema
 from marshmallow.validate import Length, ValidationError
 
 from superset.exceptions import SupersetException
@@ -92,7 +92,7 @@ def validate_json_metadata(value: Union[bytes, bytearray, str]) -> None:
         value_obj = json.loads(value)
     except json.decoder.JSONDecodeError:
         raise ValidationError("JSON not valid")
-    errors = DashboardJSONMetadataSchema(strict=True).validate(value_obj, partial=False)
+    errors = DashboardJSONMetadataSchema().validate(value_obj, partial=False)
     if errors:
         raise ValidationError(errors)
 
@@ -110,12 +110,16 @@ class DashboardJSONMetadataSchema(Schema):
 
 
 class BaseDashboardSchema(Schema):
-    @pre_load
-    def pre_load(self, data: Dict[str, Any]) -> None:  # pylint: disable=no-self-use
+    # pylint: disable=no-self-use,unused-argument
+    @post_load
+    def post_load(self, data: Dict[str, Any], **kwargs: Any) -> Dict[str, Any]:
         if data.get("slug"):
             data["slug"] = data["slug"].strip()
             data["slug"] = data["slug"].replace(" ", "-")
             data["slug"] = re.sub(r"[^\w\-]+", "", data["slug"])
+        return data
+
+    # pylint: disable=no-self-use,unused-argument
 
 
 class DashboardPostSchema(BaseDashboardSchema):
@@ -133,7 +137,7 @@ class DashboardPostSchema(BaseDashboardSchema):
     )
     css = fields.String()
     json_metadata = fields.String(
-        description=json_metadata_description, validate=validate_json_metadata
+        description=json_metadata_description, validate=validate_json_metadata,
     )
     published = fields.Boolean(description=published_description)