Merge pull request #447 from alphagov/ris-flask-0-12

Remove flask-featureflags integration, bump flask to v0.12.4

CHANGELOG.md

@@ -2,6 +2,23 @@
 
 Records breaking changes from major version bumps
 
+## 43.0.0
+
+PR [#447](https://github.com/alphagov/digitalmarketplace-utils/pull/447)
+
+This bump removes any handling of [FeatureFlags](https://pypi.org/project/Flask-FeatureFlags/) (in e.g. app init code)
+and removes FeatureFlags as a dependency.
+
+Specifically, `dmutils.flask_init.init_app(...)` no longer accepts a `feature_flags` argument and performs no
+initialization of FeatureFlags for the app.
+
+`dmutils.status.enabled_since(...)` has been removed.
+
+`dmutils.status.get_app_status(...)` no longer adds a `flags` key to its json dictionary.
+
+The dependency on Flask has been upgraded to Flask 0.12, so potentially apps are going to have to make changes
+in concordance with http://flask.pocoo.org/docs/0.12/changelog/
+
 ## 42.0.0
 
 PR [#400](https://github.com/alphagov/digitalmarketplace-utils/pull/400)

dmutils/__init__.py

@@ -1,7 +1,5 @@
 from . import config, formats, logging, proxy_fix, request_id
 from .flask_init import init_app, init_manager
 
-import flask_featureflags  # noqa
 
-
-__version__ = '42.8.2'
+__version__ = '43.0.0'

dmutils/errors.py

@@ -1,23 +1,15 @@
 from flask import redirect, render_template, url_for, flash, session, request, current_app
-from flask_wtf.csrf import CSRFError
 from jinja2.exceptions import TemplateNotFound
 
 
-def csrf_handler(e):
+def csrf_handler(csrf_error):
     """
-    Workaround for a bug in Flask 0.10.1.
-    CSRFErrors are caught under 400 BadRequest exceptions, so this heavy-handed solution
-     catches all 400s, and immediately discards non-CSRFError instances.
-
-    :param e: CSRF exception instance
+    :param csrf_error: CSRF exception instance
     :param session: Flask session instance
     :param request: Flask request instance
     :param logger: app logger instance
     :return: redirect to login with flashed error message
     """
-    if not isinstance(e, CSRFError):
-        return render_error_page(e)
-
     if 'user_id' not in session:
         current_app.logger.info(
             u'csrf.session_expired: Redirecting user to log in page'
@@ -29,7 +21,7 @@ def csrf_handler(e):
         )
 
     flash('Your session has expired. Please log in again.', "error")
-    return redirect_to_login(e)
+    return redirect_to_login(csrf_error)
 
 
 def redirect_to_login(e):

dmutils/flask_init.py

@@ -1,7 +1,7 @@
 import os
-from flask_featureflags.contrib.inline import InlineFeatureFlag
 from . import config, logging, proxy_fix, request_id, formats, filters, errors
 from flask_script import Manager, Server
+from flask_wtf.csrf import CSRFError
 
 
 def init_app(
@@ -10,7 +10,6 @@ def init_app(
         bootstrap=None,
         data_api_client=None,
         db=None,
-        feature_flags=None,
         login_manager=None,
         search_api_client=None,
 ):
@@ -31,11 +30,6 @@ def init_app(
         data_api_client.init_app(application)
     if db:
         db.init_app(application)
-    if feature_flags:
-        # Standardize FeatureFlags, only accept inline config variables
-        feature_flags.init_app(application)
-        feature_flags.clear_handlers()
-        feature_flags.add_handler(InlineFeatureFlag())
     if login_manager:
         login_manager.init_app(application)
     if search_api_client:
@@ -68,7 +62,8 @@ def inject_global_template_variables():
             **(application.config['BASE_TEMPLATE_DATA'] or {}))
 
     # Register error handlers for CSRF errors and common error status codes
-    application.register_error_handler(400, errors.csrf_handler)
+    application.register_error_handler(CSRFError, errors.csrf_handler)
+    application.register_error_handler(400, errors.render_error_page)
     application.register_error_handler(401, errors.redirect_to_login)
     application.register_error_handler(403, errors.redirect_to_login)
     application.register_error_handler(404, errors.render_error_page)

dmutils/status.py

@@ -1,9 +1,6 @@
-import datetime
 import math
 import os
-from .formats import DATE_FORMAT
 from flask import jsonify, current_app
-from flask_featureflags import FEATURE_FLAGS_CONFIG
 
 
 def get_version_label(path):
@@ -15,19 +12,6 @@ def get_version_label(path):
         return None
 
 
-def get_flags(current_app):
-    """ Loop through config variables and return a dictionary of flags.  """
-    flags = {}
-
-    for config_var in current_app.config.keys():
-        # Check that the (inline) key starts with our config variable
-        if config_var.startswith("{}_".format(FEATURE_FLAGS_CONFIG)):
-
-                flags[config_var] = current_app.config[config_var]
-
-    return flags
-
-
 def get_disk_space_status(low_disk_percent_threshold=5):
     """Accepts a single parameter that indicates the minimum percentage of disk space which should be free for the
     instance to be considered healthy.
@@ -41,15 +25,6 @@ def get_disk_space_status(low_disk_percent_threshold=5):
     return 'OK' if disk_free_percent >= low_disk_percent_threshold else 'LOW', disk_free_percent
 
 
-def enabled_since(date_string):
-    if date_string:
-        # Check format like YYYY-MM-DD
-        datetime.datetime.strptime(date_string, DATE_FORMAT)
-        return date_string
-
-    return False
-
-
 class StatusError(Exception):
     """A stub class to use when implementing additional checks for an app's _status endpoint. See API for example.
     When raising a StatusError, make sure that the message passed in uniquely identifies the additional check you are
@@ -87,7 +62,6 @@ def get_app_status(data_api_client=None,
 
     if not ignore_dependencies:
         response_data['version'] = current_app.config['VERSION']
-        response_data['flags'] = get_flags(current_app)
 
         if data_api_client:
             data_api_status = data_api_client.get_status() or {'status': 'n/a'}

requirements.txt

@@ -1,5 +1 @@
 -e .
-
-# Dependencies not on PyPI must be listed here as well as in setup.py
-git+https://github.com/alphagov/Flask-FeatureFlags.git@1.0#egg=Flask-FeatureFlags==1.0
-

setup.py

@@ -23,10 +23,9 @@
     packages=find_packages(),
     include_package_data=True,
     install_requires=[
-         'Flask-FeatureFlags==1.0',
          'Flask-Script==2.0.6',
          'Flask-WTF==0.14.2',
-         'Flask==0.10.1',
+         'Flask==0.12.4',
          'Flask-Login>=0.2.11',
          'boto3==1.4.8',
          'contextlib2==0.4.0',

tests/test_errors.py

@@ -38,19 +38,6 @@ def test_csrf_handler_redirects_to_login(current_app, user_session, app):
             ]
 
 
-@mock.patch('dmutils.errors.render_template')
-def test_csrf_handler_sends_other_400s_to_render_error_page(render_template, app):
-
-    with app.test_request_context('/'):
-        app.config['WTF_CSRF_ENABLED'] = True
-        app.register_blueprint(external_blueprint)
-
-        assert csrf_handler(BadRequest()) == (render_template.return_value, 400)
-        assert render_template.call_args_list == [
-            mock.call('errors/400.html', error_message=None)
-        ]
-
-
 def test_unauthorised_redirects_to_login(app):
     with app.test_request_context('/'):
         app.register_blueprint(external_blueprint)

tests/test_status.py

@@ -50,7 +50,7 @@ def additional_check(key, value):
                                  None,
                                  False,
                                  [],
-                                 {'flags': {}, 'status': 'ok', 'version': 'release-0', 'disk': 'OK (90% free)'},
+                                 {'status': 'ok', 'version': 'release-0', 'disk': 'OK (90% free)'},
                                  200
                              ),
                              (  # Test case #2 - data api client OK, ignore_deps = False, result: 200
@@ -59,7 +59,7 @@ def additional_check(key, value):
                                  None,
                                  False,
                                  [],
-                                 {'flags': {}, 'api_status': {'status': 'ok'}, 'status': 'ok', 'version': 'release-0',
+                                 {'api_status': {'status': 'ok'}, 'status': 'ok', 'version': 'release-0',
                                   'disk': 'OK (90% free)'},
                                  200
                              ),
@@ -69,7 +69,7 @@ def additional_check(key, value):
                                  'ok',
                                  False,
                                  [],
-                                 {'flags': {}, 'search_api_status': {'status': 'ok'}, 'status': 'ok',
+                                 {'search_api_status': {'status': 'ok'}, 'status': 'ok',
                                   'version': 'release-0', 'disk': 'OK (90% free)'},
                                  200
                              ),
@@ -79,7 +79,7 @@ def additional_check(key, value):
                                  'ok',
                                  False,
                                  [],
-                                 {'flags': {}, 'api_status': {'status': 'ok'}, 'search_api_status': {'status': 'ok'},
+                                 {'api_status': {'status': 'ok'}, 'search_api_status': {'status': 'ok'},
                                   'status': 'ok', 'version': 'release-0', 'disk': 'OK (90% free)'},
                                  200
                              ),
@@ -90,7 +90,7 @@ def additional_check(key, value):
                                  False,
                                  [additional_check('k', 'v'), additional_check('k2', 'v2')],
                                  {
-                                     'flags': {}, 'api_status': {'status': 'ok'}, 'search_api_status': {'status': 'ok'},
+                                     'api_status': {'status': 'ok'}, 'search_api_status': {'status': 'ok'},
                                      'status': 'ok', 'version': 'release-0', 'disk': 'OK (90% free)',
                                      'k': 'v', 'k2': 'v2'
                                  },
@@ -111,8 +111,7 @@ def additional_check(key, value):
                                  'ok',
                                  False,
                                  [],
-                                 {'flags': {},
-                                  'api_status': {'status': 'error'},
+                                 {'api_status': {'status': 'error'},
                                   'search_api_status': {'status': 'ok'},
                                   'status': 'error',
                                   'version': 'release-0',
@@ -126,8 +125,7 @@ def additional_check(key, value):
                                  'error',
                                  False,
                                  [],
-                                 {'flags': {},
-                                  'api_status': {'status': 'error'},
+                                 {'api_status': {'status': 'error'},
                                   'search_api_status': {'status': 'error'},
                                   'status': 'error',
                                   'version': 'release-0',