Merge pull request #393 from mterzo/clean_error

Handle server exception when unable to connect early.

.travis.yml

@@ -10,7 +10,7 @@ install:
   - pip install -r requirements-test.txt
   - pip install -q coveralls --use-wheel
 script:
-  - pytest
+  - pytest --pep8
   - if [ "${TRAVIS_PYTHON_VERSION}" != "2.6" ]; then
       pip install bandit;
       bandit -r puppetboard;

puppetboard/app.py

@@ -16,21 +16,22 @@
     request, session, jsonify
 )
 
-from pypuppetdb import connect
 from pypuppetdb.QueryBuilder import *
 
 from puppetboard.forms import QueryForm
-from puppetboard.utils import (
-    get_or_abort, yield_or_stop, get_db_version,
-    jsonprint, prettyprint
-)
+from puppetboard.utils import (get_or_abort, yield_or_stop,
+                               get_db_version)
 from puppetboard.dailychart import get_daily_reports_chart
 
 import werkzeug.exceptions as ex
 import CommonMark
 
+from puppetboard.core import get_app, get_puppetdb, environments
+import puppetboard.errors
+
 from . import __version__
 
+
 REPORTS_COLUMNS = [
     {'attr': 'end', 'filter': 'end_time',
      'name': 'End time', 'type': 'datetime'},
@@ -48,31 +49,15 @@
     {'attr': 'form', 'name': 'Compare'},
 ]
 
-app = Flask(__name__)
-
-app.config.from_object('puppetboard.default_settings')
+app = get_app()
 graph_facts = app.config['GRAPH_FACTS']
-app.config.from_envvar('PUPPETBOARD_SETTINGS', silent=True)
-graph_facts += app.config['GRAPH_FACTS']
-app.secret_key = app.config['SECRET_KEY']
-
-app.jinja_env.filters['jsonprint'] = jsonprint
-app.jinja_env.filters['prettyprint'] = prettyprint
-
-puppetdb = connect(
-    host=app.config['PUPPETDB_HOST'],
-    port=app.config['PUPPETDB_PORT'],
-    ssl_verify=app.config['PUPPETDB_SSL_VERIFY'],
-    ssl_key=app.config['PUPPETDB_KEY'],
-    ssl_cert=app.config['PUPPETDB_CERT'],
-    timeout=app.config['PUPPETDB_TIMEOUT'],)
-
 numeric_level = getattr(logging, app.config['LOGLEVEL'].upper(), None)
-if not isinstance(numeric_level, int):
-    raise ValueError('Invalid log level: %s' % app.config['LOGLEVEL'])
+
 logging.basicConfig(level=numeric_level)
 log = logging.getLogger(__name__)
 
+puppetdb = get_puppetdb()
+
 
 @app.template_global()
 def version():
@@ -87,29 +72,10 @@ def stream_template(template_name, **context):
     return rv
 
 
-def url_for_field(field, value):
-    args = request.view_args.copy()
-    args.update(request.args.copy())
-    args[field] = value
-    return url_for(request.endpoint, **args)
-
-
-def environments():
-    envs = get_or_abort(puppetdb.environments)
-    x = []
-
-    for env in envs:
-        x.append(env['name'])
-
-    return x
-
-
 def check_env(env, envs):
     if env != '*' and env not in envs:
         abort(404)
 
-app.jinja_env.globals['url_for_field'] = url_for_field
-
 
 @app.context_processor
 def utility_processor():
@@ -119,38 +85,6 @@ def now(format='%m/%d/%Y %H:%M:%S'):
     return dict(now=now)
 
 
-@app.errorhandler(400)
-def bad_request(e):
-    envs = environments()
-    return render_template('400.html', envs=envs), 400
-
-
-@app.errorhandler(403)
-def forbidden(e):
-    envs = environments()
-    return render_template('403.html', envs=envs), 403
-
-
-@app.errorhandler(404)
-def not_found(e):
-    envs = environments()
-    return render_template('404.html', envs=envs), 404
-
-
-@app.errorhandler(412)
-def precond_failed(e):
-    """We're slightly abusing 412 to handle missing features
-    depending on the API version."""
-    envs = environments()
-    return render_template('412.html', envs=envs), 412
-
-
-@app.errorhandler(500)
-def server_error(e):
-    envs = environments()
-    return render_template('500.html', envs=envs), 500
-
-
 @app.route('/', defaults={'env': app.config['DEFAULT_ENVIRONMENT']})
 @app.route('/<env>/')
 def index(env):

