From 24d139534fb0b33f4e50dca9d90dce7cf054cbf2 Mon Sep 17 00:00:00 2001 From: Sergi Blanco-Cuaresma Date: Fri, 29 Dec 2017 12:23:58 +0100 Subject: [PATCH 1/9] Added ADSFlask --- adsws/factory.py | 65 ++++++++++++++++++------------------------------ requirements.txt | 1 + 2 files changed, 25 insertions(+), 41 deletions(-) diff --git a/adsws/factory.py b/adsws/factory.py index 04c4f4c..647f81a 100644 --- a/adsws/factory.py +++ b/adsws/factory.py @@ -12,7 +12,7 @@ import logging.handlers from werkzeug.contrib.fixers import ProxyFix -from flask import Flask, g, request, jsonify, session +from flask import g, request, jsonify, session from flask.ext.login import current_user from flask.ext.sslify import SSLify from flask.ext.consulate import Consul, ConsulConnectionError @@ -22,11 +22,13 @@ from flask_registry import Registry, ExtensionRegistry, \ PackageRegistry, ConfigurationRegistry, BlueprintAutoDiscoveryRegistry +from adsmutils import ADSFlask + from .middleware import HTTPMethodOverrideMiddleware def create_app(app_name=None, instance_path=None, static_path=None, - static_folder=None, **kwargs_config): + static_folder=None, **config): """Returns a :class:`Flask` application instance configured with common functionality for the AdsWS platform. @@ -34,7 +36,7 @@ def create_app(app_name=None, instance_path=None, static_path=None, :param instance_path: application package path :param static_path: flask.Flask static_path kwarg :param static_folder: flask.Flask static_folder kwarg - :param kwargs_config: a dictionary of settings to override + :param config: a dictionary of settings to override """ # Flask application name app_name = app_name or '.'.join(__name__.split('.')[0:-1]) @@ -51,19 +53,27 @@ def create_app(app_name=None, instance_path=None, static_path=None, except: pass - app = Flask( - app_name, - instance_path=instance_path, - instance_relative_config=False, - static_path=static_path, - static_folder=static_folder - ) + if config: + app = ADSFlask( + app_name, + instance_path=instance_path, + instance_relative_config=False, + static_path=static_path, + static_folder=static_folder, + local_config=config + ) + else: + app = ADSFlask( + app_name, + instance_path=instance_path, + instance_relative_config=False, + static_path=static_path, + static_folder=static_folder + ) # Handle both URLs with and without trailing slashes by Flask. app.url_map.strict_slashes = False - load_config(app, kwargs_config) - # Ensure SECRET_KEY has a value in the application configuration register_secret_key(app) @@ -89,7 +99,7 @@ def create_app(app_name=None, instance_path=None, static_path=None, # takes precedence) ConfigurationRegistry(app) - configure_logging(app) + configure_a_more_verbose_log_exception(app) app.wsgi_app = HTTPMethodOverrideMiddleware(app.wsgi_app) @@ -235,14 +245,9 @@ def register_secret_key(app): warnings.warn("Using insecure dev SECRET_KEY", UserWarning) -def configure_logging(app): +def configure_a_more_verbose_log_exception(app): """Configure logging.""" - try: - from cloghandler import ConcurrentRotatingFileHandler as RotatingFileHandler - except ImportError: - RotatingFileHandler = logging.handlers.RotatingFileHandler - def log_exception(exc_info): """ Override default Flask.log_exception for more verbose logging on @@ -273,28 +278,6 @@ def log_exception(exc_info): ) app.log_exception = log_exception - fn = app.config.get('LOG_FILE') - if fn is None: - fn = os.path.join(app.instance_path, 'logs/adsws.log') - - if not os.path.exists(os.path.dirname(fn)): - os.makedirs(os.path.dirname(fn)) - - rfh = RotatingFileHandler(fn, maxBytes=1000000, backupCount=10) - rfh.setFormatter(logging.Formatter( - '%(asctime)s %(levelname)s: %(message)s ' - '[in %(pathname)s:%(lineno)d]') - ) - # NOTE: - # Setting the level on just the handler seems to have *no* effect; - # setting the level on app.logger seems to have the desired effect. - # I do not understand this behavior - # rfh.setLevel(app.config.get('LOG_LEVEL', logging.INFO)) - app.logger.setLevel((app.config.get('LOG_LEVEL', logging.INFO))) - if rfh not in app.logger.handlers: - app.logger.addHandler(rfh) - app.logger.debug("Logging initialized") - def make_session_permanent(): """ diff --git a/requirements.txt b/requirements.txt index 0721799..b693153 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,4 @@ +git+https://github.com/adsabs/ADSMicroserviceUtils.git@master flask-headers==1.0 git+https://github.com/adsabs/flask-limiter.git@968a0faaa6fce47bf3fb7928ca2b10a00917d6eb#egg=flask-limiter flask-cors==1.9.0 From f562e04d1c07584295fcfc316621c92004e0d26a Mon Sep 17 00:00:00 2001 From: Sergi Blanco-Cuaresma Date: Tue, 2 Jan 2018 14:38:56 +0100 Subject: [PATCH 2/9] Bugfix: load all the necessary config files --- adsws/factory.py | 70 ++++-------------------------------- adsws/config.py => config.py | 0 2 files changed, 6 insertions(+), 64 deletions(-) rename adsws/config.py => config.py (100%) diff --git a/adsws/factory.py b/adsws/factory.py index 647f81a..c320381 100644 --- a/adsws/factory.py +++ b/adsws/factory.py @@ -74,6 +74,12 @@ def create_app(app_name=None, instance_path=None, static_path=None, # Handle both URLs with and without trailing slashes by Flask. app.url_map.strict_slashes = False + # Load 'adsws/account/config.py' + try: + app.config.from_object('%s.config' % app.name) + except (IOError, ImportError): + app.logger.warning("Could not load object {}.config".format(app.name)) + # Ensure SECRET_KEY has a value in the application configuration register_secret_key(app) @@ -160,70 +166,6 @@ def on_405(e): return jsonify(dict(error='Method not allowed')), 405 -def load_config(app, kwargs_config): - """ - writes to app.config heiracharchly based on files on disk and consul - :param app: flask.Flask application instance - :param kwargs_config: dictionary to update the config - :return: None - """ - - try: - app.config.from_object('adsws.config') - except (IOError, ImportError): - app.logger.warning("Could not load object adsws.config") - try: - app.config.from_object('%s.config' % app.name) - except (IOError, ImportError): - app.logger.warning("Could not load object {}.config".format(app.name)) - - try: - f = os.path.join(app.instance_path, 'config.py') - if os.path.exists(f): - app.config.from_pyfile(f) - except IOError: - app.logger.warning("Could not load {}".format(f)) - - try: - f = os.path.join(app.instance_path, 'local_config.py') - if os.path.exists(f): - app.config.from_pyfile(f) - except IOError: - app.logger.warning("Could not load {}".format(f)) - - try: - f = os.path.join(app.instance_path, '%s.local_config.py' % app.name) - if os.path.exists(f): - app.config.from_pyfile(f) - except IOError: - app.logger.warning("Could not load {}".format(f)) - - try: - consul = Consul(app) - consul.apply_remote_config() - except ConsulConnectionError: - app.logger.warning( - "Could not load config from consul at {}".format( - os.environ.get('CONSUL_HOST', 'localhost') - ) - ) - - if kwargs_config: - app.config.update(kwargs_config) - - # old baggage... Consul used to store keys in hexadecimal form - # so the production/staging databases both convert that into raw bytes - # but those raw bytes were non-ascii chars (unsafe to pass through - # env vars). So we must continue converting hex ... - if app.config.get('SECRET_KEY', None): - try: - app.config['SECRET_KEY'] = app.config['SECRET_KEY'].decode('hex') - app.logger.warning('Converted SECRET_KEY from hex format into bytes') - except TypeError: - app.logger.warning('Most likely the SECRET_KEY is not in hex format') - - - def set_translations(): """Add under ``g._`` an already configured internationalization function. diff --git a/adsws/config.py b/config.py similarity index 100% rename from adsws/config.py rename to config.py From 032bf2c348c1088c8fae9dd6e2e1bd181d52d569 Mon Sep 17 00:00:00 2001 From: Sergi Blanco-Cuaresma Date: Tue, 2 Jan 2018 14:43:25 +0100 Subject: [PATCH 3/9] Upgraded postgres library to fix travis problem --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index b693153..9569ecd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,7 +5,7 @@ flask-cors==1.9.0 flask-RESTful==0.3.4 flask-sslify==0.1.5 Flask==0.11.1 -psycopg2==2.6.1 +psycopg2==2.7.3.2 Flask-SQLAlchemy==2.1 Flask-login==0.2.11 Flask-Security==1.7.4 From 573964bb64e884d5cb055857bcae4808d8a4fd81 Mon Sep 17 00:00:00 2001 From: Sergi Blanco-Cuaresma Date: Tue, 2 Jan 2018 16:02:50 +0100 Subject: [PATCH 4/9] Ensure good order when loading config files --- adsws/factory.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/adsws/factory.py b/adsws/factory.py index c320381..c27bd68 100644 --- a/adsws/factory.py +++ b/adsws/factory.py @@ -74,9 +74,14 @@ def create_app(app_name=None, instance_path=None, static_path=None, # Handle both URLs with and without trailing slashes by Flask. app.url_map.strict_slashes = False - # Load 'adsws/account/config.py' + # Load 'adsws/XXXX/config.py' but give preference to values loaded from + # main config file as loaded by ADSFlask + import flask try: - app.config.from_object('%s.config' % app.name) + config = flask.config.Config(app.config['PROJ_HOME']) + config.from_object('%s.config' % app.name) + config.update(app.config) + app.config = config except (IOError, ImportError): app.logger.warning("Could not load object {}.config".format(app.name)) From 994ac1f0d2f2fdd96327437e8f64be07b0d85b36 Mon Sep 17 00:00:00 2001 From: Sergi Blanco-Cuaresma Date: Wed, 3 Jan 2018 10:31:51 +0100 Subject: [PATCH 5/9] Recover load_config for testing --- adsws/factory.py | 84 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 74 insertions(+), 10 deletions(-) diff --git a/adsws/factory.py b/adsws/factory.py index c27bd68..148bb38 100644 --- a/adsws/factory.py +++ b/adsws/factory.py @@ -74,16 +74,17 @@ def create_app(app_name=None, instance_path=None, static_path=None, # Handle both URLs with and without trailing slashes by Flask. app.url_map.strict_slashes = False - # Load 'adsws/XXXX/config.py' but give preference to values loaded from - # main config file as loaded by ADSFlask - import flask - try: - config = flask.config.Config(app.config['PROJ_HOME']) - config.from_object('%s.config' % app.name) - config.update(app.config) - app.config = config - except (IOError, ImportError): - app.logger.warning("Could not load object {}.config".format(app.name)) + load_config(app, config) + ## Load 'adsws/XXXX/config.py' but give preference to values loaded from + ## main config file as loaded by ADSFlask + #import flask + #try: + #config = flask.config.Config(app.config['PROJ_HOME']) + #config.from_object('%s.config' % app.name) + #config.update(app.config) + #app.config = config + #except (IOError, ImportError): + #app.logger.warning("Could not load object {}.config".format(app.name)) # Ensure SECRET_KEY has a value in the application configuration register_secret_key(app) @@ -171,6 +172,69 @@ def on_405(e): return jsonify(dict(error='Method not allowed')), 405 +def load_config(app, kwargs_config): + """ + writes to app.config heiracharchly based on files on disk and consul + :param app: flask.Flask application instance + :param kwargs_config: dictionary to update the config + :return: None + """ + + try: + app.config.from_object('adsws.config') + except (IOError, ImportError): + app.logger.warning("Could not load object adsws.config") + try: + app.config.from_object('%s.config' % app.name) + except (IOError, ImportError): + app.logger.warning("Could not load object {}.config".format(app.name)) + + try: + f = os.path.join(app.instance_path, 'config.py') + if os.path.exists(f): + app.config.from_pyfile(f) + except IOError: + app.logger.warning("Could not load {}".format(f)) + + try: + f = os.path.join(app.instance_path, 'local_config.py') + if os.path.exists(f): + app.config.from_pyfile(f) + except IOError: + app.logger.warning("Could not load {}".format(f)) + + try: + f = os.path.join(app.instance_path, '%s.local_config.py' % app.name) + if os.path.exists(f): + app.config.from_pyfile(f) + except IOError: + app.logger.warning("Could not load {}".format(f)) + + try: + consul = Consul(app) + consul.apply_remote_config() + except ConsulConnectionError: + app.logger.warning( + "Could not load config from consul at {}".format( + os.environ.get('CONSUL_HOST', 'localhost') + ) + ) + + if kwargs_config: + app.config.update(kwargs_config) + + # old baggage... Consul used to store keys in hexadecimal form + # so the production/staging databases both convert that into raw bytes + # but those raw bytes were non-ascii chars (unsafe to pass through + # env vars). So we must continue converting hex ... + if app.config.get('SECRET_KEY', None): + try: + app.config['SECRET_KEY'] = app.config['SECRET_KEY'].decode('hex') + app.logger.warning('Converted SECRET_KEY from hex format into bytes') + except TypeError: + app.logger.warning('Most likely the SECRET_KEY is not in hex format') + + def set_translations(): """Add under ``g._`` an already configured internationalization function. From e2688f0d54295f08ef34cff151a41bc774ab0937 Mon Sep 17 00:00:00 2001 From: Sergi Blanco-Cuaresma Date: Wed, 3 Jan 2018 11:20:32 +0100 Subject: [PATCH 6/9] Reorganized load_config to change priorities --- adsws/factory.py | 71 ++++++++++++++++-------------------------------- 1 file changed, 24 insertions(+), 47 deletions(-) diff --git a/adsws/factory.py b/adsws/factory.py index 148bb38..b0cc452 100644 --- a/adsws/factory.py +++ b/adsws/factory.py @@ -75,16 +75,6 @@ def create_app(app_name=None, instance_path=None, static_path=None, app.url_map.strict_slashes = False load_config(app, config) - ## Load 'adsws/XXXX/config.py' but give preference to values loaded from - ## main config file as loaded by ADSFlask - #import flask - #try: - #config = flask.config.Config(app.config['PROJ_HOME']) - #config.from_object('%s.config' % app.name) - #config.update(app.config) - #app.config = config - #except (IOError, ImportError): - #app.logger.warning("Could not load object {}.config".format(app.name)) # Ensure SECRET_KEY has a value in the application configuration register_secret_key(app) @@ -171,54 +161,41 @@ def on_429(e): def on_405(e): return jsonify(dict(error='Method not allowed')), 405 +def __load_config(app, method_name, method_argument): + # Load config but give preference to values loaded from + # main config file as loaded by ADSFlask + import flask + try: + config = flask.config.Config(app.config['PROJ_HOME']) + method_to_call = getattr(config, method_name) + method_to_call(method_argument) + config.update(app.config) + app.config = config + except (IOError, ImportError): + app.logger.warning("Could not load object {}.config".format(app.name)) def load_config(app, kwargs_config): """ - writes to app.config heiracharchly based on files on disk and consul + writes to app.config heiracharchly based on files on disk :param app: flask.Flask application instance :param kwargs_config: dictionary to update the config :return: None """ - try: - app.config.from_object('adsws.config') - except (IOError, ImportError): - app.logger.warning("Could not load object adsws.config") - try: - app.config.from_object('%s.config' % app.name) - except (IOError, ImportError): - app.logger.warning("Could not load object {}.config".format(app.name)) - - try: - f = os.path.join(app.instance_path, 'config.py') - if os.path.exists(f): - app.config.from_pyfile(f) - except IOError: - app.logger.warning("Could not load {}".format(f)) + __load_config(app, 'from_object', 'adsws.config') + __load_config(app, 'from_object', '%s.config' % app.name) - try: - f = os.path.join(app.instance_path, 'local_config.py') - if os.path.exists(f): - app.config.from_pyfile(f) - except IOError: - app.logger.warning("Could not load {}".format(f)) + f = os.path.join(app.instance_path, 'config.py') + if os.path.exists(f): + __load_config(app, 'from_pyfile', f) - try: - f = os.path.join(app.instance_path, '%s.local_config.py' % app.name) - if os.path.exists(f): - app.config.from_pyfile(f) - except IOError: - app.logger.warning("Could not load {}".format(f)) + f = os.path.join(app.instance_path, 'local_config.py') + if os.path.exists(f): + __load_config(app, 'from_pyfile', f) - try: - consul = Consul(app) - consul.apply_remote_config() - except ConsulConnectionError: - app.logger.warning( - "Could not load config from consul at {}".format( - os.environ.get('CONSUL_HOST', 'localhost') - ) - ) + f = os.path.join(app.instance_path, '%s.local_config.py' % app.name) + if os.path.exists(f): + __load_config(app, 'from_pyfile', f) if kwargs_config: app.config.update(kwargs_config) From f1b58db96aa89cdb6d26ca10059a52666492fc27 Mon Sep 17 00:00:00 2001 From: Sergi Blanco-Cuaresma Date: Wed, 17 Jan 2018 15:17:21 +0100 Subject: [PATCH 7/9] Added decorator to use the right app context for imported views --- adsws/api/discoverer/utils.py | 15 +++++++++++++++ adsws/factory.py | 3 +-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/adsws/api/discoverer/utils.py b/adsws/api/discoverer/utils.py index 499ce0a..595221f 100644 --- a/adsws/api/discoverer/utils.py +++ b/adsws/api/discoverer/utils.py @@ -10,7 +10,19 @@ from importlib import import_module from adsws.ext.ratelimiter import ratelimit, limit_func, scope_func, key_func from flask.ext.consulate import ConsulService +from functools import wraps +def local_app_context(local_app): + """ + Ensure that an imported view is run under the correct application context + """ + def real_local_app_context_decorator(f): + @wraps(f) + def decorated_function(*args, **kwargs): + with local_app.app_context(): + return f(*args, **kwargs) + return decorated_function + return real_local_app_context_decorator def bootstrap_local_module(service_uri, deploy_path, app): """ @@ -46,6 +58,9 @@ def bootstrap_local_module(service_uri, deploy_path, app): else: attr_base = view + # ensure the current_app matches local_app and not API app + view = local_app_context(local_app)(view) + # Decorate the view with ratelimit if hasattr(attr_base, 'rate_limit'): d = attr_base.rate_limit[0] diff --git a/adsws/factory.py b/adsws/factory.py index b0cc452..b31ca61 100644 --- a/adsws/factory.py +++ b/adsws/factory.py @@ -172,7 +172,7 @@ def __load_config(app, method_name, method_argument): config.update(app.config) app.config = config except (IOError, ImportError): - app.logger.warning("Could not load object {}.config".format(app.name)) + app.logger.warning("Could not load config (%s): %s", method_name, method_argument) def load_config(app, kwargs_config): """ @@ -182,7 +182,6 @@ def load_config(app, kwargs_config): :return: None """ - __load_config(app, 'from_object', 'adsws.config') __load_config(app, 'from_object', '%s.config' % app.name) f = os.path.join(app.instance_path, 'config.py') From eafd21659925df3373f50561710daa74046a7150 Mon Sep 17 00:00:00 2001 From: Sergi Blanco-Cuaresma Date: Fri, 19 Jan 2018 16:33:45 +0100 Subject: [PATCH 8/9] Added specific adsmutils version --- adsws/factory.py | 32 +++++++++++++++----------------- requirements.txt | 6 +----- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/adsws/factory.py b/adsws/factory.py index b31ca61..03822d7 100644 --- a/adsws/factory.py +++ b/adsws/factory.py @@ -53,23 +53,14 @@ def create_app(app_name=None, instance_path=None, static_path=None, except: pass - if config: - app = ADSFlask( - app_name, - instance_path=instance_path, - instance_relative_config=False, - static_path=static_path, - static_folder=static_folder, - local_config=config - ) - else: - app = ADSFlask( - app_name, - instance_path=instance_path, - instance_relative_config=False, - static_path=static_path, - static_folder=static_folder - ) + app = ADSFlask( + app_name, + instance_path=instance_path, + instance_relative_config=False, + static_path=static_path, + static_folder=static_folder, + local_config = config or {} + ) # Handle both URLs with and without trailing slashes by Flask. app.url_map.strict_slashes = False @@ -162,6 +153,13 @@ def on_405(e): return jsonify(dict(error='Method not allowed')), 405 def __load_config(app, method_name, method_argument): + """ + Load configuration into the application using the method + `from_object` or `from_pyfile` with the argument pointing + to a python object or a python filename. + """ + if method_name not in ("from_object", "from_pyfile"): + raise Exception("Unsupported method: '{}'".format(method_name)) # Load config but give preference to values loaded from # main config file as loaded by ADSFlask import flask diff --git a/requirements.txt b/requirements.txt index 9569ecd..2346a2b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -git+https://github.com/adsabs/ADSMicroserviceUtils.git@master +git+https://github.com/adsabs/ADSMicroserviceUtils.git@v1.0.1 flask-headers==1.0 git+https://github.com/adsabs/flask-limiter.git@968a0faaa6fce47bf3fb7928ca2b10a00917d6eb#egg=flask-limiter flask-cors==1.9.0 @@ -6,7 +6,6 @@ flask-RESTful==0.3.4 flask-sslify==0.1.5 Flask==0.11.1 psycopg2==2.7.3.2 -Flask-SQLAlchemy==2.1 Flask-login==0.2.11 Flask-Security==1.7.4 Flask-Menu==0.4.0 @@ -20,12 +19,9 @@ WTForms-Alchemy==0.13.3 Flask-Registry==0.2.0 Flask-Email==1.4.4 Flask-Mail==0.9.1 -SQLAlchemy==1.0.14 sqlalchemy-utils==0.27.7 Werkzeug==0.10.4 requests>=2.7.0 alembic==0.8.1 -six>=1.10.0 redis==2.10.5 -ConcurrentLogHandler==0.9.1 flask-consulate==0.1.2 From e0c0361d4d95382fc36dc3245a83fceb680870ed Mon Sep 17 00:00:00 2001 From: Sergi Blanco-Cuaresma Date: Fri, 19 Jan 2018 19:22:57 +0100 Subject: [PATCH 9/9] Upgraded adsmutils version that fixes a local_config bug --- adsws/factory.py | 2 +- requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/adsws/factory.py b/adsws/factory.py index 03822d7..cca8875 100644 --- a/adsws/factory.py +++ b/adsws/factory.py @@ -59,7 +59,7 @@ def create_app(app_name=None, instance_path=None, static_path=None, instance_relative_config=False, static_path=static_path, static_folder=static_folder, - local_config = config or {} + local_config=config or {} ) # Handle both URLs with and without trailing slashes by Flask. diff --git a/requirements.txt b/requirements.txt index 2346a2b..9b86b59 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -git+https://github.com/adsabs/ADSMicroserviceUtils.git@v1.0.1 +git+https://github.com/adsabs/ADSMicroserviceUtils.git@v1.0.2 flask-headers==1.0 git+https://github.com/adsabs/flask-limiter.git@968a0faaa6fce47bf3fb7928ca2b10a00917d6eb#egg=flask-limiter flask-cors==1.9.0