fix: feature flags typing

UPDATING.md

@@ -26,6 +26,7 @@ assists people when migrating to a new version.
 
 ### Breaking Changes
 
+- [15254](https://github.com/apache/superset/pull/15254): Previously `QUERY_COST_FORMATTERS_BY_ENGINE` and `SQL_VALIDATORS_BY_ENGINE` were defined in the feature flag dictionary in the `config.py` file. These should now be defined as a top-level config, with the feature flag dictionary being reserved for boolean only values.
 - [17290](https://github.com/apache/superset/pull/17290): Bumps pandas to `1.3.4` and pyarrow to `5.0.0`
 - [16660](https://github.com/apache/incubator-superset/pull/16660): The `columns` Jinja parameter has been renamed `table_columns` to make the `columns` query object parameter available in the Jinja context.
 - [16711](https://github.com/apache/incubator-superset/pull/16711): The `url_param` Jinja function will now by default escape the result. For instance, the value `O'Brien` will now be changed to `O''Brien`. To disable this behavior, call `url_param` with `escape_result` set to `False`: `url_param("my_key", "my default", escape_result=False)`.

superset-frontend/src/SqlLab/components/SqlEditor/index.jsx

@@ -82,6 +82,11 @@ const SET_QUERY_EDITOR_SQL_DEBOUNCE_MS = 2000;
 const VALIDATION_DEBOUNCE_MS = 600;
 const WINDOW_RESIZE_THROTTLE_MS = 100;
 
+const appContainer = document.getElementById('app');
+const bootstrapData = JSON.parse(appContainer.getAttribute('data-bootstrap'));
+const validatorMap =
+  bootstrapData?.common?.conf?.SQL_VALIDATORS_BY_ENGINE || {};
+
 const LimitSelectStyled = styled.span`
   .ant-dropdown-trigger {
     align-items: center;
@@ -392,8 +397,7 @@ class SqlEditor extends React.PureComponent {
   canValidateQuery() {
     // Check whether or not we can validate the current query based on whether
     // or not the backend has a validator configured for it.
-    const validatorMap = window.featureFlags.SQL_VALIDATORS_BY_ENGINE;
-    if (this.props.database && validatorMap != null) {
+    if (this.props.database) {
       return validatorMap.hasOwnProperty(this.props.database.backend);
     }
     return false;

superset/config.py

@@ -798,10 +798,12 @@ class CeleryConfig:  # pylint: disable=too-few-public-methods
 #
 #     return out
 #
-# FEATURE_FLAGS = {
-#     "ESTIMATE_QUERY_COST": True,
-#     "QUERY_COST_FORMATTERS_BY_ENGINE": {"postgresql": postgres_query_cost_formatter},
-# }
+#  Then on define the formatter on the config:
+#
+# "QUERY_COST_FORMATTERS_BY_ENGINE": {"postgresql": postgres_query_cost_formatter},
+QUERY_COST_FORMATTERS_BY_ENGINE: Dict[
+    str, Callable[[List[Dict[str, Any]]], List[Dict[str, Any]]]
+] = {}
 
 # Flag that controls if limit should be enforced on the CTA (create table as queries).
 SQLLAB_CTAS_NO_LIMIT = False

superset/utils/feature_flag_manager.py

@@ -15,7 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 from copy import deepcopy
-from typing import Any, Dict
+from typing import Dict
 
 from flask import Flask
 
@@ -25,15 +25,15 @@ def __init__(self) -> None:
         super().__init__()
         self._get_feature_flags_func = None
         self._is_feature_enabled_func = None
-        self._feature_flags: Dict[str, Any] = {}
+        self._feature_flags: Dict[str, bool] = {}
 
     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]:
+    def get_feature_flags(self) -> Dict[str, bool]:
         if self._get_feature_flags_func:
             return self._get_feature_flags_func(deepcopy(self._feature_flags))
         if callable(self._is_feature_enabled_func):

superset/views/base.py

@@ -96,6 +96,7 @@
     "DISPLAY_MAX_ROW",
     "GLOBAL_ASYNC_QUERIES_TRANSPORT",
     "GLOBAL_ASYNC_QUERIES_POLLING_DELAY",
+    "SQL_VALIDATORS_BY_ENGINE",
     "SQLALCHEMY_DOCS_URL",
     "SQLALCHEMY_DISPLAY_TEXT",
     "GLOBAL_ASYNC_QUERIES_WEBSOCKET_URL",

superset/views/core.py

@@ -51,7 +51,6 @@
     conf,
     db,
     event_logger,
-    get_feature_flags,
     is_feature_enabled,
     results_backend,
     results_backend_use_msgpack,
@@ -2221,9 +2220,9 @@ def estimate_query_cost(  # pylint: disable=no-self-use
             return json_error_response(utils.error_msg_from_exception(ex))
 
         spec = mydb.db_engine_spec
-        query_cost_formatters: Dict[str, Any] = get_feature_flags().get(
-            "QUERY_COST_FORMATTERS_BY_ENGINE", {}
-        )
+        query_cost_formatters: Dict[str, Any] = app.config[
+            "QUERY_COST_FORMATTERS_BY_ENGINE"
+        ]
         query_cost_formatter = query_cost_formatters.get(
             spec.engine, spec.query_cost_formatter
         )
@@ -2414,7 +2413,7 @@ def validate_sql_json(
             )
 
         spec = mydb.db_engine_spec
-        validators_by_engine = get_feature_flags().get("SQL_VALIDATORS_BY_ENGINE")
+        validators_by_engine = app.config["SQL_VALIDATORS_BY_ENGINE"]
         if not validators_by_engine or spec.engine not in validators_by_engine:
             return json_error_response(
                 "no SQL validator is configured for {}".format(spec.engine), status=400

tests/integration_tests/sql_validator_tests.py

@@ -34,13 +34,11 @@
 
 from .base_tests import SupersetTestCase
 
-PRESTO_TEST_FEATURE_FLAGS = {
-    "SQL_VALIDATORS_BY_ENGINE": {
-        "presto": "PrestoDBSQLValidator",
-        "sqlite": "PrestoDBSQLValidator",
-        "postgresql": "PrestoDBSQLValidator",
-        "mysql": "PrestoDBSQLValidator",
-    }
+PRESTO_SQL_VALIDATORS_BY_ENGINE = {
+    "presto": "PrestoDBSQLValidator",
+    "sqlite": "PrestoDBSQLValidator",
+    "postgresql": "PrestoDBSQLValidator",
+    "mysql": "PrestoDBSQLValidator",
 }
 
 
@@ -65,8 +63,8 @@ def test_validate_sql_endpoint_noconfig(self):
 
     @patch("superset.views.core.get_validator_by_name")
     @patch.dict(
-        "superset.extensions.feature_flag_manager._feature_flags",
-        PRESTO_TEST_FEATURE_FLAGS,
+        "superset.config.SQL_VALIDATORS_BY_ENGINE",
+        PRESTO_SQL_VALIDATORS_BY_ENGINE,
         clear=True,
     )
     def test_validate_sql_endpoint_mocked(self, get_validator_by_name):
@@ -98,8 +96,8 @@ def test_validate_sql_endpoint_mocked(self, get_validator_by_name):
 
     @patch("superset.views.core.get_validator_by_name")
     @patch.dict(
-        "superset.extensions.feature_flag_manager._feature_flags",
-        PRESTO_TEST_FEATURE_FLAGS,
+        "superset.config.SQL_VALIDATORS_BY_ENGINE",
+        PRESTO_SQL_VALIDATORS_BY_ENGINE,
         clear=True,
     )
     def test_validate_sql_endpoint_failure(self, get_validator_by_name):