puppetboard/core.py

@@ -0,0 +1,63 @@
+from __future__ import unicode_literals
+from __future__ import absolute_import
+
+import logging
+
+from flask import Flask
+
+from pypuppetdb import connect
+from puppetboard.utils import (jsonprint, prettyprint, url_for_field,
+                               get_or_abort)
+
+from . import __version__
+
+APP = None
+PUPPETDB = None
+
+
+def get_app():
+    global APP
+
+    if APP is None:
+        app = Flask(__name__)
+        app.config.from_object('puppetboard.default_settings')
+        app.config.from_envvar('PUPPETBOARD_SETTINGS', silent=True)
+        app.secret_key = app.config['SECRET_KEY']
+
+        numeric_level = getattr(logging, app.config['LOGLEVEL'].upper(), None)
+        if not isinstance(numeric_level, int):
+            raise ValueError('Invalid log level: %s' % app.config['LOGLEVEL'])
+
+        app.jinja_env.filters['jsonprint'] = jsonprint
+        app.jinja_env.filters['prettyprint'] = prettyprint
+        app.jinja_env.globals['url_for_field'] = url_for_field
+        APP = app
+
+    return APP
+
+
+def get_puppetdb():
+    global PUPPETDB
+
+    if PUPPETDB is None:
+        app = get_app()
+        puppetdb = connect(host=app.config['PUPPETDB_HOST'],
+                           port=app.config['PUPPETDB_PORT'],
+                           ssl_verify=app.config['PUPPETDB_SSL_VERIFY'],
+                           ssl_key=app.config['PUPPETDB_KEY'],
+                           ssl_cert=app.config['PUPPETDB_CERT'],
+                           timeout=app.config['PUPPETDB_TIMEOUT'],)
+        PUPPETDB = puppetdb
+
+    return PUPPETDB
+
+
+def environments():
+    puppetdb = get_puppetdb()
+    envs = get_or_abort(puppetdb.environments)
+    x = []
+
+    for env in envs:
+        x.append(env['name'])
+
+    return x

puppetboard/errors.py

@@ -0,0 +1,45 @@
+from __future__ import unicode_literals
+from __future__ import absolute_import
+
+from puppetboard.core import get_app, environments
+from werkzeug.exceptions import InternalServerError
+from flask import render_template
+from . import __version__
+
+app = get_app()
+
+
+@app.errorhandler(400)
+def bad_request(e):
+    envs = environments()
+    return render_template('400.html', envs=envs), 400
+
+
+@app.errorhandler(403)
+def forbidden(e):
+    envs = environments()
+    return render_template('403.html', envs=envs), 403
+
+
+@app.errorhandler(404)
+def not_found(e):
+    envs = environments()
+    return render_template('404.html', envs=envs), 404
+
+
+@app.errorhandler(412)
+def precond_failed(e):
+    """We're slightly abusing 412 to handle missing features
+    depending on the API version."""
+    envs = environments()
+    return render_template('412.html', envs=envs), 412
+
+
+@app.errorhandler(500)
+def server_error(e):
+    envs = []
+    try:
+        envs = environments()
+    except InternalServerError as e:
+        pass
+    return render_template('500.html', envs=envs), 500

puppetboard/utils.py

@@ -8,8 +8,7 @@
 from requests.exceptions import HTTPError, ConnectionError
 from pypuppetdb.errors import EmptyResponseError
 
-from flask import abort
-
+from flask import abort, request, url_for
 
 # Python 3 compatibility
 try:
@@ -20,6 +19,13 @@
 log = logging.getLogger(__name__)
 
 
+def url_for_field(field, value):
+    args = request.view_args.copy()
+    args.update(request.args.copy())
+    args[field] = value
+    return url_for(request.endpoint, **args)
+
+
 def jsonprint(value):
     return json.dumps(value, indent=2, separators=(',', ': '))
 

