etr2460 commented on a change in pull request #10274: URL: https://github.com/apache/incubator-superset/pull/10274#discussion_r452522716
########## File path: superset-frontend/src/components/ErrorMessage/constants.ts ########## @@ -0,0 +1,31 @@ +/** + * 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 { t } from '@superset-ui/translation'; + +const ROOT_ERROR_REFERENCE_URL = + 'https://superset.apache.org/error_code_reference.html#'; + +export const ERROR_DATASOURCE_TOO_LARGE = { + message: t('Error 1000 - The datasource is too large to query.'), + link: `${ROOT_ERROR_REFERENCE_URL}error-1000`, +}; Review comment: So there are a couple things being conflated here I think. First off, an idea of API error codes. These are already consistently defined and described on both the backend and the frontend here: https://github.com/apache/incubator-superset/blob/master/superset/errors.py#L24 and here: https://github.com/apache/incubator-superset/blob/master/superset-frontend/src/components/ErrorMessage/types.ts#L21 Part of the problem is that each of these errors from a software perspective could have multiple possible root causes and mitigations from the user's perspective. In this case, a timeout error can have multiple different root causes and mitigations, and these root causes/mitigations could apply to many different software errors too. We could add another field onto the error payload coming from the backend to include root causes and mitigations, but I also want to ensure we have freedom on the frontend to render these errors in whatever rich format we see fit. Thus why the software's idea of errors exists on the backend, but the user interpretations live in the frontend. Finally, as to making the errors more structural: I agree this can be defined better, I'll restructure the TS object. However, I'm a bit anxious about investing a bunch of time into autogenning docs from it since we just voted to change doc systems a couple days ago. It might be better to wait for the doc migration, then to build an autogenned doc page with error details ---------------------------------------------------------------- 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: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
