Proper feature flags

conf/development.ini

@@ -22,13 +22,6 @@ es.host: http://localhost:9200
 
 h.autologin: True
 
-h.feature.accounts: True
-h.feature.api: True
-h.feature.claim: False
-h.feature.notification: False
-h.feature.queue: False
-h.feature.streamer: False
-
 mail.default_sender: "Annotation Daemon" <no-reply@localhost>
 
 pyramid.debug_all: True

conf/production.ini

@@ -17,14 +17,6 @@ use: egg:h
 #h.client_id:
 #h.client_secret:
 
-# Feature flags
-h.feature.accounts: True
-h.feature.api: True
-h.feature.claim: False
-h.feature.notification: True
-h.feature.queue: True
-h.feature.streamer: True
-
 # Mail server configuration -- see the pyramid_mailer documentation
 mail.default_sender: "Annotation Daemon" <no-reply@localhost>
 #mail.host: localhost

conf/testext.ini

@@ -1,13 +1,6 @@
 [app:main]
 use: egg:h
 
-h.feature.accounts: True
-h.feature.api: True
-h.feature.claim: True
-h.feature.notification: True
-h.feature.queue: True
-h.feature.streamer: True
-
 sqlalchemy.url: sqlite://
 
 webassets.base_dir: h:static

h/accounts/test/views_test.py

@@ -8,6 +8,7 @@
 from pyramid import httpexceptions
 from pyramid.testing import DummyRequest as _DummyRequest
 
+from h.conftest import DummyFeature
 from h.conftest import DummySession
 
 from h.accounts.views import ajax_form
@@ -20,8 +21,12 @@
 
 class DummyRequest(_DummyRequest):
     def __init__(self, *args, **kwargs):
-        # Add a dummy database session to the request
-        params = {'db': DummySession()}
+        params = {
+            # Add a dummy database session to the request
+            'db': DummySession(),
+            # Add a dummy feature flag querier to the request
+            'feature': DummyFeature(),
+        }
         params.update(kwargs)
         super(DummyRequest, self).__init__(*args, **params)
 
@@ -32,11 +37,6 @@ def __init__(self, **kwargs):
             setattr(self, k, kwargs[k])
 
 
-def configure(config):
-    config.registry.feature = MagicMock()
-    config.registry.feature.return_value = None
-
-
 # A fake version of colander.Invalid for use when testing validate_form
 class FakeInvalid(object):
     def __init__(self, errors):

h/accounts/views.py

@@ -473,7 +473,7 @@ def profile(self):
         model = {}
         if userid:
             model["email"] = User.get_by_id(request, userid).email
-        if request.registry.feature('notification'):
+        if request.feature('notification'):
             model['subscriptions'] = Subscriptions.get_subscriptions_for_uri(
                 userid)
         return {'model': model}

h/api/queue.py

@@ -10,6 +10,11 @@ def annotation(event):
     request = event.request
     annotation = event.annotation
     action = event.action