test/test_app.py

@@ -820,4 +820,4 @@ def test_node_facts_json(client, mocker,
     for line in result_json['data']:
         assert len(line) == 2
 
-    assert 'chart' not in result_json
+    assert 'chart' not in result_json

test/test_app_error.py

@@ -1,7 +1,9 @@
 import pytest
 from flask import Flask, current_app
+from werkzeug.exceptions import InternalServerError
 from puppetboard import app
-
+from puppetboard.errors import (bad_request, forbidden, not_found,
+                                precond_failed, server_error)
 from bs4 import BeautifulSoup
 
 
@@ -16,9 +18,17 @@ def mock_puppetdb_environments(mocker):
                                return_value=environemnts)
 
 
+@pytest.fixture
+def mock_server_error(mocker):
+    def raiseInternalServerError():
+        raise InternalServerError('Hello world')
+    return mocker.patch('puppetboard.core.environments',
+                        side_effect=raiseInternalServerError)
+
+
 def test_error_bad_request(mock_puppetdb_environments):
     with app.app.test_request_context():
-        (output, error_code) = app.bad_request(None)
+        (output, error_code) = bad_request(None)
         soup = BeautifulSoup(output, 'html.parser')
 
         assert 'The request sent to PuppetDB was invalid' in soup.p.text
@@ -27,7 +37,7 @@ def test_error_bad_request(mock_puppetdb_environments):
 
 def test_error_forbidden(mock_puppetdb_environments):
     with app.app.test_request_context():
-        (output, error_code) = app.forbidden(None)
+        (output, error_code) = forbidden(None)
         soup = BeautifulSoup(output, 'html.parser')
 
         long_string = "%s %s" % ('What you were looking for has',
@@ -38,7 +48,7 @@ def test_error_forbidden(mock_puppetdb_environments):
 
 def test_error_not_found(mock_puppetdb_environments):
     with app.app.test_request_context():
-        (output, error_code) = app.not_found(None)
+        (output, error_code) = not_found(None)
         soup = BeautifulSoup(output, 'html.parser')
 
         long_string = "%s %s" % ('What you were looking for could not',
@@ -49,7 +59,7 @@ def test_error_not_found(mock_puppetdb_environments):
 
 def test_error_precond(mock_puppetdb_environments):
     with app.app.test_request_context():
-        (output, error_code) = app.precond_failed(None)
+        (output, error_code) = precond_failed(None)
         soup = BeautifulSoup(output, 'html.parser')
 
         long_string = "%s %s" % ('You\'ve configured Puppetboard with an API',
@@ -60,8 +70,16 @@ def test_error_precond(mock_puppetdb_environments):
 
 def test_error_server(mock_puppetdb_environments):
     with app.app.test_request_context():
-        (output, error_code) = app.server_error(None)
+        (output, error_code) = server_error(None)
         soup = BeautifulSoup(output, 'html.parser')
 
         assert 'Internal Server Error' in soup.h2.text
         assert error_code == 500
+
+
+def test_early_error_server(mock_server_error):
+    with app.app.test_request_context():
+        (output, error_code) = server_error(None)
+        soup = BeautifulSoup(output, 'html.parser')
+        assert 'Internal Server Error' in soup.h2.text
+        assert error_code == 500

test/test_docker_settings.py

@@ -1,7 +1,7 @@
 import pytest
 import os
 from puppetboard import docker_settings
-from puppetboard import app
+import puppetboard.core
 
 try:
     import future.utils
@@ -100,12 +100,14 @@ def test_graph_facts_custom(cleanUpEnv):
     assert 'extra' in facts
 
 
-def test_bad_log_value(cleanUpEnv):
+def test_bad_log_value(cleanUpEnv, mocker):
     os.environ['LOGLEVEL'] = 'g'
     os.environ['PUPPETBOARD_SETTINGS'] = '../puppetboard/docker_settings.py'
     reload(docker_settings)
+
+    puppetboard.core.APP = None
     with pytest.raises(ValueError) as error:
-        reload(app)
+        puppetboard.core.get_app()
 
 
 def test_default_table_selctor(cleanUpEnv):