Launchpad itself

Merge lp:~mwhudson/launchpad/feature-flag-xmlrpc into lp:launchpad

  1. feature-flag-xmlrpc
  2. Merge into devel
Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: no longer in the source branch.
Merged at revision: 13986
Proposed branch: lp:~mwhudson/launchpad/feature-flag-xmlrpc
Merge into: lp:launchpad
Diff against target: 523 lines (+250/-37)
13 files modified
configs/development/launchpad-lazr.conf (+1/-0)
lib/canonical/config/schema-lazr.conf (+6/-0)
lib/canonical/launchpad/interfaces/launchpad.py (+2/-0)
lib/canonical/launchpad/xmlrpc/application.py (+6/-1)
lib/lp/services/features/__init__.py (+12/-0)
lib/lp/services/features/configure.zcml (+7/-0)
lib/lp/services/features/scopes.py (+45/-26)
lib/lp/services/features/tests/test_scopes.py (+5/-3)
lib/lp/services/features/tests/test_webapp.py (+3/-3)
lib/lp/services/features/tests/test_xmlrpc.py (+99/-0)
lib/lp/services/features/xmlrpc.py (+59/-0)
lib/lp/testing/factory.py (+2/-2)
utilities/page-performance-report.ini (+3/-2)
To merge this branch: bzr merge lp:~mwhudson/launchpad/feature-flag-xmlrpc
Reviewer Review Type Date Requested Status
Robert Collins (community) Approve
Review via email: mp+75673@code.launchpad.net

Commit message

[r=lifeless][bug=853635] Add a private XML-RPC interface to query feature flags

Description of the change

This branch adds a private XML-RPC interface to query feature flags. It supports the team: scope (if you pass a username) and arbitrary other scopes.

It's been a while, so I've probably forgotten some coding standards things. I did remember to run "make lint" though :)

To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

