[GitHub] [superset] ktmud commented on a change in pull request #12705: feat(explore): allow opening charts with missing dataset

[GitBox <gi...@apache.org>]

ktmud commented on a change in pull request #12705:
URL: https://github.com/apache/superset/pull/12705#discussion_r563001847



##########
File path: superset-frontend/src/common/components/index.tsx
##########
@@ -28,6 +28,7 @@ import { DropDownProps } from 'antd/lib/dropdown';
  */
 // eslint-disable-next-line no-restricted-imports
 export {
+  Alert,

Review comment:
       Wanted to use Antd Alert, but it looks inconsistent with the error box in chart container.

##########
File path: superset/connectors/connector_registry.py
##########
@@ -44,12 +46,23 @@ def register_sources(cls, datasource_config: "OrderedDict[str, List[str]]") -> N
     def get_datasource(
         cls, datasource_type: str, datasource_id: int, session: Session
     ) -> "BaseDatasource":
-        return (
+        """Safely get a datasource without raising any errors.
+        Returns None if `datasource_type` is not registered or `datasource_id` does
+        not exists."""
+        if datasource_type not in cls.sources:
+            raise DatasetNotFoundError()

Review comment:
       `handle_api_exception` will be able to render the error properly.

##########
File path: superset-frontend/src/explore/components/DatasourcePanel.tsx
##########
@@ -17,57 +17,25 @@
  * under the License.
  */
 import React, { useEffect, useState } from 'react';
-import { styled, t, QueryFormData } from '@superset-ui/core';
+import { styled, t } from '@superset-ui/core';
 import { Collapse } from 'src/common/components';
 import {
   ColumnOption,
   MetricOption,
-  ControlType,
+  ControlConfig,
+  DatasourceMeta,
 } from '@superset-ui/chart-controls';
 import { debounce } from 'lodash';
 import { matchSorter, rankings } from 'match-sorter';
 import { ExploreActions } from '../actions/exploreActions';
 import Control from './Control';
 
-interface DatasourceControl {
-  validationErrors: Array<any>;
-  mapStateToProps: QueryFormData;
-  type: ControlType;
-  label: string;
-  datasource?: DatasourceControl;
+interface DatasourceControl extends ControlConfig {
+  datasource?: DatasourceMeta;

Review comment:
       Bycatch: reuse typing from `@superset-ui/chart-controls`.

##########
File path: superset/views/datasource.py
##########
@@ -58,9 +58,7 @@ def save(self) -> FlaskResponse:
             try:
                 check_ownership(orm_datasource)
             except SupersetSecurityException:
-                return json_error_response(
-                    f"{DatasetForbiddenError.message}", DatasetForbiddenError.status
-                )
+                raise DatasetForbiddenError()

Review comment:
       `handle_api_exception` will be able to render the error properly.

##########
File path: superset-frontend/src/explore/components/DatasourcePanel.tsx
##########
@@ -279,6 +240,13 @@ export default function DataSourcePanel({
           </Collapse.Panel>
         </Collapse>
       </div>
+    </>
+  );
+
+  return (
+    <DatasourceContainer>
+      <Control {...datasourceControl} name="datasource" actions={actions} />
+      {datasource.id != null && mainBody}

Review comment:
       Datasource ID is `null` when it is unavailable.

##########
File path: superset/views/core.py
##########
@@ -771,25 +770,31 @@ def explore(  # pylint: disable=too-many-locals,too-many-return-statements
                 status=400,
             )
 
-        if action in ("saveas", "overwrite"):
+        if datasource_id and action in ("saveas", "overwrite"):
             return self.save_or_overwrite_slice(
                 slc,
                 slice_add_perm,
                 slice_overwrite_perm,
                 slice_download_perm,
                 datasource_id,
                 cast(str, datasource_type),
-                datasource.name,
+                datasource_name,
             )
 
         standalone = (
             request.args.get(utils.ReservedUrlParameters.STANDALONE.value) == "true"
         )
+        dummy_datasource_data: Dict[str, Any] = {
+            "type": datasource_type,
+            "name": datasource_name,
+            "columns": [],
+            "metrics": [],
+        }

Review comment:
       Adding a dummy datasource is easier than fixing countless places on the frontend where it's expected to exist.

##########
File path: superset/utils/core.py
##########
@@ -504,6 +505,8 @@ def base_json_conv(  # pylint: disable=inconsistent-return-statements,too-many-r
             return obj.decode("utf-8")
         except Exception:  # pylint: disable=broad-except
             return "[bytes]"
+    if isinstance(obj, LazyString):
+        return str(obj)

Review comment:
       Make sure translations can be properly outputted in json API response.




----------------------------------------------------------------
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