+
+    # We only publish these events to NSQ if the 'queue' feature is enabled.
+    if not request.feature('queue'):
+        return
+
     queue = request.get_queue_writer()
     data = {
         'action': action,

h/app.py

@@ -58,31 +58,21 @@ def create_app(global_config, **settings):
 
     config.add_tween('h.tweens.csrf_tween_factory')
 
-    if config.registry.feature('accounts'):
-        config.set_authentication_policy(session_authn)
-        config.set_authorization_policy(acl_authz)
-        config.include('h.accounts')
-
-    if config.registry.feature('api'):
-        api_app = create_api(settings)
-        api_view = wsgiapp2(api_app)
-        config.add_view(api_view, name='api', decorator=strip_vhm)
-        # Add the view again with the 'index' route name, otherwise it will
-        # not take precedence over the index when a virtual root is in use.
-        config.add_view(api_view, name='api', decorator=strip_vhm,
-                        route_name='index')
-
-    if config.registry.feature('claim'):
-        config.include('h.claim')
-
-    if config.registry.feature('queue'):
-        config.include('h.queue')
-
-    if config.registry.feature('streamer'):
-        config.include('h.streamer')
-
-    if config.registry.feature('notification'):
-        config.include('h.notification')
+    config.set_authentication_policy(session_authn)
+    config.set_authorization_policy(acl_authz)
+    config.include('h.accounts')
+    config.include('h.claim')
+    config.include('h.notification')
+    config.include('h.queue')
+    config.include('h.streamer')
+
+    api_app = create_api(settings)
+    api_view = wsgiapp2(api_app)
+    config.add_view(api_view, name='api', decorator=strip_vhm)
+    # Add the view again with the 'index' route name, otherwise it will
+    # not take precedence over the index when a virtual root is in use.
+    config.add_view(api_view, name='api', decorator=strip_vhm,
+                    route_name='index')
 
     return config.make_wsgi_app()
 
@@ -104,12 +94,9 @@ def create_api(global_config, **settings):
 
     config.include('h.auth')
     config.include('h.api.db')
+    config.include('h.api.queue')
     config.include('h.api.views')
 
-    if config.registry.feature('queue'):
-        config.include('h.queue')
-        config.include('h.api.queue')
-
     app = config.make_wsgi_app()
     app = permit_cors(app,
                       allow_headers=('Authorization',

h/assets.yaml

@@ -137,8 +137,11 @@ app_js:
       contents: h:static/scripts/app.coffee
       depends:
         - h:static/scripts/*.coffee
+        - h:static/scripts/features.js
         - h:static/scripts/directive/*.coffee
+        - h:static/scripts/directive/*.js
         - h:static/scripts/filter/*.coffee
+        - h:static/scripts/filter/*.js
     - config
 
 app_css:

h/claim/test/views_test.py

@@ -4,13 +4,25 @@
 
 import deform
 from pytest import raises
-from pyramid.testing import DummyRequest
+from pyramid.testing import DummyRequest as _DummyRequest
 from pyramid.httpexceptions import HTTPFound, HTTPNotFound
 
+from h.conftest import DummyFeature
+
 from ..views import claim_account
 from ..views import update_account
 
 
+class DummyRequest(_DummyRequest):
+    def __init__(self, *args, **kwargs):
+        params = {
+            # Add a dummy feature flag querier to the request
+            'feature': DummyFeature(),
+        }
+        params.update(kwargs)
+        super(DummyRequest, self).__init__(*args, **params)
+
+
 def test_claim_account_returns_form():
     request = _get_request()
 

h/claim/views.py

@@ -51,13 +51,17 @@ def _validate_request(request):
     Check that the passed request is appropriate for proceeding with account
     claim. Asserts that:
 
+    - the 'claim' feature is toggled on
     - no-one is logged in
     - the claim token is provided and authentic
     - the user referred to in the token exists
     - the user referred to in the token has not already claimed their account
 
     and raises for redirect or 404 otherwise.
     """
+    if not request.feature('claim'):
+        raise exc.HTTPNotFound()
+
     # If signed in, redirect to stream
     if request.authenticated_userid is not None:
         _perform_logged_in_redirect(request)

h/config.py

@@ -1,6 +1,5 @@
 import os
 import logging
-import re
 import urlparse
 
 from pyramid.settings import asbool
@@ -16,7 +15,6 @@ def settings_from_environment():
     _setup_db(settings)
     _setup_elasticsearch(settings)
     _setup_email(settings)
-    _setup_features(settings)
     _setup_nsqd(settings)
     _setup_redis(settings)
     _setup_secrets(settings)
@@ -114,18 +112,6 @@ def _setup_email(settings):
         settings['mail.port'] = mail_port
 
 
-def _setup_features(settings):
-    # Allow feature flag overrides from the environment. For example, to
-    # override the 'foo_bar' feature you can set the environment variable
-    # FEATURE_FOO_BAR.
-    patt = re.compile(r'FEATURE_([A-Z_]+)')
-    keys = [k for k in os.environ.iterkeys() if patt.match(k)]
-    for k in keys:
-        name = 'h.feature.{}'.format(patt.match(k).group(1).lower())
-        value = asbool(os.environ[k])
-        settings[name] = value
-
-
 def _setup_nsqd(settings):
     if 'NSQD_PORT' in os.environ:
         r_host = os.environ['NSQD_PORT_4150_TCP_ADDR']

h/conftest.py

@@ -13,6 +13,23 @@
 from h import db
 
 
+class DummyFeature(object):
+
+    """
+    A dummy feature flag looker-upper.
+
+    Because we're probably testing all feature-flagged functionality, this
+    feature client defaults every flag to *True*, which is the exact opposite
+    of what happens outside of testing.
+    """
+
+    def __init__(self):
+        self.flags = {}
+
+    def __call__(self, name, *args, **kwargs):
+        return self.flags.get(name, True)
+
+
 class DummySession(object):
 
     """
@@ -83,17 +100,6 @@ def destroy():
     return db.Session
 
 
-@pytest.fixture(autouse=True)
-def feature_flags(config):
-    class DummyFeatureClient(object):
-        def __init__(self):
-            self.flags = {}
-        def __call__(self, name, *args, **kwargs):
-            return self.flags.get(name, True)
-
-    config.registry.feature = DummyFeatureClient()
-
-
 @pytest.fixture()
 def mailer(config):
     from pyramid_mailer.interfaces import IMailer