[GitHub] [incubator-superset] etr2460 commented on a change in pull request #11970: feat: add hook for dataset health check

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

etr2460 commented on a change in pull request #11970:
URL: https://github.com/apache/incubator-superset/pull/11970#discussion_r543780336



##########
File path: superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx
##########
@@ -91,4 +93,10 @@ describe('DatasourceControl', () => {
     );
     expect(menuWrapper.find(Menu.Item)).toHaveLength(2);
   });
+
+  it('should render health check message', () => {
+    const wrapper = setup();
+    const icons = wrapper.find(Icon);
+    expect(icons.first().prop('name')).toBe('alert-solid');

Review comment:
       should we also make sure the warning message is shown here?

##########
File path: superset-frontend/src/explore/components/controls/DatasourceControl.jsx
##########
@@ -213,10 +213,13 @@ class DatasourceControl extends React.PureComponent {
       </Menu>
     );
 
+    // eslint-disable-next-line camelcase
+    const { health_check_message: healthCheckMessage } = datasource;
+
     return (
       <Styles className="DatasourceControl">
         <ControlHeader {...this.props} />
-        <div>
+        <div style={{ display: 'flex' }}>

Review comment:
       instead of an inline style, should we use a `styled.div` to apply this style?

##########
File path: superset/config.py
##########
@@ -991,3 +991,8 @@ class CeleryConfig:  # pylint: disable=too-few-public-methods
     except Exception:
         logger.exception("Found but failed to import local superset_config")
         raise
+
+
+# It's possible to add a dataset health check logic which is specific to your system.
+# It will get executed each time when user open a chart's explore view.
+DATASET_HEALTH_CHECK = None

Review comment:
       can you add the signature about the health check to the comment here?
   
   Or maybe even better, include a default function that always passes. It's actually a bit unclear to me how this should be used (should I return True if the dataset passes? None?)




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