You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by su...@apache.org on 2020/08/10 23:31:43 UTC

[incubator-superset] 06/06: more work on making it work, + feature flag

This is an automated email from the ASF dual-hosted git repository.

suddjian pushed a commit to branch dynamic-plugin-import
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git

commit 42f97e728ffaeafebfc352288bb204b3fc58e40e
Author: David Aaron Suddjian <aa...@gmail.com>
AuthorDate: Fri Jul 10 13:56:47 2020 -0700

    more work on making it work, + feature flag
---
 .../src/components/AlteredSliceTag.jsx             | 25 ++++++----------
 .../DynamicPlugins/DynamicPluginProvider.tsx       |  8 +++--
 .../explore/components/ControlPanelsContainer.jsx  |  4 +--
 superset-frontend/src/featureFlags.ts              |  1 +
 .../src/utils/getControlsForVizType.js             | 35 ++++++++++++----------
 superset/app.py                                    | 17 ++++++-----
 superset/config.py                                 |  1 +
 7 files changed, 47 insertions(+), 44 deletions(-)

diff --git a/superset-frontend/src/components/AlteredSliceTag.jsx b/superset-frontend/src/components/AlteredSliceTag.jsx
index d158461..509d0f1 100644
--- a/superset-frontend/src/components/AlteredSliceTag.jsx
+++ b/superset-frontend/src/components/AlteredSliceTag.jsx
@@ -53,9 +53,7 @@ export default class AlteredSliceTag extends React.Component {
     super(props);
     const diffs = this.getDiffs(props);
 
-    const controlsMap = getControlsForVizType(this.props.origFormData.viz_type);
-
-    this.state = { diffs, hasDiffs: !isEmpty(diffs), controlsMap };
+    this.state = { diffs, hasDiffs: !isEmpty(diffs) };
   }
 
   UNSAFE_componentWillReceiveProps(newProps) {
@@ -96,6 +94,7 @@ export default class AlteredSliceTag extends React.Component {
   }
 
   formatValue(value, key) {
+    const controlsMap = getControlsForVizType(this.props.origFormData.viz_type);
     // Format display value based on the control type
     // or the value type
     if (value === undefined) {
@@ -103,8 +102,8 @@ export default class AlteredSliceTag extends React.Component {
     } else if (value === null) {
       return 'null';
     } else if (
-      this.state.controlsMap[key] &&
-      this.state.controlsMap[key].type === 'AdhocFilterControl'
+      controlsMap[key] &&
+      controlsMap[key].type === 'AdhocFilterControl'
     ) {
       if (!value.length) {
         return '[]';
@@ -118,14 +117,11 @@ export default class AlteredSliceTag extends React.Component {
           return `${v.subject} ${v.operator} ${filterVal}`;
         })
         .join(', ');
-    } else if (
-      this.state.controlsMap[key] &&
-      this.state.controlsMap[key].type === 'BoundsControl'
-    ) {
+    } else if (controlsMap[key] && controlsMap[key].type === 'BoundsControl') {
       return `Min: ${value[0]}, Max: ${value[1]}`;
     } else if (
-      this.state.controlsMap[key] &&
-      this.state.controlsMap[key].type === 'CollectionControl'
+      controlsMap[key] &&
+      controlsMap[key].type === 'CollectionControl'
     ) {
       return value.map(v => safeStringify(v)).join(', ');
     } else if (typeof value === 'boolean') {
@@ -139,6 +135,7 @@ export default class AlteredSliceTag extends React.Component {
   }
 
   renderRows() {
+    const controlsMap = getControlsForVizType(this.props.origFormData.viz_type);
     const diffs = this.state.diffs;
     const rows = [];
     for (const key in diffs) {
@@ -146,11 +143,7 @@ export default class AlteredSliceTag extends React.Component {
         <Tr key={key}>
           <Td
             column="control"
-            data={
-              (this.state.controlsMap[key] &&
-                this.state.controlsMap[key].label) ||
-              key
-            }
+            data={(controlsMap[key] && controlsMap[key].label) || key}
           />
           <Td column="before">{this.formatValue(diffs[key].before, key)}</Td>
           <Td column="after">{this.formatValue(diffs[key].after, key)}</Td>
diff --git a/superset-frontend/src/components/DynamicPlugins/DynamicPluginProvider.tsx b/superset-frontend/src/components/DynamicPlugins/DynamicPluginProvider.tsx
index b14f4bd..476c1f9 100644
--- a/superset-frontend/src/components/DynamicPlugins/DynamicPluginProvider.tsx
+++ b/superset-frontend/src/components/DynamicPlugins/DynamicPluginProvider.tsx
@@ -1,5 +1,6 @@
 import React, { useEffect, useReducer } from 'react';
 import { SupersetClient, Json } from '@superset-ui/connection';
+import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
 import {
   PluginContext,
   PluginContextType,
@@ -96,6 +97,7 @@ export default function DynamicPluginProvider({ children }: Props) {
     ...dummyPluginContext,
     // eslint-disable-next-line @typescript-eslint/no-use-before-define
     fetchAll,
+    loading: isFeatureEnabled(FeatureFlag.DYNAMIC_PLUGINS),
     // TODO: Write fetchByKeys
   });
 
@@ -130,7 +132,7 @@ export default function DynamicPluginProvider({ children }: Props) {
             // eslint-disable-next-line no-console
             console.error(
               `Failed to load plugin ${plugin.key} with the following error:`,
-              err,
+              err.stack,
             );
             error = err;
           }
@@ -148,7 +150,9 @@ export default function DynamicPluginProvider({ children }: Props) {
   }
 
   useEffect(() => {
-    fetchAll();
+    if (isFeatureEnabled(FeatureFlag.DYNAMIC_PLUGINS)) {
+      fetchAll();
+    }
   }, []);
 
   return (
diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.jsx b/superset-frontend/src/explore/components/ControlPanelsContainer.jsx
index 6d610e4..2cd3269 100644
--- a/superset-frontend/src/explore/components/ControlPanelsContainer.jsx
+++ b/superset-frontend/src/explore/components/ControlPanelsContainer.jsx
@@ -171,9 +171,9 @@ class ControlPanelsContainer extends React.Component {
   }
 
   render() {
-    const cpRegistry = getChartControlPanelRegistry();
+    const controlPanelRegistry = getChartControlPanelRegistry();
     if (
-      !cpRegistry.has(this.props.form_data.viz_type) &&
+      !controlPanelRegistry.has(this.props.form_data.viz_type) &&
       this.context.loading
     ) {
       // TODO replace with a snazzy loading spinner
diff --git a/superset-frontend/src/featureFlags.ts b/superset-frontend/src/featureFlags.ts
index 0853578..8cb177a 100644
--- a/superset-frontend/src/featureFlags.ts
+++ b/superset-frontend/src/featureFlags.ts
@@ -26,6 +26,7 @@ export enum FeatureFlag {
   ESTIMATE_QUERY_COST = 'ESTIMATE_QUERY_COST',
   SHARE_QUERIES_VIA_KV_STORE = 'SHARE_QUERIES_VIA_KV_STORE',
   SQLLAB_BACKEND_PERSISTENCE = 'SQLLAB_BACKEND_PERSISTENCE',
+  DYNAMIC_PLUGINS = 'DYNAMIC_PLUGINS',
 }
 
 export type FeatureFlagMap = {
diff --git a/superset-frontend/src/utils/getControlsForVizType.js b/superset-frontend/src/utils/getControlsForVizType.js
index 3be5b7e..fd0abc1 100644
--- a/superset-frontend/src/utils/getControlsForVizType.js
+++ b/superset-frontend/src/utils/getControlsForVizType.js
@@ -21,26 +21,29 @@ import memoize from 'lodash/memoize';
 import { getChartControlPanelRegistry } from '@superset-ui/chart';
 import controls from '../explore/controls';
 
-const getControlsForVizType = memoize(vizType => {
+const memoizedControls = memoize((vizType, controlPanel) => {
   const controlsMap = {};
-  getChartControlPanelRegistry()
-    .get(vizType)
-    .controlPanelSections.forEach(section => {
-      section.controlSetRows.forEach(row => {
-        row.forEach(control => {
-          if (!control) return;
-          if (typeof control === 'string') {
-            // For now, we have to look in controls.jsx to get the config for some controls.
-            // Once everything is migrated out, delete this if statement.
-            controlsMap[control] = controls[control];
-          } else if (control.name && control.config) {
-            // condition needed because there are elements, e.g. <hr /> in some control configs (I'm looking at you, FilterBox!)
-            controlsMap[control.name] = control.config;
-          }
-        });
+  controlPanel.controlPanelSections.forEach(section => {
+    section.controlSetRows.forEach(row => {
+      row.forEach(control => {
+        if (!control) return;
+        if (typeof control === 'string') {
+          // For now, we have to look in controls.jsx to get the config for some controls.
+          // Once everything is migrated out, delete this if statement.
+          controlsMap[control] = controls[control];
+        } else if (control.name && control.config) {
+          // condition needed because there are elements, e.g. <hr /> in some control configs (I'm looking at you, FilterBox!)
+          controlsMap[control.name] = control.config;
+        }
       });
     });
+  });
   return controlsMap;
 });
 
+const getControlsForVizType = vizType => {
+  const controlPanel = getChartControlPanelRegistry().get(vizType);
+  return memoizedControls(vizType, controlPanel);
+};
+
 export default getControlsForVizType;
diff --git a/superset/app.py b/superset/app.py
index 5800001..295cd1a 100644
--- a/superset/app.py
+++ b/superset/app.py
@@ -239,14 +239,15 @@ class SupersetAppInitializer:
             category="",
             category_icon="",
         )
-        appbuilder.add_view(
-            DynamicPluginsView,
-            "Custom Plugins",
-            label=__("Custom Plugins"),
-            category="Manage",
-            category_label=__("Manage"),
-            icon="fa-puzzle-piece",
-        )
+        if feature_flag_manager.is_feature_enabled("DYNAMIC_PLUGINS"):
+            appbuilder.add_view(
+                DynamicPluginsView,
+                "Custom Plugins",
+                label=__("Custom Plugins"),
+                category="Manage",
+                category_label=__("Manage"),
+                icon="fa-puzzle-piece",
+            )
         appbuilder.add_view(
             CssTemplateModelView,
             "CSS Templates",
diff --git a/superset/config.py b/superset/config.py
index cfef8c2..bcce955 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -308,6 +308,7 @@ DEFAULT_FEATURE_FLAGS: Dict[str, bool] = {
     "SIP_38_VIZ_REARCHITECTURE": False,
     "TAGGING_SYSTEM": False,
     "SQLLAB_BACKEND_PERSISTENCE": False,
+    "DYNAMIC_PLUGINS": False,
 }
 
 # This is merely a default.