Merge lp:~mbp/launchpad/flags into lp:launchpad/db-devel

[Martin Pool]

Add FeatureController to read feature flags from the db

Second instalment of https://dev.launchpad.net/LEP/FeatureFlags

This adds a class that tells you about active features in a particular scope. It would be usable as is but I will do more to automatically give you the right context in a web app request, etc.

[Robert Collins (lifeless)]

I think referencing the LEP in the docstring is ok but not a substitute for including the docs in the tree - better to include the information in the module so that people don't need a web connection to understand whats going on.

I like the intended performance notes - please put them in the docstring so the API docs will show them.

I would perhaps say '
53 + This presents a higher-level interface over the storm model objects,
54 + oriented only towards readers.
55 +
56 + At this level flag names and scope names are presented as strings for
57 + easier use in Python code, though the values remain unicode. They
58 + should always be ascii like Python identifiers.
'

As
This presents an interface optimised for use in code making conditional choices based on the feature flags. At this level ....

This -
165 + # the Launchpad Collection knows how to find a good store to start from,
166 +

is a bit weird to mention.

169 === added file 'lib/lp/services/features/scopes.py'
170 --- lib/lp/services/features/scopes.py 1970-01-01 00:00:00 +0000
171 +++ lib/lp/services/features/scopes.py 2010-07-21 21:16:48 +0000
172 @@ -0,0 +1,5 @@
173 +# Copyright 2010 Canonical Ltd. This software is licensed under the
174 +# GNU Affero General Public License version 3 (see the file LICENSE).
175 +
176 +"""Look up current feature flags.
177 +"""

This seems unused - you might want to trim it for now.

179 === added directory 'lib/lp/services/features/tests'
180 === added file 'lib/lp/services/features/tests/__init__.py'
181 --- lib/lp/services/features/tests/__init__.py 1970-01-01 00:00:00 +0000
182 +++ lib/lp/services/features/tests/__init__.py 2010-07-21 21:16:48 +0000
183 @@ -0,0 +1,4 @@
184 +# Copyright 2010 Canonical Ltd. This software is licensed under the
185 +# GNU Affero General Public License version 3 (see the file LICENSE).
186 +
187 +pass

The pass isn't needed :)

189 === added file 'lib/lp/services/features/tests/test_db_settings.py'
190 --- lib/lp/services/features/tests/test_db_settings.py 1970-01-01 00:00:00 +0000
191 +++ lib/lp/services/features/tests/test_db_settings.py 2010-07-21 21:16:48 +0000
192 @@ -0,0 +1,26 @@
193 +# Copyright 2010 Canonical Ltd. This software is licensed under the
194 +# GNU Affero General Public License version 3 (see the file LICENSE).
195 +
196 +"""Tests for feature settings coming from the database"""
197 +
198 +
199 +from __future__ import with_statement
200 +__metaclass__ = type
201 +
202 +import testtools

Launchpad has a TestCase of its own you should use.

204 +from canonical.testing import layers
205 +from lp.services.features.model import (
206 + FeatureFlag,
207 + FeatureFlagCollection,
208 + )
209 +
210 +
211 +class TestFeatureModel(testtools.TestCase):
212 +
213 + layer = layers.DatabaseFunctionalLayer
214 +
215 + def test_defaultEmptyCollection(self):
216 + # there are no settings in the sampledata
217 + coll = FeatureFlagCollection()
218 + self.assertTrue(coll.select().is_empty())

I'd make that statement stronger: there should be no settings...

220 === added file 'lib/lp/services/features/tests/test_flags.py'
221 --- lib/lp/services/fea...

review: Approve

[Martin Pool (mbp)]

Incremental diff:

 * facade loads everything at construction time
 * remove write facade methods

=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py 2010-07-22 10:48:14 +0000
+++ lib/lp/services/features/flags.py 2010-07-22 12:28:32 +0000
@@ -6,21 +6,16 @@
 __metaclass__ = type

-from lp.services.features import model
+from lp.services.features.model import (
+ getFeatureStore,
+ FeatureFlag,
+ )

 from storm.locals import (
     Desc,
     )

-# Intended performance: when this object is first constructed, it will read
-# the whole current feature flags from the database. This will take a few ms.
-# The controller is then supposed to be held in a thread-local for the
-# duration of the request. The scopes can be changed over the lifetime of the
-# controller, because we might not know enough to determine all the active
-# scopes when the object's first created. This isn't validated to work yet.
-
-
 class FeatureController(object):
     """A FeatureController tells application code what features are active.

@@ -30,18 +25,21 @@
       of the current web request, or the user for whom a job is being run, or
       something similar.

- This presents a higher-level facade that is independent of how the flags
- are stored. At the moment they are stored in the database but callers
- shouldn't need to know about that: they could go into memcachedb or
- elsewhere in future.
+ FeatureController presents a high level interface for application code to
+ query flag values, without it needing to know that they are stored in the database.

     At this level flag names and scope names are presented as strings for
     easier use in Python code, though the values remain unicode. They
     should always be ascii like Python identifiers.

- One instance of this should be constructed for the lifetime of code that
- has consistent configuration values. For instance there will be one per web
- app request.
+ One instance of FeatureController should be constructed for the lifetime
+ of code that has consistent configuration values. For instance there will
+ be one per web app request.
+
+ Intended performance: when this object is first constructed, it will read
+ the whole current feature flags from the database. This will take a few ms.
+ The controller is then supposed to be held in a thread-local for the
+ duration of the request.

     See <https://dev.launchpad.net/LEP/FeatureFlags>
     """
