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

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

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



##########
File path: superset-frontend/src/explore/components/controls/DatasourceControl.jsx
##########
@@ -164,16 +169,22 @@ class DatasourceControl extends React.PureComponent {
       </Menu>
     );
 
-    // eslint-disable-next-line camelcase
     const { health_check_message: healthCheckMessage } = datasource;
 
     return (
       <Styles className="DatasourceControl">
         <div className="data-container">
           <Icon name="dataset-physical" className="dataset-svg" />
-          <Tooltip title={datasource.name}>
-            <span className="title-select">{datasource.name}</span>
-          </Tooltip>
+          {/* Add a tooltip only for long dataset names */}
+          {datasource.name.length > 20 ? (

Review comment:
       Could be nice to have this in a `constants.ts`

##########
File path: superset-frontend/src/explore/components/controls/DatasourceControl.jsx
##########
@@ -196,6 +207,35 @@ class DatasourceControl extends React.PureComponent {
             </Tooltip>
           </Dropdown>
         </div>
+        {/* missing dataset */}
+        {datasource.id == null && (
+          <div className="error-alert">
+            <ErrorAlert
+              level="warning"
+              title={t('Missing dataset')}
+              source="explore"
+              subtitle={
+                <>
+                  <p>
+                    {t(
+                      'The dataset linked to this chart may have been deleted.',

Review comment:
       Could we be more explicit here as in the other error - "Dataset does not exist".

##########
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."""

Review comment:
       nit:
   ```suggestion
           not exist."""
   ```

##########
File path: superset/models/slice.py
##########
@@ -206,7 +206,7 @@ def digest(self) -> str:
         """
         Returns a MD5 HEX digest that makes this dashboard unique
         """
-        return utils.md5_hex(self.params)
+        return utils.md5_hex(self.params or "")

Review comment:
       Not that it matters, but I've always been allergic to md5 on empty string:
   ```
   >>> import hashlib
   >>> hashlib.md5(''.encode()).hexdigest()
   'd41d8cd98f00b204e9800998ecf8427e'
   ```
   
   ```suggestion
           return utils.md5_hex(self.params) if self.params else ''
   ```

##########
File path: superset/datasets/commands/exceptions.py
##########
@@ -54,7 +57,7 @@ class DatasetExistsValidationError(ValidationError):
 
     def __init__(self, table_name: str) -> None:
         super().__init__(
-            get_datasource_exist_error_msg(table_name), field_name="table_name"
+            [get_dataset_exist_error_msg(table_name)], field_name="table_name"

Review comment:
       Nice catch 👍 




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