feat: feature flag configurable custom backend (#16618)

* feat: feature flag configurable custom backend

* fix lint

* simpler approach

* fix tests

* revert dependency updates

* Update superset/utils/feature_flag_manager.py

Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>

* Update superset/config.py

Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>

Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
(cherry picked from commit f2bc139e35)

superset-frontend/spec/javascripts/sqllab/actions/sqlLab_spec.js

@@ -613,7 +613,7 @@ describe('async actions', () => {
 
     describe('queryEditorSetSql', () => {
       describe('with backend persistence flag on', () => {
-        it('does not update the tab state in the backend', () => {
+        it('updates the tab state in the backend', () => {
           expect.assertions(2);
 
           const sql = 'SELECT * ';
@@ -629,7 +629,7 @@ describe('async actions', () => {
       });
     });
     describe('with backend persistence flag off', () => {
-      it('updates the tab state in the backend', () => {
+      it('does not update the tab state in the backend', () => {
         const backendPersistenceOffMock = jest
           .spyOn(featureFlags, 'isFeatureEnabled')
           .mockImplementation(

superset-frontend/src/SqlLab/actions/sqlLab.js

@@ -949,6 +949,11 @@ export function queryEditorSetQueryLimit(queryEditor, queryLimit) {
 
 export function queryEditorSetTemplateParams(queryEditor, templateParams) {
   return function (dispatch) {
+    dispatch({
+      type: QUERY_EDITOR_SET_TEMPLATE_PARAMS,
+      queryEditor,
+      templateParams,
+    });
     const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE)
       ? SupersetClient.put({
           endpoint: encodeURI(`/tabstateview/${queryEditor.id}`),
@@ -956,24 +961,16 @@ export function queryEditorSetTemplateParams(queryEditor, templateParams) {
         })
       : Promise.resolve();
 
-    return sync
+    return sync.catch(() =>
-      .then(() =>
+      dispatch(
-        dispatch({
+        addDangerToast(
-          type: QUERY_EDITOR_SET_TEMPLATE_PARAMS,
+          t(
-          queryEditor,
+            'An error occurred while setting the tab template parameters. ' +
-          templateParams,
+              'Please contact your administrator.',
-        }),
-      )
-      .catch(() =>
-        dispatch(
-          addDangerToast(
-            t(
-              'An error occurred while setting the tab template parameters. ' +
-                'Please contact your administrator.',
-            ),
           ),
         ),
-      );
+      ),
+    );
   };
 }
 

superset-frontend/src/explore/components/DatasourcePanel/types.ts

@@ -25,3 +25,13 @@ export interface DatasourcePanelDndItem {
   value: DndItemValue;
   type: DndItemType;
 }
+
+export function isDatasourcePanelDndItem(
+  item: any,
+): item is DatasourcePanelDndItem {
+  return item?.value && item?.type;
+}
+
+export function isSavedMetric(item: any): item is Metric {
+  return item?.metric_name;
+}

superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx

@@ -45,10 +45,12 @@ import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetr
 import {
   DatasourcePanelDndItem,
   DndItemValue,
+  isSavedMetric,
 } from 'src/explore/components/DatasourcePanel/types';
 import { DndItemType } from 'src/explore/components/DndItemType';
 import { ControlComponentProps } from 'src/explore/components/Control';
 
+const EMPTY_OBJECT = {};
 const DND_ACCEPTED_TYPES = [
   DndItemType.Column,
   DndItemType.Metric,
@@ -78,7 +80,9 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {
   );
   const [partitionColumn, setPartitionColumn] = useState(undefined);
   const [newFilterPopoverVisible, setNewFilterPopoverVisible] = useState(false);
-  const [droppedItem, setDroppedItem] = useState<DndItemValue | null>(null);
+  const [droppedItem, setDroppedItem] = useState<
+    DndItemValue | typeof EMPTY_OBJECT
+  >({});
 
   const optionsForSelect = (
     columns: ColumnMeta[],
@@ -342,12 +346,12 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {
   );
 
   const handleClickGhostButton = useCallback(() => {
-    setDroppedItem(null);
+    setDroppedItem({});
     togglePopover(true);
   }, [togglePopover]);
 
   const adhocFilter = useMemo(() => {
-    if (droppedItem?.metric_name) {
+    if (isSavedMetric(droppedItem)) {
       return new AdhocFilter({
         expressionType: EXPRESSION_TYPES.SQL,
         clause: CLAUSES.HAVING,

superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx

@@ -34,7 +34,10 @@ import { usePrevious } from 'src/common/hooks/usePrevious';
 import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric';
 import AdhocMetricPopoverTrigger from 'src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger';
 import MetricDefinitionValue from 'src/explore/components/controls/MetricControl/MetricDefinitionValue';
-import { DatasourcePanelDndItem } from 'src/explore/components/DatasourcePanel/types';
+import {
+  DatasourcePanelDndItem,
+  isDatasourcePanelDndItem,
+} from 'src/explore/components/DatasourcePanel/types';
 import { DndItemType } from 'src/explore/components/DndItemType';
 import DndSelectLabel from 'src/explore/components/controls/DndColumnSelectControl/DndSelectLabel';
 import { savedMetricType } from 'src/explore/components/controls/MetricControl/types';
@@ -143,9 +146,9 @@ export const DndMetricSelect = (props: any) => {
   const [value, setValue] = useState<ValueType[]>(
     coerceAdhocMetrics(props.value),
   );
-  const [droppedItem, setDroppedItem] = useState<DatasourcePanelDndItem | null>(
+  const [droppedItem, setDroppedItem] = useState<
-    null,
+    DatasourcePanelDndItem | typeof EMPTY_OBJECT
-  );
+  >({});
   const [newMetricPopoverVisible, setNewMetricPopoverVisible] = useState(false);
   const prevColumns = usePrevious(columns);
   const prevSavedMetrics = usePrevious(savedMetrics);
@@ -323,13 +326,16 @@ export const DndMetricSelect = (props: any) => {
   );
 
   const handleClickGhostButton = useCallback(() => {
-    setDroppedItem(null);
+    setDroppedItem({});
     togglePopover(true);
   }, [togglePopover]);
 
   const adhocMetric = useMemo(() => {
-    if (droppedItem?.type === DndItemType.Column) {
+    if (
-      const itemValue = droppedItem?.value as ColumnMeta;
+      isDatasourcePanelDndItem(droppedItem) &&
+      droppedItem.type === DndItemType.Column
+    ) {
+      const itemValue = droppedItem.value as ColumnMeta;
       const config: Partial<AdhocMetric> = {
         column: { column_name: itemValue?.column_name },
       };

superset/config.py

@@ -427,7 +427,14 @@ FEATURE_FLAGS: Dict[str, bool] = {}
 #         feature_flags_dict['some_feature'] = g.user and g.user.get_id() == 5
 #     return feature_flags_dict
 GET_FEATURE_FLAGS_FUNC: Optional[Callable[[Dict[str, bool]], Dict[str, bool]]] = None
-
+# A function that receives a feature flag name and an optional default value.
+# Has a similar utility to GET_FEATURE_FLAGS_FUNC but it's useful to not force the
+# evaluation of all feature flags when just evaluating a single one.
+#
+# Note that the default `get_feature_flags` will evaluate each feature with this
+# callable when the config key is set, so don't use both GET_FEATURE_FLAGS_FUNC
+# and IS_FEATURE_ENABLED_FUNC in conjunction.
+IS_FEATURE_ENABLED_FUNC: Optional[Callable[[str, Optional[bool]], bool]] = None
 # A function that expands/overrides the frontend `bootstrap_data.common` object.
 # Can be used to implement custom frontend functionality,
 # or dynamically change certain configs.

superset/utils/feature_flag_manager.py

@@ -24,24 +24,36 @@ class FeatureFlagManager:
     def __init__(self) -> None:
         super().__init__()
         self._get_feature_flags_func = None
+        self._is_feature_enabled_func = None
         self._feature_flags: Dict[str, Any] = {}
 
     def init_app(self, app: Flask) -> None:
         self._get_feature_flags_func = app.config["GET_FEATURE_FLAGS_FUNC"]
+        self._is_feature_enabled_func = app.config["IS_FEATURE_ENABLED_FUNC"]
         self._feature_flags = app.config["DEFAULT_FEATURE_FLAGS"]
         self._feature_flags.update(app.config["FEATURE_FLAGS"])
 
     def get_feature_flags(self) -> Dict[str, Any]:
         if self._get_feature_flags_func:
             return self._get_feature_flags_func(deepcopy(self._feature_flags))
-
+        if callable(self._is_feature_enabled_func):
+            return dict(
+                map(
+                    lambda kv: (kv[0], self._is_feature_enabled_func(kv[0], kv[1])),
+                    self._feature_flags.items(),
+                )
+            )
         return self._feature_flags
 
     def is_feature_enabled(self, feature: str) -> bool:
         """Utility function for checking whether a feature is turned on"""
+        if self._is_feature_enabled_func:
+            return (
+                self._is_feature_enabled_func(feature, self._feature_flags[feature])
+                if feature in self._feature_flags
+                else False
+            )
         feature_flags = self.get_feature_flags()
-
         if feature_flags and feature in feature_flags:
             return feature_flags[feature]
-
         return False

tests/integration_tests/feature_flag_tests.py

@@ -16,10 +16,16 @@
 # under the License.
 from unittest.mock import patch
 
-from superset import is_feature_enabled
+from parameterized import parameterized
+
+from superset import get_feature_flags, is_feature_enabled
 from tests.integration_tests.base_tests import SupersetTestCase
 
 
+def dummy_is_feature_enabled(feature_flag_name: str, default: bool = True) -> bool:
+    return True if feature_flag_name.startswith("True_") else default
+
+
 class TestFeatureFlag(SupersetTestCase):
     @patch.dict(
         "superset.extensions.feature_flag_manager._feature_flags",
@@ -38,3 +44,40 @@ class TestFeatureFlag(SupersetTestCase):
     def test_feature_flags(self):
         self.assertEqual(is_feature_enabled("foo"), "bar")
         self.assertEqual(is_feature_enabled("super"), "set")
+
+
+@patch.dict(
+    "superset.extensions.feature_flag_manager._feature_flags",
+    {"True_Flag1": False, "True_Flag2": True, "Flag3": False, "Flag4": True},
+    clear=True,
+)
+class TestFeatureFlagBackend(SupersetTestCase):
+    @parameterized.expand(
+        [
+            ("True_Flag1", True),
+            ("True_Flag2", True),
+            ("Flag3", False),
+            ("Flag4", True),
+            ("True_DoesNotExist", False),
+        ]
+    )
+    @patch(
+        "superset.extensions.feature_flag_manager._is_feature_enabled_func",
+        dummy_is_feature_enabled,
+    )
+    def test_feature_flags_override(self, feature_flag_name, expected):
+        self.assertEqual(is_feature_enabled(feature_flag_name), expected)
+
+    @patch(
+        "superset.extensions.feature_flag_manager._is_feature_enabled_func",
+        dummy_is_feature_enabled,
+    )
+    @patch(
+        "superset.extensions.feature_flag_manager._get_feature_flags_func", None,
+    )
+    def test_get_feature_flags(self):
+        feature_flags = get_feature_flags()
+        self.assertEqual(
+            feature_flags,
+            {"True_Flag1": True, "True_Flag2": True, "Flag3": False, "Flag4": True},
+        )