You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2023/09/14 21:11:09 UTC
[superset] branch sc_71594 updated (047f9157fe -> 2d030002e0)
This is an automated email from the ASF dual-hosted git repository.
beto pushed a change to branch sc_71594
in repository https://gitbox.apache.org/repos/asf/superset.git
omit 047f9157fe feat: generic marshmallow error component
new 2d030002e0 feat: generic marshmallow error component
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (047f9157fe)
\
N -- N -- N refs/heads/sc_71594 (2d030002e0)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
superset/datasets/schemas.py | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
[superset] 01/01: feat: generic marshmallow error component
Posted by be...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch sc_71594
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 2d030002e07c4ba247d4bfaee450d5cb7e04cbb8
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Tue Sep 5 12:13:18 2023 -0700
feat: generic marshmallow error component
---
.../src/components/Datasource/DatasourceModal.tsx | 13 ++-
.../ErrorMessage/MarshmallowErrorMessage.tsx | 109 +++++++++++++++++++++
.../src/components/ErrorMessage/types.ts | 3 +-
.../src/components/FilterableTable/index.tsx | 30 +-----
superset-frontend/src/hooks/useJsonTreeTheme.ts | 41 ++++++++
superset-frontend/src/setup/setupErrorMessages.ts | 5 +
superset/datasets/api.py | 1 -
superset/datasets/schemas.py | 12 +++
superset/errors.py | 3 +
superset/exceptions.py | 17 ++++
10 files changed, 201 insertions(+), 33 deletions(-)
diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.tsx b/superset-frontend/src/components/Datasource/DatasourceModal.tsx
index f9c40c47ba..fde88ee280 100644
--- a/superset-frontend/src/components/Datasource/DatasourceModal.tsx
+++ b/superset-frontend/src/components/Datasource/DatasourceModal.tsx
@@ -31,7 +31,7 @@ import {
import Modal from 'src/components/Modal';
import AsyncEsmComponent from 'src/components/AsyncEsmComponent';
-import { getClientErrorObject } from 'src/utils/getClientErrorObject';
+import ErrorMessageWithStackTrace from 'src/components/ErrorMessage/ErrorMessageWithStackTrace';
import withToasts from 'src/components/MessageToasts/withToasts';
import { useSelector } from 'react-redux';
@@ -202,11 +202,16 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
})
.catch(response => {
setIsSaving(false);
- getClientErrorObject(response).then(({ error }) => {
+ response.json().then(errorJson => {
modal.error({
- title: t('Error'),
- content: error || t('An error has occurred'),
+ title: t('Error saving dataset'),
okButtonProps: { danger: true, className: 'btn-danger' },
+ content: (
+ <ErrorMessageWithStackTrace
+ error={errorJson.errors[0]}
+ source="crud"
+ />
+ ),
});
});
});
diff --git a/superset-frontend/src/components/ErrorMessage/MarshmallowErrorMessage.tsx b/superset-frontend/src/components/ErrorMessage/MarshmallowErrorMessage.tsx
new file mode 100644
index 0000000000..0718840121
--- /dev/null
+++ b/superset-frontend/src/components/ErrorMessage/MarshmallowErrorMessage.tsx
@@ -0,0 +1,109 @@
+/**
+ * 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.
+ */
+import React from 'react';
+import { JSONTree } from 'react-json-tree';
+import { css, styled, t } from '@superset-ui/core';
+
+import { useJsonTreeTheme } from 'src/hooks/useJsonTreeTheme';
+import Collapse from 'src/components/Collapse';
+import { ErrorMessageComponentProps } from './types';
+
+interface MarshmallowErrorExtra {
+ messages: object;
+ payload: object;
+ issue_codes: {
+ code: number;
+ message: string;
+ }[];
+}
+
+const StyledUl = styled.ul`
+ padding-left: ${({ theme }) => theme.gridUnit * 5}px;
+ padding-top: ${({ theme }) => theme.gridUnit * 4}px;
+`;
+
+const collapseStyle = theme => css`
+ .ant-collapse-arrow {
+ left: 0px !important;
+ }
+ .ant-collapse-header {
+ padding-left: ${theme.gridUnit * 4}px !important;
+ }
+ .ant-collapse-content-box {
+ padding: 0px !important;
+ }
+`;
+
+const extractInvalidValues = (messages: object, payload: object): string[] => {
+ const invalidValues: string[] = [];
+
+ const recursiveExtract = (messages: object, payload: object) => {
+ Object.keys(messages).forEach(key => {
+ const value = payload[key];
+ const message = messages[key];
+
+ if (Array.isArray(message)) {
+ message.forEach(errorMessage => {
+ invalidValues.push(`${errorMessage}: ${value}`);
+ });
+ } else {
+ recursiveExtract(message, value);
+ }
+ });
+ };
+ recursiveExtract(messages, payload);
+ return invalidValues;
+};
+
+export default function MarshmallowErrorMessage({
+ error,
+ // eslint-disable-next-line @typescript-eslint/no-unused-vars
+ source = 'crud',
+ subtitle,
+}: ErrorMessageComponentProps<MarshmallowErrorExtra>) {
+ const jsonTreeTheme = useJsonTreeTheme();
+ const { extra, message } = error;
+
+ return (
+ <div>
+ {subtitle && <h4>{subtitle}</h4>}
+
+ {message}
+
+ <StyledUl>
+ {extractInvalidValues(extra.messages, extra.payload).map(
+ (value, index) => (
+ <li key={index}>{value}</li>
+ ),
+ )}
+ </StyledUl>
+
+ <Collapse ghost css={collapseStyle}>
+ <Collapse.Panel header={t('Details')} key="details" css={collapseStyle}>
+ <JSONTree
+ data={extra.messages}
+ shouldExpandNode={() => true}
+ hideRoot
+ theme={jsonTreeTheme}
+ />
+ </Collapse.Panel>
+ </Collapse>
+ </div>
+ );
+}
diff --git a/superset-frontend/src/components/ErrorMessage/types.ts b/superset-frontend/src/components/ErrorMessage/types.ts
index d3fe5bfdf7..7c4c3fe94a 100644
--- a/superset-frontend/src/components/ErrorMessage/types.ts
+++ b/superset-frontend/src/components/ErrorMessage/types.ts
@@ -79,6 +79,7 @@ export const ErrorTypeEnum = {
// API errors
INVALID_PAYLOAD_FORMAT_ERROR: 'INVALID_PAYLOAD_FORMAT_ERROR',
INVALID_PAYLOAD_SCHEMA_ERROR: 'INVALID_PAYLOAD_SCHEMA_ERROR',
+ MARSHMALLOW_ERROR: 'MARSHMALLOW_ERROR',
} as const;
type ValueOf<T> = T[keyof T];
@@ -88,7 +89,7 @@ export type ErrorType = ValueOf<typeof ErrorTypeEnum>;
// Keep in sync with superset/views/errors.py
export type ErrorLevel = 'info' | 'warning' | 'error';
-export type ErrorSource = 'dashboard' | 'explore' | 'sqllab';
+export type ErrorSource = 'dashboard' | 'explore' | 'sqllab' | 'crud';
export type SupersetError<ExtraType = Record<string, any> | null> = {
error_type: ErrorType;
diff --git a/superset-frontend/src/components/FilterableTable/index.tsx b/superset-frontend/src/components/FilterableTable/index.tsx
index 91fc1f4477..8e7d54ef9f 100644
--- a/superset-frontend/src/components/FilterableTable/index.tsx
+++ b/superset-frontend/src/components/FilterableTable/index.tsx
@@ -27,6 +27,7 @@ import {
useTheme,
} from '@superset-ui/core';
import { useDebounceValue } from 'src/hooks/useDebounceValue';
+import { useJsonTreeTheme } from 'src/hooks/useJsonTreeTheme';
import Button from '../Button';
import CopyToClipboard from '../CopyToClipboard';
import ModalTrigger from '../ModalTrigger';
@@ -183,32 +184,7 @@ const FilterableTable = ({
return complexColumns[columnKey] ? truncated : content;
};
- const theme = useTheme();
- const [jsonTreeTheme, setJsonTreeTheme] = useState<Record<string, string>>();
-
- const getJsonTreeTheme = () => {
- if (!jsonTreeTheme) {
- setJsonTreeTheme({
- base00: theme.colors.grayscale.dark2,
- base01: theme.colors.grayscale.dark1,
- base02: theme.colors.grayscale.base,
- base03: theme.colors.grayscale.light1,
- base04: theme.colors.grayscale.light2,
- base05: theme.colors.grayscale.light3,
- base06: theme.colors.grayscale.light4,
- base07: theme.colors.grayscale.light5,
- base08: theme.colors.error.base,
- base09: theme.colors.error.light1,
- base0A: theme.colors.error.light2,
- base0B: theme.colors.success.base,
- base0C: theme.colors.primary.light1,
- base0D: theme.colors.primary.base,
- base0E: theme.colors.primary.dark1,
- base0F: theme.colors.error.dark1,
- });
- }
- return jsonTreeTheme;
- };
+ const jsonTreeTheme = useJsonTreeTheme();
const getWidthsForColumns = () => {
const PADDING = 50; // accounts for cell padding and width of sorting icon
@@ -293,7 +269,7 @@ const FilterableTable = ({
modalBody={
<JSONTree
data={jsonObject}
- theme={getJsonTreeTheme()}
+ theme={jsonTreeTheme}
valueRenderer={renderBigIntStrToNumber}
/>
}
diff --git a/superset-frontend/src/hooks/useJsonTreeTheme.ts b/superset-frontend/src/hooks/useJsonTreeTheme.ts
new file mode 100644
index 0000000000..a052e22a63
--- /dev/null
+++ b/superset-frontend/src/hooks/useJsonTreeTheme.ts
@@ -0,0 +1,41 @@
+/**
+ * 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.
+ */
+import { useTheme } from '@superset-ui/core';
+
+export const useJsonTreeTheme = () => {
+ const theme = useTheme();
+ return {
+ base00: theme.colors.grayscale.dark2,
+ base01: theme.colors.grayscale.dark1,
+ base02: theme.colors.grayscale.base,
+ base03: theme.colors.grayscale.light1,
+ base04: theme.colors.grayscale.light2,
+ base05: theme.colors.grayscale.light3,
+ base06: theme.colors.grayscale.light4,
+ base07: theme.colors.grayscale.light5,
+ base08: theme.colors.error.base,
+ base09: theme.colors.error.light1,
+ base0A: theme.colors.error.light2,
+ base0B: theme.colors.success.base,
+ base0C: theme.colors.primary.light1,
+ base0D: theme.colors.primary.base,
+ base0E: theme.colors.primary.dark1,
+ base0F: theme.colors.error.dark1,
+ };
+};
diff --git a/superset-frontend/src/setup/setupErrorMessages.ts b/superset-frontend/src/setup/setupErrorMessages.ts
index bebb22b20c..59842f190a 100644
--- a/superset-frontend/src/setup/setupErrorMessages.ts
+++ b/superset-frontend/src/setup/setupErrorMessages.ts
@@ -20,6 +20,7 @@ import getErrorMessageComponentRegistry from 'src/components/ErrorMessage/getErr
import { ErrorTypeEnum } from 'src/components/ErrorMessage/types';
import TimeoutErrorMessage from 'src/components/ErrorMessage/TimeoutErrorMessage';
import DatabaseErrorMessage from 'src/components/ErrorMessage/DatabaseErrorMessage';
+import MarshmallowErrorMessage from 'src/components/ErrorMessage/MarshmallowErrorMessage';
import ParameterErrorMessage from 'src/components/ErrorMessage/ParameterErrorMessage';
import DatasetNotFoundErrorMessage from 'src/components/ErrorMessage/DatasetNotFoundErrorMessage';
@@ -144,5 +145,9 @@ export default function setupErrorMessages() {
ErrorTypeEnum.FAILED_FETCHING_DATASOURCE_INFO_ERROR,
DatasetNotFoundErrorMessage,
);
+ errorMessageComponentRegistry.registerValue(
+ ErrorTypeEnum.MARSHMALLOW_ERROR,
+ MarshmallowErrorMessage,
+ );
setupErrorMessagesExtra();
}
diff --git a/superset/datasets/api.py b/superset/datasets/api.py
index 9bbb6acd21..d68e1523bf 100644
--- a/superset/datasets/api.py
+++ b/superset/datasets/api.py
@@ -330,7 +330,6 @@ class DatasetRestApi(BaseSupersetModelRestApi):
@expose("/<pk>", methods=("PUT",))
@protect()
- @safe
@statsd_metrics
@event_logger.log_this_with_context(
action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.put",
diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py
index 66d80e1842..2040fb9b15 100644
--- a/superset/datasets/schemas.py
+++ b/superset/datasets/schemas.py
@@ -24,6 +24,7 @@ from marshmallow.validate import Length
from marshmallow_sqlalchemy import SQLAlchemyAutoSchema
from superset.datasets.models import Dataset
+from superset.exceptions import SupersetMarshmallowValidationError
get_delete_ids_schema = {"type": "array", "items": {"type": "integer"}}
get_export_ids_schema = {"type": "array", "items": {"type": "integer"}}
@@ -123,6 +124,17 @@ class DatasetPutSchema(Schema):
is_managed_externally = fields.Boolean(allow_none=True, dump_default=False)
external_url = fields.String(allow_none=True)
+ def handle_error(
+ self,
+ error: ValidationError,
+ data: dict[str, Any],
+ **kwargs: Any,
+ ) -> None:
+ """
+ Return SIP-40 error.
+ """
+ raise SupersetMarshmallowValidationError(error, data)
+
class DatasetDuplicateSchema(Schema):
base_model_id = fields.Integer(required=True)
diff --git a/superset/errors.py b/superset/errors.py
index 167ec2d3b4..b0d1d6faff 100644
--- a/superset/errors.py
+++ b/superset/errors.py
@@ -90,6 +90,7 @@ class SupersetErrorType(StrEnum):
# API errors
INVALID_PAYLOAD_FORMAT_ERROR = "INVALID_PAYLOAD_FORMAT_ERROR"
INVALID_PAYLOAD_SCHEMA_ERROR = "INVALID_PAYLOAD_SCHEMA_ERROR"
+ MARSHMALLOW_ERROR = "MARSHMALLOW_ERROR"
# Report errors
REPORT_NOTIFICATION_ERROR = "REPORT_NOTIFICATION_ERROR"
@@ -144,6 +145,7 @@ ISSUE_CODES = {
1035: _("Failed to start remote query on a worker."),
1036: _("The database was deleted."),
1037: _("Custom SQL fields cannot contain sub-queries."),
+ 1040: _("The submitted payload failed validation."),
}
@@ -181,6 +183,7 @@ ERROR_TYPES_TO_ISSUE_CODES_MAPPING = {
SupersetErrorType.ASYNC_WORKERS_ERROR: [1035],
SupersetErrorType.DATABASE_NOT_FOUND_ERROR: [1011, 1036],
SupersetErrorType.CONNECTION_DATABASE_TIMEOUT: [1001, 1009],
+ SupersetErrorType.MARSHMALLOW_ERROR: [1040],
}
diff --git a/superset/exceptions.py b/superset/exceptions.py
index 5f1df73c86..3642a9279e 100644
--- a/superset/exceptions.py
+++ b/superset/exceptions.py
@@ -278,3 +278,20 @@ class QueryNotFoundException(SupersetException):
class ColumnNotFoundException(SupersetException):
status = 404
+
+
+class SupersetMarshmallowValidationError(SupersetErrorException):
+ """
+ Exception to be raised for Marshmallow validation errors.
+ """
+
+ status = 422
+
+ def __init__(self, exc: ValidationError, payload: dict[str, Any]):
+ error = SupersetError(
+ message=_("The schema of the submitted payload is invalid."),
+ error_type=SupersetErrorType.MARSHMALLOW_ERROR,
+ level=ErrorLevel.ERROR,
+ extra={"messages": exc.messages, "payload": payload},
+ )
+ super().__init__(error)