You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/05/06 17:09:44 UTC
[GitHub] [incubator-superset] john-bodley commented on a change in pull request #9753: feat: convert backend chart errors to the new error type
john-bodley commented on a change in pull request #9753:
URL: https://github.com/apache/incubator-superset/pull/9753#discussion_r420945670
##########
File path: setup.cfg
##########
@@ -45,7 +45,7 @@ combine_as_imports = true
include_trailing_comma = true
line_length = 88
known_first_party = superset
-known_third_party =alembic,apispec,backoff,bleach,celery,click,colorama,contextlib2,croniter,cryptography,dataclasses,dateutil,flask,flask_appbuilder,flask_babel,flask_caching,flask_compress,flask_login,flask_migrate,flask_sqlalchemy,flask_talisman,flask_testing,flask_wtf,geohash,geopy,humanize,isodate,jinja2,markdown,markupsafe,marshmallow,msgpack,numpy,pandas,parsedatetime,pathlib2,polyline,prison,pyarrow,pyhive,pytz,retry,selenium,setuptools,simplejson,sphinx_rtd_theme,sqlalchemy,sqlalchemy_utils,sqlparse,werkzeug,wtforms,wtforms_json,yaml
+known_third_party =alembic,apispec,backoff,bleach,celery,click,colorama,contextlib2,croniter,cryptography,dateutil,flask,flask_appbuilder,flask_babel,flask_caching,flask_compress,flask_login,flask_migrate,flask_sqlalchemy,flask_talisman,flask_testing,flask_wtf,geohash,geopy,humanize,isodate,jinja2,markdown,markupsafe,marshmallow,msgpack,numpy,pandas,parsedatetime,pathlib2,polyline,prison,pyarrow,pyhive,pytz,retry,selenium,setuptools,simplejson,sphinx_rtd_theme,sqlalchemy,sqlalchemy_utils,sqlparse,werkzeug,wtforms,wtforms_json,yaml
Review comment:
`dataclasses` should be included here as it's not part of the `stdlib` in Python 3.6.
##########
File path: superset/errors.py
##########
@@ -0,0 +1,59 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from dataclasses import dataclass
+from enum import Enum
+from typing import Any, Dict
+
+
+class SupersetErrorType(str, Enum):
+ """
+ Types of errors that can exist within Superset.
+
+ Keep in sync with superset-frontend/src/components/ErrorMessage/types.ts
+ """
+
+ FRONTEND_CSRF_ERROR = "FRONTEND_CSRF_ERROR"
+ FRONTEND_NETWORK_ERROR = "FRONTEND_NETWORK_ERROR"
+ FRONTEND_TIMEOUT_ERROR = "FRONTEND_TIMEOUT_ERROR"
+
Review comment:
Nit. Nix empty lines?
##########
File path: superset-frontend/src/components/ErrorMessage/types.ts
##########
@@ -17,18 +17,22 @@
* under the License.
*/
-// TODO: Add more error types as we classify more errors
+// Keep in sync with superset/views/errors.py
export const ErrorTypeEnum = {
- // Generic errors created on the frontend
FRONTEND_CSRF_ERROR: 'FRONTEND_CSRF_ERROR',
FRONTEND_NETWORK_ERROR: 'FRONTEND_NETWORK_ERROR',
FRONTEND_TIMEOUT_ERROR: 'FRONTEND_TIMEOUT_ERROR',
+
Review comment:
Nit. Nix empty lines?
##########
File path: superset/errors.py
##########
@@ -0,0 +1,59 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from dataclasses import dataclass
+from enum import Enum
+from typing import Any, Dict
+
+
+class SupersetErrorType(str, Enum):
+ """
+ Types of errors that can exist within Superset.
+
+ Keep in sync with superset-frontend/src/components/ErrorMessage/types.ts
+ """
+
+ FRONTEND_CSRF_ERROR = "FRONTEND_CSRF_ERROR"
+ FRONTEND_NETWORK_ERROR = "FRONTEND_NETWORK_ERROR"
+ FRONTEND_TIMEOUT_ERROR = "FRONTEND_TIMEOUT_ERROR"
+
+ GENERIC_DB_ENGINE_ERROR = "GENERIC_DB_ENGINE_ERROR"
+
+ VIZ_GET_DF_ERROR = "VIZ_GET_DF_ERROR"
+
+
+class ErrorLevel(str, Enum):
+ """
+ Levels of errors that can exist within Superset.
+
+ Keep in sync with superset-frontend/src/components/ErrorMessage/types.ts
+ """
+
+ INFO = "info"
+ WARNING = "warning"
+ ERROR = "error"
+
+
+@dataclass
+class SupersetError:
+ """
+ An error that is returned to a client.
+ """
+
+ message: str
+ error_type: SupersetErrorType
+ level: ErrorLevel
+ extra: Dict[str, Any]
Review comment:
Do you want to make this optional?
##########
File path: superset/viz.py
##########
@@ -460,8 +462,19 @@ def get_df_payload(self, query_obj=None, **kwargs):
is_loaded = True
except Exception as ex:
logger.exception(ex)
- if not self.error_message:
- self.error_message = "{}".format(ex)
+
+ error = dataclasses.asdict(
+ SupersetError(
+ message="{}".format(ex),
+ level=ErrorLevel.ERROR,
+ type=SupersetErrorType.VIZ_GET_DF_ERROR,
+ extra={},
Review comment:
See note regarding making this optional.
##########
File path: superset/viz.py
##########
@@ -460,8 +462,19 @@ def get_df_payload(self, query_obj=None, **kwargs):
is_loaded = True
except Exception as ex:
logger.exception(ex)
- if not self.error_message:
- self.error_message = "{}".format(ex)
+
+ error = dataclasses.asdict(
+ SupersetError(
+ message="{}".format(ex),
Review comment:
Why not `message=str(ex)`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org