This is an automated email from the ASF dual-hosted git repository.
maximebeauchemin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 3ae02d1 Allow for dynamic feature flags (#6808)
3ae02d1 is described below
commit 3ae02d1a542ed66522f6a0a51433490e7da9ea30
Author: Maxime Beauchemin <[email protected]>
AuthorDate: Wed Feb 27 15:11:38 2019 -0800
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 3f72f18..5e77328 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 e5bfd77..c44328e 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 f0f5fbf..4f3c8e8 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 e50e8e0..8884bbb 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 757b8d7..d22414b 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'
