feature flags need to self document

Reassigning to benji, hoping poolie hasn't started

In working on this issue I've found that feature flag names and scopes
aren't registered before use, therefore all possible feature flags and
scopes can not be listed given the system as it stands.

To complete this feature request we will need a registry of feature flag
names that specifies the allowable names and modify the code to raise an
exception when an unregistered feature flag is requested.

Scopes are generated by code (ScopesFromRequest.lookup()) so a similar
(but slightly more dynamic) registry would be required for them.

Is the above approach acceptable? Was there another approach in mind
when this bug was filed that I'm missing?

So a registry is one approach.

Another is to scan the code for flag names that are used. I'm kindof partial to this simply because of the strong schemaless design.

Its important - vital in fact - that we stay low key and easy to use though ;)

Scopes are currently documented in the scope controller docstring; exposing that more accessibly (e.g. publish to the wiki automatically) would possibly be sufficient, or a registry w/introspection would be fine too.

(Generally speaking feel free to steal any assigned bug from me. I hadn't written any code on this yet.)

They are not registered at the moment, and to fix this bug they need to be. There are of course various ways to do this in Python. One that I was thinking of was:

 * register them centrally
 * refer to them just by string name
 * raise an error or a warning if a name is used that was not previously registered

Does Launchpad already have a Registry concept like in bzr, or is there a Zope idiom we should use?

Please request a review from me (but don't block on getting an answer), and feel free to ping me if you want to talk.

On Wed, Jan 5, 2011 at 6:05 PM, Martin Pool <email address hidden> wrote:
> They are not registered at the moment, and to fix this bug they need to
> be.  There are of course various ways to do this in Python.  One that I
> was thinking of was:
>
>  * register them centrally
>  * refer to them just by string name
>  * raise an error or a warning if a name is used that was not previously registered

For flag names that's the approach I was favoring.

> Does Launchpad already have a Registry concept like in bzr, or is there
> a Zope idiom we should use?

Yep, the Zope component architecture can be used as a general registry
like this. When I want a registry of callable things the ZCA is where
I start.

The scopes registry will need to be a little more complicated, but it
won't be too bad.

> Please request a review from me (but don't block on getting an answer),
> and feel free to ping me if you want to talk.

Will do.
--
Benji York

On Wed, Jan 5, 2011 at 6:01 PM, Robert Collins
<email address hidden> wrote:
> So a registry is one approach.
>
> Another is to scan the code for flag names that are used. I'm kindof
> partial to this simply because of the strong schemaless design.

I considered that approach. A purely textual scan doesn't seem like it
will work because we get a lot of testing related false hits. Analysing
the AST seems like it would suffer from the same problem.

Do you have another approach or variant that would perform better?
--
Benji York

On Fri, Jan 7, 2011 at 4:18 AM, Benji York <email address hidden> wrote:
> On Wed, Jan 5, 2011 at 6:01 PM, Robert Collins
> <email address hidden> wrote:
>> So a registry is one approach.
>>
>> Another is to scan the code for flag names that are used. I'm kindof
>> partial to this simply because of the strong schemaless design.
>
> I considered that approach.  A purely textual scan doesn't seem like it
> will work because we get a lot of testing related false hits.  Analysing
> the AST seems like it would suffer from the same problem.
>
> Do you have another approach or variant that would perform better?

Well, the only false hits we should expect are those testing feature
flags themselves - and they are blacklistable.

A registry would be ok - I don't want to dictate implementation - but
lets not make it hard to work with. Uhm, what does that mean? We
shouldn't have to make changes in lp.services.features when we use a
new feature flag; we shouldn't have to make changes in
lp.services.features when we add a new component to lp. we shouldn't
need to fiddle with zcml in those cases either.

Not too that the bug I filed asks for the scopes and flags to be
listed in API doc, *not* for runtime warnings or other such things.

On Thu, Jan 6, 2011 at 1:36 PM, Robert Collins
<email address hidden> wrote:
> On Fri, Jan 7, 2011 at 4:18 AM, Benji York <email address hidden> wrote:
>> On Wed, Jan 5, 2011 at 6:01 PM, Robert Collins
>> <email address hidden> wrote:
>>> So a registry is one approach.
>>>
>>> Another is to scan the code for flag names that are used. I'm kindof
>>> partial to this simply because of the strong schemaless design.
>>
>> I considered that approach.  A purely textual scan doesn't seem like it
>> will work because we get a lot of testing related false hits.  Analysing
>> the AST seems like it would suffer from the same problem.
>>
>> Do you have another approach or variant that would perform better?
>
> Well, the only false hits we should expect are those testing feature
> flags themselves - and they are blacklistable.

Seems hacky to me -- which is saying something ;) -- but it may be our
only choice given my understanding of the below.

> A registry would be ok - I don't want to dictate implementation - but
> lets not make it hard to work with. Uhm, what does that mean?

1)
> We shouldn't have to make changes in lp.services.features when we use a
> new feature flag;

Easy enough.

2)
> we shouldn't have to make changes in lp.services.features when we add a new
> component to lp.