* if we passed a dict of values from the client, would that make it
easier to consider other factors later (like the client ip, bzr
version, front end server, etc)? bearing in mind that different
machines that talk to the xmlrpc server may have different subsets of
these, hardcoding them as parameters may be inflexible. (maybe xmlrpc
can't do a dict but you can do a list of pairs of strings.)

* maybe FixedScope could be somewhere more general (why not have it
available in production, for that matter.)

* there are developer docs in the tree that ought to be updated.

this looks really useful, thanks.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

1. I tried a bit, but I just can't make a dict of values for the scopes work in my head. I've rejigged the scope code for more reusability though (and removed a use of ILaunchBag \o/).

2. Sure, I've moved FixedScope into scopes.py now.

3. Where are the developer docs? In my tree lib/lp/services/features/doc/features.txt is 0 bytes long, which is a bit odd...

Cheers,
mwh

Revision history for this message
Robert Collins (lifeless) wrote :

I kindof had it in my head that we could inject more info about the
context in the method - e.g. rather than saying
scopes=('pageid:1234',) we could use the standard scope lookup code
and just inject the current user, current server etc into that.

This is particularly relevant if you consider the scope 'random:0.5'
(true 50% of the time).

I think this needs a (small) bit of rework to be able to delegate to
the stock scope facilities TBH.

Even if the first cut doesn't *need* that, I think the contract should
be one that is amenable to that. IOW, rather than passing in the live
scopes, pass in the context used to derive live scopes, and get rid of
your custom scope lookup code. (It duplicates the team scope handling
anyway, and that also is a must-fix).

-Rob

Revision history for this message
Robert Collins (lifeless) wrote :

Ah, this is much better.

+ scopes = [
177 DefaultScope(),
181 + PageScope(request._orig_env.get('launchpad.pageid', '')),
182 + ServerScope(),
183 + ]

I would like there to be one-place to add scopes like RandomScope etc - so perhaps:

default_scopes = [DefaultScope()]

and then in the relevant places

scopes = list(default_scopes)
scopes.extend([PageScope(...), ServerScope(),...])

etc.

What do you think?

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Funny you should suggest that, as I'd already written basically exactly that in my working copy...

I've also updated the developer documentation a bit.

Revision history for this message
Robert Collins (lifeless) wrote :

You are missing a : in your dev docs, other than that looks wicked to me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'configs/development/launchpad-lazr.conf'
2--- configs/development/launchpad-lazr.conf 2011-09-14 14:37:43 +0000
3+++ configs/development/launchpad-lazr.conf 2011-09-19 04:10:29 +0000
4@@ -158,6 +158,7 @@
5 geonames_identity: lpdev
6 storm_cache: generational
7 storm_cache_size: 100
8+feature_flags_endpoint: http://xmlrpc-private.launchpad.dev:8087/featureflags/
9
10 [launchpad_session]
11 cookie: launchpad_dev
12
13=== modified file 'lib/canonical/config/schema-lazr.conf'
14--- lib/canonical/config/schema-lazr.conf 2011-09-16 14:55:43 +0000
15+++ lib/canonical/config/schema-lazr.conf 2011-09-19 04:10:29 +0000
16@@ -1211,6 +1211,12 @@
17 # The number of items to display:
18 homepage_recent_posts_count: 6
19
20+# The URL of the XML-RPC endpoint that allows querying of feature
21+# flags. This should implement IFeatureFlagApplication.
22+#
23+# datatype: string
24+feature_flags_endpoint:
25+
26 [launchpad_session]
27 # The database connection string.
28 # datatype: pgconnection
29
30=== modified file 'lib/canonical/launchpad/interfaces/launchpad.py'
31--- lib/canonical/launchpad/interfaces/launchpad.py 2011-05-27 19:53:20 +0000
32+++ lib/canonical/launchpad/interfaces/launchpad.py 2011-09-19 04:10:29 +0000
33@@ -152,6 +152,8 @@
34 softwarecenteragent = Attribute(
35 """Software center agent XML-RPC end point.""")
36
37+ featureflags = Attribute("""Feature flag information endpoint""")
38+
39
40 class IAuthServerApplication(ILaunchpadApplication):
41 """Launchpad legacy AuthServer application root."""
42
43=== modified file 'lib/canonical/launchpad/xmlrpc/application.py'
44--- lib/canonical/launchpad/xmlrpc/application.py 2011-04-28 21:36:47 +0000
45+++ lib/canonical/launchpad/xmlrpc/application.py 2011-09-19 04:10:29 +0000
46@@ -36,6 +36,7 @@
47 )
48 from lp.registry.interfaces.mailinglist import IMailingListApplication
49 from lp.registry.interfaces.person import ISoftwareCenterAgentApplication
50+from lp.services.features.xmlrpc import IFeatureFlagApplication
51
52
53 # NOTE: If you add a traversal here, you should update
54@@ -73,6 +74,11 @@
55 """See `IPrivateApplication`."""
56 return getUtility(ISoftwareCenterAgentApplication)
57
58+ @property
59+ def featureflags(self):
60+ """See `IPrivateApplication`."""
61+ return getUtility(IFeatureFlagApplication)
62+
63
64 class ISelfTest(Interface):
65 """XMLRPC external interface for testing the XMLRPC external interface."""
66@@ -127,4 +133,3 @@
67
68 def run_test(self):
69 return "OK"
70-
71
72=== modified file 'lib/lp/services/features/__init__.py'
73--- lib/lp/services/features/__init__.py 2011-08-02 23:42:08 +0000
74+++ lib/lp/services/features/__init__.py 2011-09-19 04:10:29 +0000
75@@ -125,6 +125,18 @@
76 if value:
77 print value
78
79+Checking flags without access to the database
80+=============================================
81+
82+Feature flags can also be checked without access to the database by making use
83+of the 'getFeatureFlag' XML-RPC method.
84+
85+ server_proxy = xmlrpclib.ServerProxy(
86+ config.launchpad.feature_flags_endpoint, allow_none=True)
87+ if server_proxy.getFeatureFlag(
88+ 'codehosting.use_forking_server', ['user:' + user_name]):
89+ pass
90+
91 Debugging feature usage
92 =======================
93
94
95=== modified file 'lib/lp/services/features/configure.zcml'
96--- lib/lp/services/features/configure.zcml 2010-08-12 06:55:37 +0000
97+++ lib/lp/services/features/configure.zcml 2011-09-19 04:10:29 +0000
98@@ -19,4 +19,11 @@
99 handler="lp.services.features.webapp.end_request"
100 />
101
102+ <securedutility
103+ class="lp.services.features.xmlrpc.FeatureFlagApplication"
104+ provides="lp.services.features.xmlrpc.IFeatureFlagApplication">
105+ <allow
106+ interface="lp.services.features.xmlrpc.IFeatureFlagApplication"/>
107+ </securedutility>
108+
109 </configure>
110
111=== modified file 'lib/lp/services/features/scopes.py'
112--- lib/lp/services/features/scopes.py 2011-08-02 23:42:08 +0000
113+++ lib/lp/services/features/scopes.py 2011-09-19 04:10:29 +0000
114@@ -9,9 +9,14 @@
115 """
116
117 __all__ = [
118+ 'DefaultScope',
119+ 'default_scopes',
120+ 'FixedScope',
121 'HANDLERS',
122+ 'MultiScopeHandler',
123 'ScopesForScript',
124 'ScopesFromRequest',
125+ 'TeamScope',
126 'undocumented_scopes',
127 ]
128
129@@ -19,9 +24,7 @@
130
131 import re
132
133-from zope.component import getUtility
134-
135-from canonical.launchpad.webapp.interfaces import ILaunchBag
136+from lp.registry.interfaces.person import IPerson
137 from lp.services.propertycache import cachedproperty
138 import canonical.config
139
140@@ -60,14 +63,7 @@
141 return True
142
143
144-class BaseWebRequestScope(BaseScope):
145- """Base class for scopes that key off web request attributes."""
146-
147- def __init__(self, request):
148- self.request = request
149-
150-
151-class PageScope(BaseWebRequestScope):
152+class PageScope(BaseScope):
153 """The current page ID.
154
155 Pageid scopes are written as 'pageid:' + the pageid to match. Pageids
156@@ -81,6 +77,9 @@
157
158 pattern = r'pageid:'
159
160+ def __init__(self, pageid):
161+ self._pageid = pageid
162+
163 def lookup(self, scope_name):
164 """Is the given scope match the current pageid?"""
165 pageid_scope = scope_name[len('pageid:'):]
166@@ -104,12 +103,11 @@
167
168 @cachedproperty
169 def _request_pageid_namespace(self):
170- return tuple(self._pageid_to_namespace(
171- self.request._orig_env.get('launchpad.pageid', '')))
172+ return tuple(self._pageid_to_namespace(self._pageid))
173
174
175 class TeamScope(BaseScope):
176- """The current user's team memberships.
177+ """A user's team memberships.
178
179 Team ID scopes are written as 'team:' + the team name to match.
180
181@@ -119,6 +117,9 @@
182
183 pattern = r'team:'
184
185+ def __init__(self, person):
186+ self.person = person
187+
188 def lookup(self, scope_name):
189 """Is the given scope a team membership?
190
191@@ -127,10 +128,7 @@
192 fixed to reduce this to one query).
193 """
194 team_name = scope_name[len('team:'):]
195- person = getUtility(ILaunchBag).user
196- if person is None:
197- return False
198- return person.inTeam(team_name)
199+ return self.person.inTeam(team_name)
200
201
202 class ServerScope(BaseScope):
203@@ -169,6 +167,20 @@
204 return scope_name == self.script_scope
205
206
207+class FixedScope(BaseScope):
208+ """A scope that matches an exact value.
209+
210+ Functionally `ScriptScope` and `DefaultScope` are equivalent to instances
211+ of this class, but their docstings are used on the +feature-info page.
212+ """
213+
214+ def __init__(self, scope):
215+ self.pattern = re.escape(scope) + '$'
216+
217+ def lookup(self, scope_name):
218+ return True
219+
220+
221 # These are the handlers for all of the allowable scopes, listed here so that
222 # we can for example show all of them in an admin page. Any new scope will
223 # need a scope handler and that scope handler has to be added to this list.
224@@ -213,21 +225,28 @@
225 undocumented_scopes.add(scope_name)
226
227
228+default_scopes = (DefaultScope(),)
229+
230+
231 class ScopesFromRequest(MultiScopeHandler):
232 """Identify feature scopes based on request state."""
233
234 def __init__(self, request):
235- super(ScopesFromRequest, self).__init__([
236- DefaultScope(),
237- PageScope(request),
238- TeamScope(),
239- ServerScope()])
240+ scopes = list(default_scopes)
241+ scopes.extend([
242+ PageScope(request._orig_env.get('launchpad.pageid', '')),
243+ ServerScope(),
244+ ])
245+ person = IPerson(request.principal, None)
246+ if person is not None:
247+ scopes.append(TeamScope(person))
248+ super(ScopesFromRequest, self).__init__(scopes)
249
250
251 class ScopesForScript(MultiScopeHandler):
252 """Identify feature scopes for a given script."""
253
254 def __init__(self, script_name):
255- super(ScopesForScript, self).__init__([
256- DefaultScope(),
257- ScriptScope(script_name)])
258+ scopes = list(default_scopes)
259+ scopes.append(ScriptScope(script_name))
260+ super(ScopesForScript, self).__init__(scopes)
261
262=== modified file 'lib/lp/services/features/tests/test_scopes.py'
263--- lib/lp/services/features/tests/test_scopes.py 2011-08-02 23:42:08 +0000
264+++ lib/lp/services/features/tests/test_scopes.py 2011-09-19 04:10:29 +0000
265@@ -9,18 +9,20 @@
266 from lp.testing import TestCaseWithFactory
267 from lp.services.features.scopes import (
268 BaseScope,
269- BaseWebRequestScope,
270 MultiScopeHandler,
271 ScopesForScript,
272 ScriptScope,
273 )
274
275
276-class FakeScope(BaseWebRequestScope):
277+class FakeScope(BaseScope):
278 pattern = r'fake:'
279
280+ def __init__(self, name):
281+ self.name = name
282+
283 def lookup(self, scope_name):
284- return scope_name == (self.pattern + self.request)
285+ return scope_name == (self.pattern + self.name)
286
287
288 class TestScopes(TestCaseWithFactory):
289
290=== modified file 'lib/lp/services/features/tests/test_webapp.py'
291--- lib/lp/services/features/tests/test_webapp.py 2011-01-20 18:33:25 +0000
292+++ lib/lp/services/features/tests/test_webapp.py 2011-09-19 04:10:29 +0000
293@@ -86,14 +86,14 @@
294
295 def test_team_scope_outside_team(self):
296 request = LaunchpadTestRequest()
297+ self.factory.loginAsAnyone(request)
298 scopes = webapp.ScopesFromRequest(request)
299- self.factory.loginAsAnyone()
300 self.assertFalse(scopes.lookup('team:nonexistent'))
301
302 def test_team_scope_in_team(self):
303 request = LaunchpadTestRequest()
304- scopes = webapp.ScopesFromRequest(request)
305 member = self.factory.makePerson()
306 team = self.factory.makeTeam(members=[member])
307- login_as(member)
308+ login_as(member, request)
309+ scopes = webapp.ScopesFromRequest(request)
310 self.assertTrue(scopes.lookup('team:%s' % team.name))
311
312=== added file 'lib/lp/services/features/tests/test_xmlrpc.py'
313--- lib/lp/services/features/tests/test_xmlrpc.py 1970-01-01 00:00:00 +0000
314+++ lib/lp/services/features/tests/test_xmlrpc.py 2011-09-19 04:10:29 +0000
315@@ -0,0 +1,99 @@
316+# Copyright 2011 Canonical Ltd. This software is licensed under the
317+# GNU Affero General Public License version 3 (see the file LICENSE).
318+
319+"""Tests for FeatureFlagApplication."""
320+
321+__metaclass__ = type
322+
323+import xmlrpclib
324+
325+from canonical.testing.layers import DatabaseFunctionalLayer
326+from canonical.config import config
327+from lp.services import features
328+from lp.services.features.flags import FeatureController
329+from lp.services.features.rulesource import StormFeatureRuleSource
330+from lp.services.features.scopes import (
331+ DefaultScope,
332+ FixedScope,
333+ MultiScopeHandler,
334+ )
335+from lp.services.features.xmlrpc import FeatureFlagApplication
336+from lp.testing import (
337+ feature_flags,
338+ set_feature_flag,
339+ TestCaseWithFactory,
340+ )
341+from lp.testing.xmlrpc import XMLRPCTestTransport
342+
343+
344+class TestGetFeatureFlag(TestCaseWithFactory):
345+
346+ layer = DatabaseFunctionalLayer
347+
348+ def setUp(self):
349+ TestCaseWithFactory.setUp(self)
350+ self.endpoint = FeatureFlagApplication()
351+
352+ def installFeatureController(self, feature_controller):
353+ old_features = features.get_relevant_feature_controller()
354+ features.install_feature_controller(feature_controller)
355+ self.addCleanup(
356+ features.install_feature_controller, old_features)
357+
358+ def test_getFeatureFlag_returns_None_by_default(self):
359+ self.assertIs(None, self.endpoint.getFeatureFlag(u'unknown'))
360+
361+ def test_getFeatureFlag_returns_true_for_set_flag(self):
362+ flag_name = u'flag'
363+ with feature_flags():
364+ set_feature_flag(flag_name, u'1')
365+ self.assertEqual(u'1', self.endpoint.getFeatureFlag(flag_name))
366+
367+ def test_getFeatureFlag_ignores_relevant_feature_controller(self):
368+ # getFeatureFlag should only consider the scopes it is asked to
369+ # consider, not any that happen to be active due to the XML-RPC
370+ # request itself.
371+ flag_name = u'flag'
372+ scope_name = u'scope'
373+ self.installFeatureController(
374+ FeatureController(
375+ MultiScopeHandler(
376+ [DefaultScope(), FixedScope(scope_name)]).lookup,
377+ StormFeatureRuleSource()))
378+ set_feature_flag(flag_name, u'1', scope_name)
379+ self.assertEqual(None, self.endpoint.getFeatureFlag(flag_name))
380+
381+ def test_getFeatureFlag_considers_supplied_scope(self):
382+ flag_name = u'flag'
383+ scope_name = u'scope'
384+ with feature_flags():
385+ set_feature_flag(flag_name, u'value', scope_name)
386+ self.assertEqual(
387+ u'value',
388+ self.endpoint.getFeatureFlag(flag_name, [scope_name]))
389+
390+ def test_getFeatureFlag_turns_user_into_team_scope(self):
391+ flag_name = u'flag'
392+ person = self.factory.makePerson()
393+ team = self.factory.makeTeam(members=[person])
394+ with feature_flags():
395+ set_feature_flag(flag_name, u'value', u'team:' + team.name)
396+ self.assertEqual(
397+ u'value',
398+ self.endpoint.getFeatureFlag(
399+ flag_name, ['user:' + person.name]))
400+
401+ def test_xmlrpc_interface_unset(self):
402+ sp = xmlrpclib.ServerProxy(
403+ config.launchpad.feature_flags_endpoint,
404+ transport=XMLRPCTestTransport(), allow_none=True)
405+ self.assertEqual(None, sp.getFeatureFlag(u'flag'))
406+
407+ def test_xmlrpc_interface_set(self):
408+ sp = xmlrpclib.ServerProxy(
409+ config.launchpad.feature_flags_endpoint,
410+ transport=XMLRPCTestTransport(), allow_none=True)
411+ flag_name = u'flag'
412+ with feature_flags():
413+ set_feature_flag(flag_name, u'1')
414+ self.assertEqual(u'1', sp.getFeatureFlag(flag_name))
415
416=== added file 'lib/lp/services/features/xmlrpc.py'
417--- lib/lp/services/features/xmlrpc.py 1970-01-01 00:00:00 +0000
418+++ lib/lp/services/features/xmlrpc.py 2011-09-19 04:10:29 +0000
419@@ -0,0 +1,59 @@
420+# Copyright 2011 Canonical Ltd. This software is licensed under the
421+# GNU Affero General Public License version 3 (see the file LICENSE).
422+
423+"""FeatureFlagApplication allows access to information about feature flags."""
424+
425+__metaclass__ = type
426+__all__ = [
427+ 'IFeatureFlagApplication',
428+ 'FeatureFlagApplication',
429+ ]
430+
431+from zope.component import getUtility
432+from zope.interface import implements
433+
434+from canonical.launchpad.webapp.interfaces import ILaunchpadApplication
435+from lp.registry.interfaces.person import IPersonSet
436+from lp.services.features.flags import FeatureController
437+from lp.services.features.rulesource import StormFeatureRuleSource
438+from lp.services.features.scopes import (
439+ default_scopes,
440+ FixedScope,
441+ MultiScopeHandler,
442+ TeamScope,
443+ )
444+
445+
446+class IFeatureFlagApplication(ILaunchpadApplication):
447+ """Mailing lists application root."""
448+
449+ def getFeatureFlag(flag_name, username=None, scopes=()):
450+ """Return the value of the given feature flag.
451+
452+ :param flag_name: The name of the flag to query.
453+ :param username: If supplied, the name of a Person to use in
454+ evaluating the 'team:' scope.
455+ :param scopes: A list of scopes to consider active. The 'default'
456+ scope is always considered to be active, and does not need to be
457+ included here.
458+ """
459+
460+
461+class FeatureFlagApplication:
462+
463+ implements(IFeatureFlagApplication)
464+
465+ def getFeatureFlag(self, flag_name, active_scopes=()):
466+ scopes = list(default_scopes)
467+ for scope_name in active_scopes:
468+ if scope_name.startswith('user:'):
469+ person = getUtility(IPersonSet).getByName(
470+ scope_name[len('user:'):])
471+ if person is not None:
472+ scopes.append(TeamScope(person))
473+ else:
474+ scopes.append(FixedScope(scope_name))
475+ flag_name = unicode(flag_name)
476+ controller = FeatureController(
477+ MultiScopeHandler(scopes).lookup, StormFeatureRuleSource())
478+ return controller.getFlag(flag_name)
479
480=== modified file 'lib/lp/testing/factory.py'
481--- lib/lp/testing/factory.py 2011-09-16 18:33:57 +0000
482+++ lib/lp/testing/factory.py 2011-09-19 04:10:29 +0000
483@@ -505,7 +505,7 @@
484 for any other required objects.
485 """
486
487- def loginAsAnyone(self):
488+ def loginAsAnyone(self, participation=None):
489 """Log in as an arbitrary person.
490
491 If you want to log in as a celebrity, including admins, see
492@@ -513,7 +513,7 @@
493 """
494 login(ANONYMOUS)
495 person = self.makePerson()
496- login_as(person)
497+ login_as(person, participation)
498 return person
499
500 @with_celebrity_logged_in('admin')
501
502=== modified file 'utilities/page-performance-report.ini'
503--- utilities/page-performance-report.ini 2011-04-28 21:52:17 +0000
504+++ utilities/page-performance-report.ini 2011-09-19 04:10:29 +0000
505@@ -11,7 +11,8 @@
506 [^/]+($|/
507 (?!\+haproxy|\+opstats|\+access-token
508 |((authserver|bugs|bazaar|codehosting|
509- codeimportscheduler|mailinglists|softwarecenteragent)/\w+$)))
510+ codeimportscheduler|mailinglists|softwarecenteragent|
511+ featureflags)/\w+$)))
512 Other=^/
513
514 Launchpad Frontpage=^https?://launchpad\.[^/]+(/index\.html)?$
515@@ -54,7 +55,7 @@
516 Private XML-RPC=^https://(launchpad|xmlrpc)[^/]+/
517 (authserver|bugs|codehosting|
518 codeimportscheduler|mailinglists|
519- softwarecenteragent)/\w+$
520+ softwarecenteragent|featureflags)/\w+$
521
522 [metrics]
523 ppr_all=All Launchpad except operational pages