launchpad-reviewers team mailing list archive

[Martin Pool <mbp@xxxxxxxxxxxxx>]

Martin Pool has proposed merging lp:~mbp/launchpad/feature-modulus into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~mbp/launchpad/feature-modulus/+merge/84042

Add a userslice:x,y feature scope, so that 1% of the users can enjoy 80% of the features.
-- 
https://code.launchpad.net/~mbp/launchpad/feature-modulus/+merge/84042
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~mbp/launchpad/feature-modulus into lp:launchpad.

=== modified file 'lib/lp/services/features/scopes.py'
--- lib/lp/services/features/scopes.py	2011-09-22 01:41:31 +0000
+++ lib/lp/services/features/scopes.py	2011-12-01 01:14:28 +0000
@@ -17,6 +17,7 @@
     'ScopesForScript',
     'ScopesFromRequest',
     'TeamScope',
+    'UserSliceScope',
     'undocumented_scopes',
     ]
 
@@ -107,7 +108,22 @@
             self._request._orig_env.get('launchpad.pageid', '')))
 
 
-class TeamScope(BaseScope):
+class ScopeWithPerson(BaseScope):
+    """An abstract base scope that matches on the current user of the request.
+
+    Intended for subclassing, not direct use.
+    """
+
+    def __init__(self, get_person):
+        self._get_person = get_person
+        self._person = None
+
+    @cachedproperty
+    def person(self):
+        return self._get_person()
+
+
+class TeamScope(ScopeWithPerson):
     """A user's team memberships.
 
     Team ID scopes are written as 'team:' + the team name to match.
@@ -122,14 +138,6 @@
 
     pattern = r'team:'
 
-    def __init__(self, get_person):
-        self._get_person = get_person
-        self._person = None
-
-    @cachedproperty
-    def person(self):
-        return self._get_person()
-
     def lookup(self, scope_name):
         """Is the given scope a team membership?
 
@@ -142,6 +150,36 @@
             return self.person.inTeam(team_name)
 
 
+class UserSliceScope(ScopeWithPerson):
+    """Selects a slice of all users based on their id.
+
+    Written as 'userslice:a,b' with 0<=a<b will slice the entire user
+    population into b approximately equal sub-populations, and then take the
+    a'th zero-based index.
+
+    For example, to test a new feature for 1% of users, and a different
+    version of it for a different 1% you can use `userslice:10,100` and
+    userslice:20,100`.
+
+    You may wish to avoid using the same a or b for different rules so that
+    some users don't have all the fun by being in eg 0,100.
+    """
+
+    pattern = r'userslice:(\d+),(\d+)'
+
+    def lookup(self, scope_name):
+        match = self.compiled_pattern.match(scope_name)
+        if not match:
+            return  # Shouldn't happen...
+        try:
+            modulus = int(match.group(1))
+            divisor = int(match.group(2))
+        except ValueError:
+            return
+        person_id = self.person.id
+        return (person_id % divisor) == modulus
+
+
 class ServerScope(BaseScope):
     """Matches the current server.
 
@@ -254,7 +292,8 @@
         scopes.extend([
             PageScope(request),
             ServerScope(),
-            TeamScope(person_from_request)
+            TeamScope(person_from_request),
+            UserSliceScope(person_from_request),
             ])
         super(ScopesFromRequest, self).__init__(scopes)
 

=== modified file 'lib/lp/services/features/tests/test_scopes.py'
--- lib/lp/services/features/tests/test_scopes.py	2011-09-21 21:12:02 +0000
+++ lib/lp/services/features/tests/test_scopes.py	2011-12-01 01:14:28 +0000
@@ -6,16 +6,22 @@
 __metaclass__ = type
 
 from canonical.testing.layers import DatabaseFunctionalLayer
-from lp.testing import TestCaseWithFactory
+
+from lp.testing import (
+    TestCaseWithFactory,
+    )
+
 from lp.services.features.scopes import (
     BaseScope,
     MultiScopeHandler,
     ScopesForScript,
     ScriptScope,
+    UserSliceScope,
     )
 
 
 class FakeScope(BaseScope):
+
     pattern = r'fake:'
 
     def __init__(self, name):
@@ -69,3 +75,24 @@
         script_name = self.factory.getUniqueString()
         scopes = ScopesForScript(script_name)
         self.assertFalse(scopes.lookup("script:other"))
+
+
+class TestUserSliceScope(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_user_modulus(self):
+        person = self.factory.makePerson()
+        # NB: scopes take a callable that returns the person, that in
+        # production comes from the request.
+        scope = UserSliceScope(lambda: person)
+        # Effectively selects everyone; should always be true.
+        self.assertTrue(scope.lookup('userslice:0,1'))
+        # Exactly one of these should be true.
+        checks = 7
+        matches = []
+        for i in range(checks):
+            name = 'userslice:%d,%d' % (i, checks)
+            if scope.lookup(name):
+                matches.append(name)
+        self.assertEquals(len(matches), 1, matches)