From 3ae02d1a542ed66522f6a0a51433490e7da9ea30 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Wed, 27 Feb 2019 15:11:38 -0800 Subject: [PATCH] Allow for dynamic feature flags (#6808) * Allow for dynamic feature flags Giving more control over feature flags, allowing administrator to define custom logic around whether features are enabled for particular users / roles. The exposed function can be used for things like: * progressive rollout of features (1%, 5%, 50%, 100%) * experimentation * role-based feature affectation (only admins see a particular feature) * fix build * Addressing comments * Addressing @hughhh's comments --- superset/__init__.py | 18 ++++++++++++------ superset/config.py | 16 ++++++++++++++++ superset/views/base.py | 4 ++-- tests/base_tests.py | 8 ++++++-- tests/superset_test_config.py | 9 +++++++++ 5 files changed, 45 insertions(+), 10 deletions(-) diff --git a/superset/__init__.py b/superset/__init__.py index 3f72f18ac..5e77328d6 100644 --- a/superset/__init__.py +++ b/superset/__init__.py @@ -16,6 +16,7 @@ # under the License. # pylint: disable=C,R,W """Package's main module!""" +from copy import deepcopy import json import logging from logging.handlers import TimedRotatingFileHandler @@ -212,15 +213,20 @@ security_manager = appbuilder.sm results_backend = app.config.get('RESULTS_BACKEND') # Merge user defined feature flags with default feature flags -feature_flags = app.config.get('DEFAULT_FEATURE_FLAGS') -feature_flags.update(app.config.get('FEATURE_FLAGS') or {}) +_feature_flags = app.config.get('DEFAULT_FEATURE_FLAGS') or {} +_feature_flags.update(app.config.get('FEATURE_FLAGS') or {}) + + +def get_feature_flags(): + GET_FEATURE_FLAGS_FUNC = app.config.get('GET_FEATURE_FLAGS_FUNC') + if GET_FEATURE_FLAGS_FUNC: + return GET_FEATURE_FLAGS_FUNC(deepcopy(_feature_flags)) + return _feature_flags def is_feature_enabled(feature): - """ - Utility function for checking whether a feature is turned on - """ - return feature_flags.get(feature) + """Utility function for checking whether a feature is turned on""" + return get_feature_flags().get(feature) # Registering sources diff --git a/superset/config.py b/superset/config.py index e5bfd77d9..c44328e5a 100644 --- a/superset/config.py +++ b/superset/config.py @@ -204,6 +204,22 @@ LANGUAGES = { # will result in combined feature flags of { 'FOO': True, 'BAR': True, 'BAZ': True } DEFAULT_FEATURE_FLAGS = {} +# 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 +# +# GET_FEATURE_FLAGS_FUNC can be used to implement progressive rollouts, +# role-based features, or a full on A/B testing framework. +# +# from flask import g, request +# def GET_FEATURE_FLAGS_FUNC(feature_flags_dict): +# feature_flags_dict['some_feature'] = g.user and g.user.id == 5 +# return feature_flags_dict +GET_FEATURE_FLAGS_FUNC = None + + # --------------------------------------------------- # Image and file configuration # --------------------------------------------------- diff --git a/superset/views/base.py b/superset/views/base.py index f0f5fbf7e..4f3c8e8ae 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -31,7 +31,7 @@ from flask_babel import lazy_gettext as _ import simplejson as json import yaml -from superset import conf, db, feature_flags, security_manager +from superset import conf, db, get_feature_flags, security_manager from superset.exceptions import SupersetException, SupersetSecurityException from superset.translations.utils import get_language_pack from superset.utils import core as utils @@ -157,7 +157,7 @@ class BaseSupersetView(BaseView): 'conf': {k: conf.get(k) for k in FRONTEND_CONF_KEYS}, 'locale': locale, 'language_pack': get_language_pack(locale), - 'feature_flags': feature_flags, + 'feature_flags': get_feature_flags(), } diff --git a/tests/base_tests.py b/tests/base_tests.py index e50e8e0e0..8884bbb0a 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -190,10 +190,14 @@ class SupersetTestCase(unittest.TestCase): raise Exception('run_sql failed') return resp - @patch.dict('superset.feature_flags', {'FOO': True}, clear=True) + @patch.dict('superset._feature_flags', {'FOO': True}, clear=True) def test_existing_feature_flags(self): self.assertTrue(is_feature_enabled('FOO')) - @patch.dict('superset.feature_flags', {}, clear=True) + @patch.dict('superset._feature_flags', {}, clear=True) def test_nonexistent_feature_flags(self): self.assertFalse(is_feature_enabled('FOO')) + + def test_feature_flags(self): + self.assertEquals(is_feature_enabled('foo'), 'bar') + self.assertEquals(is_feature_enabled('super'), 'set') diff --git a/tests/superset_test_config.py b/tests/superset_test_config.py index 757b8d733..d22414bd2 100644 --- a/tests/superset_test_config.py +++ b/tests/superset_test_config.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. # flake8: noqa +from copy import copy from superset.config import * AUTH_USER_REGISTRATION_ROLE = 'alpha' @@ -29,6 +30,14 @@ if 'SUPERSET__SQLALCHEMY_DATABASE_URI' in os.environ: SQL_SELECT_AS_CTA = True SQL_MAX_ROW = 666 +FEATURE_FLAGS = { + 'foo': 'bar', +} +def GET_FEATURE_FLAGS_FUNC(ff): + ff_copy = copy(ff) + ff_copy['super'] = 'set' + return ff_copy + TESTING = True SECRET_KEY = 'thisismyscretkey'