From 4ce457e0dbbcd8822c32ddfb115305a2a846038f Mon Sep 17 00:00:00 2001 From: sebastianliebscher Date: Fri, 10 Nov 2023 09:39:42 +0100 Subject: [PATCH 1/2] chore: Simplify views/base --- superset/views/base.py | 67 +++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 40 deletions(-) diff --git a/superset/views/base.py b/superset/views/base.py index 4015b7a028aa6..b578c239700de 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -14,6 +14,8 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from __future__ import annotations + import dataclasses import functools import logging @@ -21,7 +23,7 @@ import traceback from datetime import datetime from importlib.resources import files -from typing import Any, Callable, cast, Optional, Union +from typing import Any, Callable, cast import simplejson as json import yaml @@ -139,15 +141,11 @@ def get_error_msg() -> str: def json_error_response( - msg: Optional[str] = None, + msg: str | None = None, status: int = 500, - payload: Optional[dict[str, Any]] = None, - link: Optional[str] = None, + payload: dict[str, Any] | None = None, ) -> FlaskResponse: - if not payload: - payload = {"error": f"{msg}"} - if link: - payload["link"] = link + payload = payload or {"error": f"{msg}"} return Response( json.dumps(payload, default=utils.json_iso_dttm_ser, ignore_nan=True), @@ -159,10 +157,9 @@ def json_error_response( def json_errors_response( errors: list[SupersetError], status: int = 500, - payload: Optional[dict[str, Any]] = None, + payload: dict[str, Any] | None = None, ) -> FlaskResponse: - if not payload: - payload = {} + payload = payload or {} payload["errors"] = [dataclasses.asdict(error) for error in errors] return Response( @@ -182,7 +179,7 @@ def data_payload_response(payload_json: str, has_error: bool = False) -> FlaskRe def generate_download_headers( - extension: str, filename: Optional[str] = None + extension: str, filename: str | None = 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}" @@ -192,7 +189,7 @@ def generate_download_headers( def deprecated( eol_version: str = "4.0.0", - new_target: Optional[str] = None, + new_target: str | None = None, ) -> Callable[[Callable[..., FlaskResponse]], Callable[..., FlaskResponse]]: """ A decorator to set an API endpoint from SupersetView has deprecated. @@ -200,7 +197,7 @@ def deprecated( """ def _deprecated(f: Callable[..., FlaskResponse]) -> Callable[..., FlaskResponse]: - def wraps(self: "BaseSupersetView", *args: Any, **kwargs: Any) -> FlaskResponse: + def wraps(self: BaseSupersetView, *args: Any, **kwargs: Any) -> FlaskResponse: message = ( "%s.%s " "This API endpoint is deprecated and will be removed in version %s" @@ -227,7 +224,7 @@ def api(f: Callable[..., FlaskResponse]) -> Callable[..., FlaskResponse]: return the response in the JSON format """ - def wraps(self: "BaseSupersetView", *args: Any, **kwargs: Any) -> FlaskResponse: + def wraps(self: BaseSupersetView, *args: Any, **kwargs: Any) -> FlaskResponse: try: return f(self, *args, **kwargs) except NoAuthorizationError: @@ -249,7 +246,7 @@ def handle_api_exception( exceptions. """ - def wraps(self: "BaseSupersetView", *args: Any, **kwargs: Any) -> FlaskResponse: + def wraps(self: BaseSupersetView, *args: Any, **kwargs: Any) -> FlaskResponse: try: return f(self, *args, **kwargs) except SupersetSecurityException as ex: @@ -294,7 +291,7 @@ def json_response(obj: Any, status: int = 200) -> FlaskResponse: ) def render_app_template( - self, extra_bootstrap_data: Optional[dict[str, Any]] = None + self, extra_bootstrap_data: dict[str, Any] | None = None ) -> FlaskResponse: payload = { "user": bootstrap_user_data(g.user, include_perms=True), @@ -335,21 +332,16 @@ def get_environment_tag() -> dict[str, Any]: def menu_data(user: User) -> dict[str, Any]: - menu = appbuilder.menu.get_data() + languages = { + lang: {**appbuilder.languages[lang], "url": appbuilder.get_url_for_locale(lang)} + for lang in appbuilder.languages + } - languages = {} - for lang in appbuilder.languages: - languages[lang] = { - **appbuilder.languages[lang], - "url": appbuilder.get_url_for_locale(lang), - } - brand_text = appbuilder.app.config["LOGO_RIGHT_TEXT"] - if callable(brand_text): + if callable(brand_text := appbuilder.app.config["LOGO_RIGHT_TEXT"]): brand_text = brand_text() - build_number = appbuilder.app.config["BUILD_NUMBER"] return { - "menu": menu, + "menu": appbuilder.menu.get_data(), "brand": { "path": appbuilder.app.config["LOGO_TARGET_PATH"] or "/superset/welcome/", "icon": appbuilder.app_icon, @@ -369,9 +361,9 @@ def menu_data(user: User) -> dict[str, Any]: "documentation_text": appbuilder.app.config["DOCUMENTATION_TEXT"], "version_string": appbuilder.app.config["VERSION_STRING"], "version_sha": appbuilder.app.config["VERSION_SHA"], - "build_number": build_number, + "build_number": appbuilder.app.config["BUILD_NUMBER"], "languages": languages, - "show_language_picker": len(languages.keys()) > 1, + "show_language_picker": len(languages) > 1, "user_is_anonymous": user.is_anonymous, "user_info_url": None if is_feature_enabled("MENU_HIDE_USER_INFO") @@ -595,11 +587,11 @@ class YamlExportMixin: # pylint: disable=too-few-public-methods Used on DatabaseView for cli compatibility """ - yaml_dict_key: Optional[str] = None + yaml_dict_key: str | None = None @action("yaml_export", __("Export to YAML"), __("Export to YAML?"), "fa-download") def yaml_export( - self, items: Union[ImportExportMixin, list[ImportExportMixin]] + self, items: ImportExportMixin | list[ImportExportMixin] ) -> FlaskResponse: if not isinstance(items, list): items = [items] @@ -682,18 +674,14 @@ def apply(self, query: Query, value: Any) -> Query: class CsvResponse(Response): - """ - Override Response to take into account csv encoding from config.py - """ + """Override Response to take into account csv encoding from config.py""" charset = conf["CSV_EXPORT"].get("encoding", "utf-8") default_mimetype = "text/csv" class XlsxResponse(Response): - """ - Override Response to use xlsx mimetype - """ + """Override Response to use xlsx mimetype""" charset = "utf-8" default_mimetype = ( @@ -704,8 +692,7 @@ class XlsxResponse(Response): def bind_field( _: Any, form: DynamicForm, unbound_field: UnboundField, options: dict[Any, Any] ) -> Field: - """ - Customize how fields are bound by stripping all whitespace. + """Customize how fields are bound by stripping all whitespace. :param form: The form :param unbound_field: The unbound field From 2019876856cc5a3d47849acd1debeb1dbdd3b9bb Mon Sep 17 00:00:00 2001 From: sebastianliebscher Date: Sat, 11 Nov 2023 09:21:47 +0100 Subject: [PATCH 2/2] address comment about docstrings --- superset/views/base.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/superset/views/base.py b/superset/views/base.py index b578c239700de..62e4dd06cfad3 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -674,14 +674,18 @@ def apply(self, query: Query, value: Any) -> Query: class CsvResponse(Response): - """Override Response to take into account csv encoding from config.py""" + """ + Override Response to take into account csv encoding from config.py + """ charset = conf["CSV_EXPORT"].get("encoding", "utf-8") default_mimetype = "text/csv" class XlsxResponse(Response): - """Override Response to use xlsx mimetype""" + """ + Override Response to use xlsx mimetype + """ charset = "utf-8" default_mimetype = ( @@ -692,7 +696,8 @@ class XlsxResponse(Response): def bind_field( _: Any, form: DynamicForm, unbound_field: UnboundField, options: dict[Any, Any] ) -> Field: - """Customize how fields are bound by stripping all whitespace. + """ + Customize how fields are bound by stripping all whitespace. :param form: The form :param unbound_field: The unbound field