Makes sense.

3)
> we shouldn't need to fiddle with zcml in those cases either.

Darn. You got me there. I had planned on using the ZCA as the
registry, adding an entry when defining a new feature flag, like so:

  <utility
      provides="lp.services.features.interfaces.IFeatureFlag"
      name="hard_timeout" />

I had planned on doing something similar for scopes.

Other registry-based options I can see are having an "add_feature_flag"
function, but that would mean calling it at import time which is
generally a bad idea or a purely data-driven registry (list of tuples or
similar) in a centralized place which would seem to violate 1 (unless we
put it in a place other than lp.services.features, which seems odd).

I'm out of registry-based ideas that either aren't good ideas or don't
violate your three conditions.

> Not too that the bug I filed asks for the scopes and flags to be
> listed in API doc, *not* for runtime warnings or other such things.

I don't think I appreciate the full meaning of the above. Are you
saying a manually maintained (and therefore potentially incorrect)
textual description of the available flags and scopes work?
--
Benji York

Is the no-zcml really a useful constraint? We use the ZCA a lot already and that5 would really use a pervasive pattern.

Sure python registration feels more natural to a lot of people, and extending our use of martian would help there, especially for view registrations. But until then, I don't see any problems with using ZCML to register feature flags.

I think scopes are very amenable to using a pseudo registry (specifically a list of 'ScopeChecker' or some such objects, which provide docs and are consulted in-order. (order matters)). We expect the number of scope checkers to change very slowly: its basic infrastructure.

For flag names:value pairs though, we're expecting them to be a micro-tool, turning up and going away at high frequency.

What would we ideally show for a flag name? Modelling it a bit like a function makes sense to me:
 name : domain : impact
that is
publicrestrictedlibrarian : boolean : when true the service is enabled
hard_timeout : [0.0, 20.0) : sets the hard timeout to float(value)

These clearly cannot be extracted from simple uses of flags - but thats ok I think: as long as we can:
 - list the flags that are consulted
 - where we have descriptions of the domain and impact show that as well

It will be easy to see where we have forgotten to document a flag, and we will have a very low overhead minimum cost for adding flags (as we do today).

zcml registration is slow, a major component of appserver and test suite startup time and very inefficient at runtime.

On Thu, Jan 6, 2011 at 3:07 PM, Robert Collins
<email address hidden> wrote:
> zcml registration is slowa major component of appserver and test suite
> startup time and very inefficient at runtime.

I can attest to ZCML processing being a large component of LP (and other
large Z3 based apps I've worked with) start up time. I don't know how
much of that work is essential, but in this case, it's hard to imagine
that registering the flags using the ZCA would be measurably slower than
making a few function calls.

As for "runtime", if you mean looking up things in the ZCA registry,
lookups are highly optimized and the times I've benchmarked it (most
recently at the Prague sprint), amortized lookups have been very close
to the time to do a single dictionary lookup.

That being said, I think the ZCML discussion is moot. I plan on doing
the following for this issue:

A) implement a scopes registry (as a data structure) in
   lp.services.features
B) have a documentation-only feature flag registry of (name, domain,
   description).
C) There will be no enforcement of registered feature flags. If an
   unregistered feature flag is requested no exception will be raised.
D) Scope registration will be required and enforced.
E) On the through-the-web display of feature flags, domains, and
   documentation include a list of feature flags that have been seen
   during the process' lifetime for which there is no documentation
   available so as to help us know when we've forgotten to document a
   feature flag.

That leaves where the information should be displayed. The feature
flags configuration page seems like a good place since the information
will be near you when you're making changes, so I'll put them there or
have a prominent link to them from there.

Another question I have is that you said that order matters for scopes.
I think what you mean by that is that scopes should be looked up in a
consistent order to avoid variable run-time behavior if more than one
scope handler would claim a particular scope string. To enforce
consistently I'm going to store scope handlers in a list so they will be
consulted in a consistent order.
--
Benji York

Sounds good to me!

Fixed in stable r12269 <http://bazaar.launchpad.net/~launchpad-pqm/launchpad/stable/revision/12269>.