@@ -49,34 +47,33 @@
     def __init__(self, scopes):
         """Construct a new view of the features for a set of scopes.
         """
- self._store = model.getFeatureStore()
- self.scopes = self._preenScopes(scopes)
+ self._store = getFeatureStore()
+ self._scopes = self._preenScopes(scopes)
+ self._cached_flags = self._queryAllFlags()

- def setScopes(self, scopes):
- self.scopes = self._preenScopes(scopes)
+ def getScopes(self):
+ return frozenset(self._scopes)

     def getFlag(self, flag_name):
- rs = (self._store
- .find(model.FeatureFlag,
- mod...

[Martin Pool (mbp)]

ec2 disappeared into thin air, perhaps because of smtp setup problems. Would someone please submit this on my behalf?

=== added directory 'lib/lp/services/features'
=== added file 'lib/lp/services/features/__init__.py'
--- lib/lp/services/features/__init__.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/features/__init__.py	2010-07-25 09:39:52 +0000
@@ -0,0 +1,10 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""lp.services.features provide dynamically configurable feature flags.
+
+These can be turned on and off by admins, and can affect particular
+defined scopes such as "beta users" or "production servers."
+
+See <https://dev.launchpad.net/LEP/FeatureFlags>
+"""
 
=== added file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/features/flags.py	2010-07-25 09:39:52 +0000
@@ -0,0 +1,85 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__all__ = ['FeatureController']
+
+__metaclass__ = type
+
+
+from lp.services.features.model import (
+    getFeatureStore,
+    FeatureFlag,
+    )
+
+
+class FeatureController(object):
+    """A FeatureController tells application code what features are active.
+
+    It does this by meshing together two sources of data:
+    - feature flags, typically set by an administrator into the database
+    - feature scopes, which would typically be looked up based on attributes
+      of the current web request, or the user for whom a job is being run, or
+      something similar.
+
+    FeatureController presents a high level interface for application code to
+    query flag values, without it needing to know that they are stored in the
+    database.
+
+    At this level flag names and scope names are presented as strings for
+    easier use in Python code, though the values remain unicode.  They
+    should always be ascii like Python identifiers.
+
+    One instance of FeatureController should be constructed for the lifetime
+    of code that has consistent configuration values.  For instance there will
+    be one per web app request.
+
+    Intended performance: when this object is first constructed, it will read
+    the whole current feature flags from the database.  This will take a few
+    ms.  The controller is then supposed to be held in a thread-local for the
+    duration of the request.
+
+    See <https://dev.launchpad.net/LEP/FeatureFlags>
+    """
+
+    def __init__(self, scopes):
+        """Construct a new view of the features for a set of scopes.
+        """
+        self._store = getFeatureStore()
+        self._scopes = self._preenScopes(scopes)
+        self._cached_flags = self._queryAllFlags()
+
+    def getScopes(self):
+        return frozenset(self._scopes)
+
+    def getFlag(self, flag_name):
+        return self._cached_flags.get(flag_name)
+
+    def getAllFlags(self):
+        """Get the feature flags active for the current scopes.
+
+        :returns: dict from flag_name (unicode) to value (unicode).
+        """
+        return dict(self._cached_flags)
+
+    def _queryAllFlags(self):
+        d = {}
+        rs = (self._store
+                .find(FeatureFlag,
+                    FeatureFlag.scope.is_in(self._scopes))
+                .order_by(FeatureFlag.priority)
+                .values(FeatureFlag.flag, FeatureFlag.value))
+        for flag, value in rs:
+            d[str(flag)] = value
+        return d
+
+    def _preenScopes(self, scopes):
+        # for convenience turn strings to unicode
+        us = []
+        for s in scopes:
+            if isinstance(s, unicode):
+                us.append(s)
+            elif isinstance(s, str):
+                us.append(unicode(s))
+            else:
+                raise TypeError("invalid scope: %r" % s)
+        return us
 
=== added file 'lib/lp/services/features/model.py'
--- lib/lp/services/features/model.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/features/model.py	2010-07-25 09:39:52 +0000
@@ -0,0 +1,42 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__all__ = [
+    'FeatureFlag',
+    'getFeatureStore',
+    ]
+
+__metaclass__ = type
+
+from zope.component import getUtility
+from storm.locals import Int, Storm, Unicode, DateTime
+
+from canonical.launchpad.webapp.interfaces import (
+    DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE)
+
+
+def getFeatureStore():
+    """Get Storm store to access feature definitions."""
+    # TODO: This is copied so many times in Launchpad; maybe it should be more
+    # general?
+    return getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
+
+
+class FeatureFlag(Storm):
+    """Database setting of a particular flag in a scope"""
+
+    __storm_table__ = 'FeatureFlag'
+    __storm_primary__ = "scope", "flag"
+
+    scope = Unicode(allow_none=False)
+    flag = Unicode(allow_none=False)
+    priority = Int(allow_none=False)
+    value = Unicode(allow_none=False)
+    date_modified = DateTime()
+
+    def __init__(self, scope, priority, flag, value):
+        super(FeatureFlag, self).__init__()
+        self.scope = scope
+        self.priority = priority
+        self.flag = flag
+        self.value = value
 
=== added file 'lib/lp/services/features/scopes.py'
--- lib/lp/services/features/scopes.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/features/scopes.py	2010-07-25 09:39:52 +0000
@@ -0,0 +1,5 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Look up current feature flags.
+"""
 
=== added directory 'lib/lp/services/features/tests'
=== added file 'lib/lp/services/features/tests/__init__.py'
--- lib/lp/services/features/tests/__init__.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/features/tests/__init__.py	2010-07-25 09:39:52 +0000
@@ -0,0 +1,2 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
 
=== added file 'lib/lp/services/features/tests/test_db_settings.py'
--- lib/lp/services/features/tests/test_db_settings.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/features/tests/test_db_settings.py	2010-07-25 09:39:52 +0000
@@ -0,0 +1,25 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for feature settings coming from the database"""
+
+
+from __future__ import with_statement
+__metaclass__ = type
+
+from canonical.testing import layers
+from lp.testing import TestCase
+from lp.services.features.model import (
+    getFeatureStore,
+    FeatureFlag,
+    )
+
+
+class TestFeatureModel(TestCase):
+
+    layer = layers.DatabaseFunctionalLayer
+
+    def test_defaultEmptyCollection(self):
+        # there are no settings in the sampledata
+        store = getFeatureStore()
+        self.assertTrue(store.find(FeatureFlag).is_empty())
 
=== added file 'lib/lp/services/features/tests/test_flags.py'
--- lib/lp/services/features/tests/test_flags.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/features/tests/test_flags.py	2010-07-25 09:39:52 +0000
@@ -0,0 +1,95 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for feature flags."""
+
+
+from __future__ import with_statement
+__metaclass__ = type
+
+import os
+
+from canonical.testing import layers
+from lp.testing import TestCase
+
+from lp.services.features import model
+from lp.services.features.flags import (
+    FeatureController,
+    )
+
+
+notification_name = 'notification.global.text'
+notification_value = u'\N{SNOWMAN} stormy Launchpad weather ahead'
+example_scope = 'beta_user'
+
+
+testdata = [
+    (example_scope, notification_name, notification_value, 100),
+    ('default', 'ui.icing', u'3.0', 100),
+    ('beta_user', 'ui.icing', u'4.0', 300),
+    ]
+
+
+class TestFeatureFlags(TestCase):
+
+    layer = layers.DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestFeatureFlags, self).setUp()
+        if os.environ.get("STORM_TRACE", None):
+            from storm.tracer import debug
+            debug(True)
+
+    def populateStore(self):
+        store = model.getFeatureStore()
+        for (scope, flag, value, priority) in testdata:
+            store.add(model.FeatureFlag(
+                scope=unicode(scope),
+                flag=unicode(flag),
+                value=value,
+                priority=priority))
+
+    def test_defaultFlags(self):
+        # the sample db has no flags set
+        control = FeatureController([])
+        self.assertEqual({},
+            control.getAllFlags())
+
+    def test_getFlag(self):
+        self.populateStore()
+        control = FeatureController(['default'])
+        self.assertEqual(u'3.0',
+            control.getFlag('ui.icing'))
+
+    def test_getAllFlags(self):
+        # can fetch all the active flags, and it gives back only the
+        # highest-priority settings
+        self.populateStore()
+        control = FeatureController(['default', 'beta_user'])
+        self.assertEqual(
+            {'ui.icing': '4.0',
+             notification_name: notification_value},
+            control.getAllFlags())
+
+    def test_overrideFlag(self):
+        # if there are multiple settings for a flag, and they match multiple
+        # scopes, the priorities determine which is matched
+        self.populateStore()
+        default_control = FeatureController(['default'])
+        self.assertEqual(
+            u'3.0',
+            default_control.getFlag('ui.icing'))
+        beta_control = FeatureController(['default', 'beta_user'])
+        self.assertEqual(
+            u'4.0',
+            beta_control.getFlag('ui.icing'))
+
+    def test_undefinedFlag(self):
+        # if the flag is not defined, we get None
+        self.populateStore()
+        control = FeatureController(['default', 'beta_user'])
+        self.assertIs(None,
+            control.getFlag('unknown_flag'))
+        no_scope_flags = FeatureController([])
+        self.assertIs(None,
+            no_scope_flags.getFlag('ui.icing'))