diff --git a/CHANGELOG.md b/CHANGELOG.md index 877a8187..b56107c5 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/dmutils/__init__.py b/dmutils/__init__.py index 6ff7c5e6..5c54ece3 100644 --- a/dmutils/__init__.py +++ b/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' diff --git a/dmutils/errors.py b/dmutils/errors.py index 1cde8bb1..fd223ffd 100644 --- a/dmutils/errors.py +++ b/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): diff --git a/dmutils/flask_init.py b/dmutils/flask_init.py index d34a302f..296bda0b 100644 --- a/dmutils/flask_init.py +++ b/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) diff --git a/dmutils/status.py b/dmutils/status.py index caf550ed..a7fd6f31 100644 --- a/dmutils/status.py +++ b/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'} diff --git a/requirements.txt b/requirements.txt index a376e74f..d6e1198b 100644 --- a/requirements.txt +++ b/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 - diff --git a/setup.py b/setup.py index 2798375c..778e29dc 100644 --- a/setup.py +++ b/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', diff --git a/tests/test_errors.py b/tests/test_errors.py index 5c08c349..32cbd22d 100644 --- a/tests/test_errors.py +++ b/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) diff --git a/tests/test_status.py b/tests/test_status.py index 70d82ead..c677d304 100644 --- a/tests/test_status.py +++ b/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',