This is an automated email from the ASF dual-hosted git repository. dpgaspar pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push: new f9db3fa [mypy] Enforcing typing for superset.dashboards (#9418) f9db3fa is described below commit f9db3faadec5e651fd3872b7af21f4099359ea06 Author: Daniel Vaz Gaspar <danielvazgas...@gmail.com> AuthorDate: Tue Apr 7 12:52:14 2020 +0100 [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 | 2 +- superset/charts/commands/delete.py | 4 ++-- superset/charts/commands/update.py | 4 ++-- superset/commands/base.py | 6 ++---- superset/dashboards/api.py | 7 +++++-- superset/dashboards/commands/bulk_delete.py | 3 ++- superset/dashboards/commands/create.py | 3 ++- superset/dashboards/commands/delete.py | 3 ++- superset/dashboards/commands/exceptions.py | 2 +- superset/dashboards/commands/update.py | 7 ++++--- superset/dashboards/dao.py | 13 +++++++------ superset/dashboards/filters.py | 5 ++++- superset/dashboards/schemas.py | 7 ++++--- superset/views/base.py | 8 +++++--- 14 files changed, 43 insertions(+), 31 deletions(-) diff --git a/setup.cfg b/setup.cfg index 33e7223..bff4adf 100644 --- a/setup.cfg +++ b/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 diff --git a/superset/charts/commands/delete.py b/superset/charts/commands/delete.py index bf85d9e..c21abee 100644 --- a/superset/charts/commands/delete.py +++ b/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() diff --git a/superset/charts/commands/update.py b/superset/charts/commands/update.py index 5d9ee2a..6a110d5 100644 --- a/superset/charts/commands/update.py +++ b/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() diff --git a/superset/commands/base.py b/superset/commands/base.py index 9c6de0c..998a756 100644 --- a/superset/commands/base.py +++ b/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 diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index e960c4c..28a720d 100644 --- a/superset/dashboards/api.py +++ b/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: diff --git a/superset/dashboards/commands/bulk_delete.py b/superset/dashboards/commands/bulk_delete.py index 07f2bef..654675b 100644 --- a/superset/dashboards/commands/bulk_delete.py +++ b/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() diff --git a/superset/dashboards/commands/create.py b/superset/dashboards/commands/create.py index da79677..38658be 100644 --- a/superset/dashboards/commands/create.py +++ b/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) diff --git a/superset/dashboards/commands/delete.py b/superset/dashboards/commands/delete.py index bc19b7c..8a69532 100644 --- a/superset/dashboards/commands/delete.py +++ b/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) diff --git a/superset/dashboards/commands/exceptions.py b/superset/dashboards/commands/exceptions.py index 76d7237..13b0d5b 100644 --- a/superset/dashboards/commands/exceptions.py +++ b/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"]) diff --git a/superset/dashboards/commands/update.py b/superset/dashboards/commands/update.py index f324835..8f23daf 100644 --- a/superset/dashboards/commands/update.py +++ b/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) diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index 923a9c9..050b507 100644 --- a/superset/dashboards/dao.py +++ b/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( diff --git a/superset/dashboards/filters.py b/superset/dashboards/filters.py index 0b4338d..c72894a 100644 --- a/superset/dashboards/filters.py +++ b/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 diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index a6b7cd4..902130e 100644 --- a/superset/dashboards/schemas.py +++ b/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(" ", "-") diff --git a/superset/views/base.py b/superset/views/base.py index 595cdbd..5f40d34 100644 --- a/superset/views/base.py +++ b/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 []