From 486b8d911fd5e3dcb61f37c6774495a76e95612e Mon Sep 17 00:00:00 2001 From: ofekisr <35701650+ofekisr@users.noreply.github.com> Date: Mon, 28 Jun 2021 16:30:13 +0300 Subject: [PATCH] refactor(feature_flags configurations): remove redundant additional configuration for default vales (#15425) --- CONTRIBUTING.md | 6 +++--- superset/cli.py | 2 +- superset/config.py | 14 +++----------- superset/utils/feature_flag_manager.py | 3 +-- tests/cli_tests.py | 8 ++++---- 5 files changed, 12 insertions(+), 21 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 94745d9a43c70..73e7a16a01700 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -605,9 +605,9 @@ export enum FeatureFlag { } ``` -`superset/config.py` contains `DEFAULT_FEATURE_FLAGS` which will be overwritten by -those specified under FEATURE_FLAGS in `superset_config.py`. For example, `DEFAULT_FEATURE_FLAGS = { 'FOO': True, 'BAR': False }` in `superset/config.py` and `FEATURE_FLAGS = { 'BAR': True, 'BAZ': True }` in `superset_config.py` will result -in combined feature flags of `{ 'FOO': True, 'BAR': True, 'BAZ': True }`. +`superset/config.py` contains `FEATURE_FLAGS` with their default values which will be +overwritten by +those specified under FEATURE_FLAGS in `superset_config.py`. The current status of the usability of each flag (stable vs testing, etc) can be found in `RESOURCES/FEATURE_FLAGS.md`. diff --git a/superset/cli.py b/superset/cli.py index 15a8ce9e14cfb..663d2bcfe3719 100755 --- a/superset/cli.py +++ b/superset/cli.py @@ -42,7 +42,7 @@ logger = logging.getLogger(__name__) -feature_flags = config.DEFAULT_FEATURE_FLAGS.copy() +feature_flags = config.FEATURE_FLAGS.copy() feature_flags.update(config.FEATURE_FLAGS) feature_flags_func = config.GET_FEATURE_FLAGS_FUNC if feature_flags_func: diff --git a/superset/config.py b/superset/config.py index ff73edd94e291..c769330c5e83b 100644 --- a/superset/config.py +++ b/superset/config.py @@ -310,12 +310,8 @@ def _try_json_readsha( # pylint: disable=unused-argument # --------------------------------------------------- # Feature flags # --------------------------------------------------- -# Feature flags that are set by default go here. Their values can be -# overwritten by those specified under FEATURE_FLAGS in superset_config.py -# For example, DEFAULT_FEATURE_FLAGS = { 'FOO': True, 'BAR': False } here -# and FEATURE_FLAGS = { 'BAR': True, 'BAZ': True } in superset_config.py -# will result in combined feature flags of { 'FOO': True, 'BAR': True, 'BAZ': True } -DEFAULT_FEATURE_FLAGS: Dict[str, bool] = { +# Feature flags that are set by default go here. +FEATURE_FLAGS: Dict[str, bool] = { # allow dashboard to use sub-domains to send chart request # you also need ENABLE_CORS and # SUPERSET_WEBSERVER_DOMAINS for list of domains @@ -394,7 +390,7 @@ def _try_json_readsha( # pylint: disable=unused-argument } # Feature flags may also be set via 'SUPERSET_FEATURE_' prefixed environment vars. -DEFAULT_FEATURE_FLAGS.update( +FEATURE_FLAGS.update( { k[len("SUPERSET_FEATURE_") :]: parse_boolean_string(v) for k, v in os.environ.items() @@ -402,11 +398,7 @@ def _try_json_readsha( # pylint: disable=unused-argument } ) -# This is merely a default. -FEATURE_FLAGS: Dict[str, bool] = {} - # A function that receives a dict of all feature flags -# (DEFAULT_FEATURE_FLAGS merged with FEATURE_FLAGS) # can alter it, and returns a similar dict. Note the dict of feature # flags passed to the function is a deepcopy of the dict in the config, # and can therefore be mutated without side-effect diff --git a/superset/utils/feature_flag_manager.py b/superset/utils/feature_flag_manager.py index 88f19c2f46692..fc42a398ab69e 100644 --- a/superset/utils/feature_flag_manager.py +++ b/superset/utils/feature_flag_manager.py @@ -28,8 +28,7 @@ def __init__(self) -> None: def init_app(self, app: Flask) -> None: self._get_feature_flags_func = app.config["GET_FEATURE_FLAGS_FUNC"] - self._feature_flags = app.config["DEFAULT_FEATURE_FLAGS"] - self._feature_flags.update(app.config["FEATURE_FLAGS"]) + self._feature_flags = app.config["FEATURE_FLAGS"].copy() def get_feature_flags(self) -> Dict[str, Any]: if self._get_feature_flags_func: diff --git a/tests/cli_tests.py b/tests/cli_tests.py index 7ea4b9350bb32..911a9f348b3bb 100644 --- a/tests/cli_tests.py +++ b/tests/cli_tests.py @@ -82,7 +82,7 @@ def test_export_datasources_original(app_context, fs): @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") @mock.patch.dict( - "superset.config.DEFAULT_FEATURE_FLAGS", {"VERSIONED_EXPORT": True}, clear=True + "superset.config.FEATURE_FLAGS", {"VERSIONED_EXPORT": True}, clear=True ) def test_export_dashboards_versioned_export(app_context, fs): """ @@ -107,7 +107,7 @@ def test_export_dashboards_versioned_export(app_context, fs): @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") @mock.patch.dict( - "superset.config.DEFAULT_FEATURE_FLAGS", {"VERSIONED_EXPORT": True}, clear=True + "superset.config.FEATURE_FLAGS", {"VERSIONED_EXPORT": True}, clear=True ) def test_export_datasources_versioned_export(app_context, fs): """ @@ -131,7 +131,7 @@ def test_export_datasources_versioned_export(app_context, fs): @mock.patch.dict( - "superset.config.DEFAULT_FEATURE_FLAGS", {"VERSIONED_EXPORT": True}, clear=True + "superset.config.FEATURE_FLAGS", {"VERSIONED_EXPORT": True}, clear=True ) @mock.patch("superset.dashboards.commands.importers.dispatcher.ImportDashboardsCommand") def test_import_dashboards_versioned_export(import_dashboards_command, app_context, fs): @@ -170,7 +170,7 @@ def test_import_dashboards_versioned_export(import_dashboards_command, app_conte @mock.patch.dict( - "superset.config.DEFAULT_FEATURE_FLAGS", {"VERSIONED_EXPORT": True}, clear=True + "superset.config.FEATURE_FLAGS", {"VERSIONED_EXPORT": True}, clear=True ) @mock.patch("superset.datasets.commands.importers.dispatcher.ImportDatasetsCommand") def test_import_datasets_versioned_export(import_datasets_command, app_context, fs):