Error-checking, pep8 conformance and linting

Author: absolutejam

requirements_test.txt

@@ -3,3 +3,4 @@ nose
 pep8
 pylint
 coverage
+requests

sensu_plugin/handler.py

@@ -6,18 +6,27 @@
 # Released under the same terms as Sensu (the MIT license); see LICENSE
 # for details.
 
+'''
+This provides a base SensuHandler class that can be used for writing
+python-based Sensu handlers.
+'''
+
 from __future__ import print_function
 import os
 import sys
+import json
 import requests
 try:
     from urlparse import urlparse
 except ImportError:
     from urllib.parse import urlparse
-from utils import *
+from sensu_plugin.utils import get_settings
 
 
 class SensuHandler(object):
+    '''
+    Class to be used as a basis for handlers.
+    '''
     def __init__(self):
         # Parse the stdin into a global event object
         stdin = sys.stdin.read()
@@ -40,8 +49,8 @@ def read_event(self, check_result):
             self.event['occurrences'] = self.event.get('occurrences', 1)
             self.event['check'] = self.event.get('check', {})
             self.event['client'] = self.event.get('client', {})
-        except Exception as e:
-            print('error reading event: ' + e.message)
+        except Exception as exception:  # pylint: disable=broad-except
+            print('error reading event: ' + exception)
             sys.exit(1)
 
     def handle(self):
@@ -63,7 +72,7 @@ def filter(self):
             self.filter_silenced()
             self.filter_dependencies()
 
-            if self.deprecated_occurrence_filtering_enabled():
+            if self.deprecated_occurrence_filtering():
                 print('warning: occurrence filtering in sensu-plugin is' +
                       'deprecated, see http://bit.ly/sensu-plugin')
                 self.filter_repeated()
@@ -78,7 +87,7 @@ def deprecated_filtering_enabled(self):
         '''
         return self.event['check'].get('enable_deprecated_filtering', False)
 
-    def deprecated_occurrence_filtering_enabled(self):
+    def deprecated_occurrence_filtering(self):
         '''
         Evaluates whether the event should be processed by the
         filter_repeated method. Defaults to true, i.e. filter_repeated
@@ -94,23 +103,23 @@ def bail(self, msg):
         '''
         Gracefully terminate with message
         '''
-        client_name = self.event['client'].get('name', 'error:no-client-name')
+        client_name = self.event.get('client', 'error:no-client-name')
         check_name = self.event['client'].get('name', 'error:no-check-name')
         print('{}: {}/{}'.format(msg, client_name, check_name))
         sys.exit(0)
 
     def get_api_settings(self):
         '''
-        Return a hash of API settings derived first from ENV['SENSU_API_URL']
+        Return a hash of API settings derived first from ENV['sensu_api_url']
         if set, then Sensu config `api` scope if configured, and finally
         falling back to to ipv4 localhost address on default API port.
 
         return dict
         '''
 
-        SENSU_API_URL = os.environ.get('SENSU_API_URL')
-        if SENSU_API_URL:
-            uri = urlparse(SENSU_API_URL)
+        sensu_api_url = os.environ.get('sensu_api_url')
+        if sensu_api_url:
+            uri = urlparse(sensu_api_url)
             self.api_settings = {
                 'host': '{0}//{1}'.format(uri.scheme, uri.hostname),
                 'port': uri.port,
@@ -125,7 +134,10 @@ def get_api_settings(self):
                 'port', 4567)
 
     # API requests
-    def api_request(method, path, blk):
+    def api_request(self, method, path):
+        '''
+        Query Sensu api for information.
+        '''
         if not hasattr(self, 'api_settings'):
             ValueError('api.json settings not found')
 
@@ -135,7 +147,6 @@ def api_request(method, path, blk):
             _request = requests.post
 
         domain = self.api_settings['host']
-        # TODO: http/https
         uri = 'http://{}:{}{}'.format(domain, self.api_settings['port'], path)
         if self.api_settings['user'] and self.api_settings['password']:
             auth = (self.api_settings['user'], self.api_settings['password'])
@@ -145,19 +156,32 @@ def api_request(method, path, blk):
         return req
 
     def stash_exists(self, path):
+        '''
+        Query Sensu API for stash data.
+        '''
         return self.api_request('get', '/stash' + path).status_code == 200
 
     def event_exists(self, client, check):
-        return self.api_request('get',
-                                '/events/{}/{}'.format(client, check)
-                               ).status_code == 200
+        '''
+        Query Sensu API for event.
+        '''
+        return self.api_request(
+            'get',
+            'events/{}/{}'.format(client, check)
+            ).status_code == 200
 
     # Filters
     def filter_disabled(self):
+        '''
+        Determine whether a check is disabled and shouldn't handle.
+        '''
         if self.event['check']['alert'] is False:
-            bail('alert disabled')
+            self.bail('alert disabled')
 
     def filter_silenced(self):
