[mypy] Enforcing typing for superset.dashboards (#9418)

* [mypy] Enforcing typing for superset.dashboards

* Make return types equal on all commands

* Make return types equal on all commands

* [dashboard] address comments same return type on commands

* lint

* lint

setup.cfg

@@ -53,7 +53,7 @@ order_by_type = false
 ignore_missing_imports = true
 no_implicit_optional = true
 
-[mypy-superset.bin.*,superset.charts.*,superset.commands.*,superset.common.*,superset.dao.*,superset.db_engine_specs.*,superset.db_engines.*,superset.examples.*]
+[mypy-superset.bin.*,superset.charts.*,superset.dashboards.*,superset.commands.*,superset.common.*,superset.dao.*,superset.db_engine_specs.*,superset.db_engines.*,superset.examples.*]
 check_untyped_defs = true
 disallow_untyped_calls = true
 disallow_untyped_defs = true

superset/charts/commands/delete.py

@@ -27,9 +27,9 @@ from superset.charts.commands.exceptions import (
 )
 from superset.charts.dao import ChartDAO
 from superset.commands.base import BaseCommand
-from superset.connectors.sqla.models import SqlaTable
 from superset.dao.exceptions import DAODeleteFailedError
 from superset.exceptions import SupersetSecurityException
+from superset.models.slice import Slice
 from superset.views.base import check_ownership
 
 logger = logging.getLogger(__name__)
@@ -39,7 +39,7 @@ class DeleteChartCommand(BaseCommand):
     def __init__(self, user: User, model_id: int):
         self._actor = user
         self._model_id = model_id
-        self._model: Optional[SqlaTable] = None
+        self._model: Optional[Slice] = None
 
     def run(self) -> Model:
         self.validate()

superset/charts/commands/update.py

@@ -32,10 +32,10 @@ from superset.charts.commands.exceptions import (
 from superset.charts.dao import ChartDAO
 from superset.commands.base import BaseCommand
 from superset.commands.utils import get_datasource_by_id, populate_owners
-from superset.connectors.sqla.models import SqlaTable
 from superset.dao.exceptions import DAOUpdateFailedError
 from superset.dashboards.dao import DashboardDAO
 from superset.exceptions import SupersetSecurityException
+from superset.models.slice import Slice
 from superset.views.base import check_ownership
 
 logger = logging.getLogger(__name__)
@@ -46,7 +46,7 @@ class UpdateChartCommand(BaseCommand):
         self._actor = user
         self._model_id = model_id
         self._properties = data.copy()
-        self._model: Optional[SqlaTable] = None
+        self._model: Optional[Slice] = None
 
     def run(self) -> Model:
         self.validate()

superset/commands/base.py

@@ -15,9 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 from abc import ABC, abstractmethod
-from typing import Optional
-
-from flask_appbuilder.models.sqla import Model
+from typing import Any
 
 
 class BaseCommand(ABC):
@@ -26,7 +24,7 @@ class BaseCommand(ABC):
     """
 
     @abstractmethod
-    def run(self) -> Optional[Model]:
+    def run(self) -> Any:
         """
         Run executes the command. Can raise command exceptions
         :raises: CommandException

superset/dashboards/api.py

@@ -15,6 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 import logging
+from typing import Any
 
 from flask import g, make_response, request, Response
 from flask_appbuilder.api import expose, protect, rison, safe
@@ -285,7 +286,9 @@ class DashboardRestApi(BaseSupersetModelRestApi):
     @protect()
     @safe
     @rison(get_delete_ids_schema)
-    def bulk_delete(self, **kwargs) -> Response:  # pylint: disable=arguments-differ
+    def bulk_delete(
+        self, **kwargs: Any
+    ) -> Response:  # pylint: disable=arguments-differ
         """Delete bulk Dashboards
         ---
         delete:
@@ -343,7 +346,7 @@ class DashboardRestApi(BaseSupersetModelRestApi):
     @protect()
     @safe
     @rison(get_export_ids_schema)
-    def export(self, **kwargs):
+    def export(self, **kwargs: Any) -> Response:
         """Export dashboards
         ---
         get:

superset/dashboards/commands/bulk_delete.py

@@ -40,10 +40,11 @@ class BulkDeleteDashboardCommand(BaseCommand):
         self._model_ids = model_ids
         self._models: Optional[List[Dashboard]] = None
 
-    def run(self):
+    def run(self) -> None:
         self.validate()
         try:
             DashboardDAO.bulk_delete(self._models)
+            return None
         except DeleteFailedError as e:
             logger.exception(e.exception)
             raise DashboardBulkDeleteFailedError()

superset/dashboards/commands/create.py

@@ -17,6 +17,7 @@
 import logging
 from typing import Dict, List, Optional
 
+from flask_appbuilder.models.sqla import Model
 from flask_appbuilder.security.sqla.models import User
 from marshmallow import ValidationError
 
@@ -38,7 +39,7 @@ class CreateDashboardCommand(BaseCommand):
         self._actor = user
         self._properties = data.copy()
 
-    def run(self):
+    def run(self) -> Model:
         self.validate()
         try:
             dashboard = DashboardDAO.create(self._properties)

superset/dashboards/commands/delete.py

@@ -17,6 +17,7 @@
 import logging
 from typing import Optional
 
+from flask_appbuilder.models.sqla import Model
 from flask_appbuilder.security.sqla.models import User
 
 from superset.commands.base import BaseCommand
@@ -40,7 +41,7 @@ class DeleteDashboardCommand(BaseCommand):
         self._model_id = model_id
         self._model: Optional[Dashboard] = None
 
-    def run(self):
+    def run(self) -> Model:
         self.validate()
         try:
             dashboard = DashboardDAO.delete(self._model)

superset/dashboards/commands/exceptions.py

@@ -32,7 +32,7 @@ class DashboardSlugExistsValidationError(ValidationError):
     Marshmallow validation error for dashboard slug already exists
     """
 
-    def __init__(self):
+    def __init__(self) -> None:
         super().__init__(_("Must be unique"), field_names=["slug"])
 
 

superset/dashboards/commands/update.py

@@ -17,12 +17,12 @@
 import logging
 from typing import Dict, List, Optional
 
+from flask_appbuilder.models.sqla import Model
 from flask_appbuilder.security.sqla.models import User
 from marshmallow import ValidationError
 
 from superset.commands.base import BaseCommand
 from superset.commands.utils import populate_owners
-from superset.connectors.sqla.models import SqlaTable
 from superset.dao.exceptions import DAOUpdateFailedError
 from superset.dashboards.commands.exceptions import (
     DashboardForbiddenError,
@@ -33,6 +33,7 @@ from superset.dashboards.commands.exceptions import (
 )
 from superset.dashboards.dao import DashboardDAO
 from superset.exceptions import SupersetSecurityException
+from superset.models.dashboard import Dashboard
 from superset.views.base import check_ownership
 
 logger = logging.getLogger(__name__)
@@ -43,9 +44,9 @@ class UpdateDashboardCommand(BaseCommand):
         self._actor = user
         self._model_id = model_id
         self._properties = data.copy()
-        self._model: Optional[SqlaTable] = None
+        self._model: Optional[Dashboard] = None
 
-    def run(self):
+    def run(self) -> Model:
         self.validate()
         try:
             dashboard = DashboardDAO.update(self._model, self._properties)

superset/dashboards/dao.py

@@ -48,13 +48,14 @@ class DashboardDAO(BaseDAO):
         return True
 
     @staticmethod
-    def bulk_delete(models: List[Dashboard], commit=True):
-        item_ids = [model.id for model in models]
+    def bulk_delete(models: Optional[List[Dashboard]], commit: bool = True) -> None:
+        item_ids = [model.id for model in models] if models else []
         # bulk delete, first delete related data
-        for model in models:
-            model.slices = []
-            model.owners = []
-            db.session.merge(model)
+        if models:
+            for model in models:
+                model.slices = []
+                model.owners = []
+                db.session.merge(model)
         # bulk delete itself
         try:
             db.session.query(Dashboard).filter(Dashboard.id.in_(item_ids)).delete(

superset/dashboards/filters.py

@@ -14,7 +14,10 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+from typing import Any
+
 from sqlalchemy import and_, or_
+from sqlalchemy.orm.query import Query
 
 from superset import db, security_manager
 from superset.models.core import FavStar
@@ -35,7 +38,7 @@ class DashboardFilter(BaseFilter):  # pylint: disable=too-few-public-methods
     if they wish to see those dashboards which are published first
     """
 
-    def apply(self, query, value):
+    def apply(self, query: Query, value: Any) -> Query:
         user_roles = [role.name.lower() for role in list(get_user_roles())]
         if "admin" in user_roles:
             return query

superset/dashboards/schemas.py

@@ -16,6 +16,7 @@
 # under the License.
 import json
 import re
+from typing import Any, Dict, Union
 
 from marshmallow import fields, pre_load, Schema
 from marshmallow.validate import Length, ValidationError
@@ -27,14 +28,14 @@ get_delete_ids_schema = {"type": "array", "items": {"type": "integer"}}
 get_export_ids_schema = {"type": "array", "items": {"type": "integer"}}
 
 
-def validate_json(value):
+def validate_json(value: Union[bytes, bytearray, str]) -> None:
     try:
         utils.validate_json(value)
     except SupersetException:
         raise ValidationError("JSON not valid")
 
 
-def validate_json_metadata(value):
+def validate_json_metadata(value: Union[bytes, bytearray, str]) -> None:
     if not value:
         return
     try:
@@ -60,7 +61,7 @@ class DashboardJSONMetadataSchema(Schema):
 
 class BaseDashboardSchema(Schema):
     @pre_load
-    def pre_load(self, data):  # pylint: disable=no-self-use
+    def pre_load(self, data: Dict[str, Any]) -> None:  # pylint: disable=no-self-use
         if data.get("slug"):
             data["slug"] = data["slug"].strip()
             data["slug"] = data["slug"].replace(" ", "-")

superset/views/base.py

@@ -27,7 +27,7 @@ from flask_appbuilder import BaseView, ModelView
 from flask_appbuilder.actions import action
 from flask_appbuilder.forms import DynamicForm
 from flask_appbuilder.models.sqla.filters import BaseFilter
-from flask_appbuilder.security.sqla.models import User
+from flask_appbuilder.security.sqla.models import Role, User
 from flask_appbuilder.widgets import ListWidget
 from flask_babel import get_locale, gettext as __, lazy_gettext as _
 from flask_wtf.form import FlaskForm
@@ -89,7 +89,9 @@ def data_payload_response(payload_json, has_error=False):
     return json_success(payload_json, status=status)
 
 
-def generate_download_headers(extension, filename=None):
+def generate_download_headers(
+    extension: str, filename: Optional[str] = None
+) -> Dict[str, Any]:
     filename = filename if filename else datetime.now().strftime("%Y%m%d_%H%M%S")
     content_disp = f"attachment; filename={filename}.{extension}"
     headers = {"Content-Disposition": content_disp}
@@ -146,7 +148,7 @@ def get_datasource_exist_error_msg(full_name):
     return __("Datasource %(name)s already exists", name=full_name)
 
 
-def get_user_roles():
+def get_user_roles() -> List[Role]:
     if g.user.is_anonymous:
         public_role = conf.get("AUTH_ROLE_PUBLIC")
         return [security_manager.find_role(public_role)] if public_role else []