+        '''
+        Determine whether a check is silenced and shouldn't handle.
+        '''
         stashes = [
             ('client', '/silence/{}'.format(self.event['client']['name'])),
             ('check', '/silence/{}/{}'.format(
@@ -166,17 +190,18 @@ def filter_silenced(self):
             ('check', '/silence/all/{}'.format(self.event['check']['name']))
         ]
         for scope, path in stashes:
-            if stash_exists(path):
-                bail(scope + ' alerts silenced')
-            # TODO: Timeout for querying Sensu API?
-            #       More appropriate in the api_request method?
+            if self.stash_exists(path):
+                self.bail(scope + ' alerts silenced')
 
     def filter_dependencies(self):
+        '''
+        Determine whether a check has dependencies.
+        '''
         dependencies = self.event['check'].get('dependencies', None)
         if dependencies is None or not isinstance(dependencies, list):
             return
         for dependency in self.event['check']['dependencies']:
-            if len(str(dependency)) == 0:
+            if not str(dependency):
                 continue
             dependency_split = tuple(dependency.split('/'))
             # If there's a dependency on a check from another client, then use
@@ -187,9 +212,12 @@ def filter_dependencies(self):
                 client = self.event['client']['name']
                 check = dependency_split[0]
             if self.event_exists(client, check):
-                bail('check dependency event exists')
+                self.bail('check dependency event exists')
 
     def filter_repeated(self):
+        '''
+        Determine whether a check is repeating.
+        '''
         defaults = {
             'occurrences': 1,
             'interval': 30,
@@ -198,8 +226,7 @@ def filter_repeated(self):
 
         # Override defaults with anything defined in the settings
         if isinstance(self.settings['sensu_plugin'], dict):
-            defaults.update(settings['sensu_plugin'])
-        end
+            defaults.update(self.settings['sensu_plugin'])
 
         occurrences = int(self.event['check'].get(
             'occurrences', defaults['occurrences']))
@@ -209,7 +236,7 @@ def filter_repeated(self):
             'refresh', defaults['refresh']))
 
         if self.event['occurrences'] < occurrences:
-            bail('not enough occurrences')
+            self.bail('not enough occurrences')
 
         if (self.event['occurrences'] > occurrences and
                 self.event['action'] == 'create'):
@@ -220,4 +247,4 @@ def filter_repeated(self):
                 (self.event['occurrences'] - occurrences) % number == 0):
             return
 
-        bail('only handling every ' + str(number) + ' occurrences')
+        self.bail('only handling every ' + str(number) + ' occurrences')

sensu_plugin/utils/__init__.py

@@ -1,25 +1,34 @@
+'''
+Utilities for loading config files, etc.
+'''
+
 import os
 import json
 
 
 def config_files():
-    SENSU_LOADED_TEMPFILE = os.environ.get('SENSU_LOADED_TEMPFILE')
-    SENSU_CONFIG_FILES = os.environ.get('SENSU_CONFIG_FILES')
-    if SENSU_LOADED_TEMPFILE and os.path.isfile(SENSU_LOADED_TEMPLATE):
-        with open(SENSU_LOADED_TEMPLATE, 'r') as template:
-            contents = template.read()
+    '''
+    Get list of currently used config files.
+    '''
+    sensu_loaded_tempfile = os.environ.get('SENSU_LOADED_TEMPFILE')
+    sensu_config_files = os.environ.get('SENSU_CONFIG_FILES')
+    if sensu_loaded_tempfile and os.path.isfile(sensu_loaded_tempfile):
+        with open(sensu_loaded_tempfile, 'r') as tempfile:
+            contents = tempfile.read()
             return contents.split(':')
-    elif SENSU_CONFIG_FILES:
-        return SENSU_CONFIG_FILES.split(':')
+    elif sensu_config_files:
+        return sensu_config_files.split(':')
     else:
         files = ['/etc/sensu/config.json']
-        [files.append('/etc/sensu/conf.d/{}'.format(filename))
-         for filename in os.listdir('/etc/sensu/conf.d')
-         if os.path.splitext(filename)[1] == '.json']
-        return files
+        return [files.append('/etc/sensu/conf.d/{}'.format(filename))
+                for filename in os.listdir('/etc/sensu/conf.d')
+                if os.path.splitext(filename)[1] == '.json']
 
 
 def get_settings():
+    '''
+    Get all currently loaded settings.
+    '''
     settings = {}
     for config_file in config_files():
         config_contents = load_config(config_file)
@@ -29,19 +38,25 @@ def get_settings():
 
 
 def load_config(filename):
+    '''
+    Read contents of config file.
+    '''
     try:
         with open(filename, 'r') as config_file:
             return json.loads(config_file.read())
-    except FileNotFoundError:
-        {}
+    except IOError:
+        pass
 
 
 def deep_merge(dict_one, dict_two):
+    '''
+    Deep merge two dicts.
+    '''
     merged = dict_one.copy()
     for key, value in dict_two.items():
         # value is equivalent to dict_two[key]
         if (key in dict_one and
-            isinstance(dict_one[key], dict) and
+                isinstance(dict_one[key], dict) and
                 isinstance(value, dict)):
             merged[key] = deep_merge(dict_one[key], value)
         elif (key in dict_one and