code cleanup + bug fixes

2 files changed, 129 insertions, 111 deletions

diff --git a/python.d/apache_cache.chart.py b/python.d/apache_cache.chart.py
index cf88a6895d..3681a85112 100644
--- a/python.d/apache_cache.chart.py
+++ b/python.d/apache_cache.chart.py
@@ -37,6 +37,10 @@ class Service(LogService):
             raw = self._get_raw_data()
             if raw is None:
                 return None
+            elif not raw:
+                return {'hit': 0,
+                        'miss': 0,
+                        'other': 0}
         except (ValueError, AttributeError):
             return None
 
diff --git a/python.d/python_modules/base.py b/python.d/python_modules/base.py
index 69af6285c6..e44edf5d05 100644
--- a/python.d/python_modules/base.py
+++ b/python.d/python_modules/base.py
@@ -1,9 +1,24 @@
 # -*- coding: utf-8 -*-
-# Description: prototypes for netdata python.d modules
+# Description: netdata python modules framework
 # Author: Pawel Krupa (paulfantom)
 
+# Remember:
+# ALL CODE NEEDS TO BE COMPATIBLE WITH Python > 2.7 and Python > 3.1
+# Follow PEP8 as much as it is possible
+# "check" and "create" CANNOT be blocking.
+# "update" CAN be blocking
+# "update" function needs to be fast, so follow:
+#   https://wiki.python.org/moin/PythonSpeed/PerformanceTips
+# basically:
+#  - use local variables wherever it is possible
+#  - avoid dots in expressions that are executed many times
+#  - use "join()" instead of "+"
+#  - use "import" only at the beginning
+#
+# using ".encode()" in one thread can block other threads as well (only in python2)
+
 import time
-import sys
+# import sys
 import os
 import socket
 import select
@@ -18,7 +33,8 @@ import threading
 import msg
 
 
-class BaseService(threading.Thread):
+# class BaseService(threading.Thread):
+class SimpleService(threading.Thread):
     """
     Prototype of Service class.
     Implemented basic functionality to run jobs by `python.d.plugin`
@@ -42,6 +58,9 @@ class BaseService(threading.Thread):
         self._dimensions = []
         self._charts = []
         self.__chart_set = False
+        self.__first_run = True
+        self.order = []
+        self.definitions = {}
         if configuration is None:
             self.error("BaseService: no configuration parameters supplied. Cannot create Service.")
             raise RuntimeError
@@ -50,6 +69,8 @@ class BaseService(threading.Thread):
             self.timetable = {}
             self.create_timetable()
 
+    # --- BASIC SERVICE CONFIGURATION ---
+
     def _extract_base_config(self, config):
         """
         Get basic parameters to run service
@@ -59,13 +80,14 @@ class BaseService(threading.Thread):
                       'retries':0}
         :param config: dict
         """
+        pop = config.pop
         try:
-            self.override_name = config.pop('name')
+            self.override_name = pop('name')
         except KeyError:
             pass
-        self.update_every = int(config.pop('update_every'))
-        self.priority = int(config.pop('priority'))
-        self.retries = int(config.pop('retries'))
+        self.update_every = int(pop('update_every'))
+        self.priority = int(pop('priority'))
+        self.retries = int(pop('retries'))
         self.retries_left = self.retries
         self.configuration = config
 
@@ -86,6 +108,8 @@ class BaseService(threading.Thread):
                           'next': now - (now % freq) + freq,
                           'freq': freq}
 
+    # --- THREAD CONFIGURATION ---
+
     def _run_once(self):
         """
         Executes self.update(interval) and draws run time chart.
@@ -93,39 +117,36 @@ class BaseService(threading.Thread):
         :return: boolean
         """
         t_start = time.time()
+        timetable = self.timetable
+        chart_name = self.chart_name
         # check if it is time to execute job update() function
-        if self.timetable['next'] > t_start:
-            #msg.debug(self.chart_name + " will be run in " +
-            #          str(int((self.timetable['next'] - t_start) * 1000)) + " ms")
-            msg.debug(self.chart_name, "will be run in", str(int((self.timetable['next'] - t_start) * 1000)), "ms")
+        if timetable['next'] > t_start:
+            self.debug(chart_name, "will be run in", str(int((timetable['next'] - t_start) * 1000)), "ms")
             return True
 
-        since_last = int((t_start - self.timetable['last']) * 1000000)
-        #msg.debug(self.chart_name +
-        #          " ready to run, after " + str(int((t_start - self.timetable['last']) * 1000)) +
-        #          " ms (update_every: " + str(self.timetable['freq'] * 1000) +
-        #          " ms, latency: " + str(int((t_start - self.timetable['next']) * 1000)) + " ms)")
-        msg.debug(self.chart_name,
-                  "ready to run, after", str(int((t_start - self.timetable['last']) * 1000)),
-                  "ms (update_every:", str(self.timetable['freq'] * 1000),
-                  "ms, latency:", str(int((t_start - self.timetable['next']) * 1000)), "ms")
+        since_last = int((t_start - timetable['last']) * 1000000)
+        self.debug(chart_name,
+                   "ready to run, after", str(int((t_start - timetable['last']) * 1000)),
+                   "ms (update_every:", str(timetable['freq'] * 1000),
+                   "ms, latency:", str(int((t_start - timetable['next']) * 1000)), "ms")
+        if self.__first_run:
+            since_last = 0
         if not self.update(since_last):
             self.error("update function failed.")
             return False
         t_end = time.time()
-        self.timetable['next'] = t_end - (t_end % self.timetable['freq']) + self.timetable['freq']
+        self.timetable['next'] = t_end - (t_end % timetable['freq']) + timetable['freq']
         # draw performance graph
         run_time = str(int((t_end - t_start) * 1000))
-        #run_time_chart = "BEGIN netdata.plugin_pythond_" + self.chart_name + " " + str(since_last) + '\n'
-        #run_time_chart += "SET run_time = " + run_time + '\n'
-        #run_time_chart += "END\n"
-        #sys.stdout.write(run_time_chart)
-        sys.stdout.write("BEGIN netdata.plugin_pythond_%s %s\nSET run_time = %s\nEND\n" % \
-                         (self.chart_name, str(since_last), run_time))
-
-        #msg.debug(self.chart_name + " updated in " + str(run_time) + " ms")
-        msg.debug(self.chart_name, "updated in", str(run_time), "ms")
+        # noinspection SqlNoDataSourceInspection
+        print("BEGIN netdata.plugin_pythond_%s %s\nSET run_time = %s\nEND\n" %
+              (self.chart_name, str(since_last), run_time))
+        # sys.stdout.write("BEGIN netdata.plugin_pythond_%s %s\nSET run_time = %s\nEND\n" %
+        #                  (self.chart_name, str(since_last), run_time))
+
+        self.debug(chart_name, "updated in", str(run_time), "ms")
         self.timetable['last'] = t_start
+        self.__first_run = False
         return True
 
     def run(self):
@@ -135,25 +156,32 @@ class BaseService(threading.Thread):
         :return: None
         """
         self.timetable['last'] = time.time()
-        while True:
+        while True:  # run forever, unless something is wrong
             try:
                 status = self._run_once()
             except Exception as e:
-                msg.error("Something wrong: ", str(e))
+                self.error("Something wrong: ", str(e))
                 return
-            if status:
+            if status:  # handle retries if update failed
                 time.sleep(self.timetable['next'] - time.time())
                 self.retries_left = self.retries
             else:
                 self.retries_left -= 1
                 if self.retries_left <= 0:
-                    msg.error("no more retries. Exiting")
+                    self.error("no more retries. Exiting")
                     return
                 else:
                     time.sleep(self.timetable['freq'])
 
+    # --- CHART ---
+
     @staticmethod
     def _format(*args):
+        """
+        Escape and convert passed arguments.
+        :param args: anything
+        :return: list
+        """
         params = []
         append = params.append
         for p in args:
@@ -170,28 +198,14 @@ class BaseService(threading.Thread):
     def _line(self, instruction, *params):
         """
         Converts *params to string and joins them with one space between every one.
+        Result is appended to self._data_stream
         :param params: str/int/float
         """
-        #self._data_stream += instruction
         tmp = list(map((lambda x: "''" if x is None or len(x) == 0 else x), params))
-
         self._data_stream += "%s %s\n" % (instruction, str(" ".join(tmp)))
 
-        # self.error(str(" ".join(tmp)))
-        # for p in params:
-        #     if p is None:
-        #         p = ""
-        #     else:
-        #         p = str(p)
-        #     if len(p) == 0:
-        #         p = "''"
-        #     if ' ' in p:
-        #         p = "'" + p + "'"
-        #     self._data_stream += " " + p
-        #self._data_stream += "\n"
-
     def chart(self, type_id, name="", title="", units="", family="",
-              category="", charttype="line", priority="", update_every=""):
+              category="", chart_type="line", priority="", update_every=""):
         """
         Defines a new chart.
         :param type_id: str
@@ -200,14 +214,13 @@ class BaseService(threading.Thread):
         :param units: str
         :param family: str
         :param category: str
-        :param charttype: str
+        :param chart_type: str
         :param priority: int/str
         :param update_every: int/str
         """
         self._charts.append(type_id)
-        #self._line("CHART", type_id, name, title, units, family, category, charttype, priority, update_every)
 
-        p = self._format(type_id, name, title, units, family, category, charttype, priority, update_every)
+        p = self._format(type_id, name, title, units, family, category, chart_type, priority, update_every)
         self._line("CHART", *p)
 
     def dimension(self, id, name=None, algorithm="absolute", multiplier=1, divisor=1, hidden=False):
@@ -239,10 +252,8 @@ class BaseService(threading.Thread):
         self._dimensions.append(str(id))
         if hidden:
             p = self._format(id, name, algorithm, multiplier, divisor, "hidden")
-            #self._line("DIMENSION", id, name, algorithm, str(multiplier), str(divisor), "hidden")
         else:
             p = self._format(id, name, algorithm, multiplier, divisor)
-            #self._line("DIMENSION", id, name, algorithm, str(multiplier), str(divisor))
 
         self._line("DIMENSION", *p)
 
@@ -294,11 +305,13 @@ class BaseService(threading.Thread):
 
     def commit(self):
         """
-        Upload new data to netdata
+        Upload new data to netdata.
         """
         print(self._data_stream)
         self._data_stream = ""
 
+    # --- ERROR HANDLING ---
+
     def error(self, *params):
         """
         Show error message on stderr
@@ -317,37 +330,7 @@ class BaseService(threading.Thread):
         """
         msg.info(self.chart_name, *params)
 
-    def check(self):
-        """
-        check() prototype
-        :return: boolean
-        """
-        msg.error("Service " + str(self.__module__) + "doesn't implement check() function")
-        return False
-
-    def create(self):
-        """
-        create() prototype
-        :return: boolean
-        """
-        msg.error("Service " + str(self.__module__) + "doesn't implement create() function?")
-        return False
-
-    def update(self, interval):
-        """
-        update() prototype
-        :param interval: int
-        :return: boolean
-        """
-        msg.error("Service " + str(self.__module__) + "doesn't implement update() function")
-        return False
-
-
-class SimpleService(BaseService):
-    def __init__(self, configuration=None, name=None):
-        self.order = []
-        self.definitions = {}
-        BaseService.__init__(self, configuration=configuration, name=name)
+    # --- MAIN METHODS ---
 
     def _get_data(self):
         """
@@ -358,9 +341,14 @@ class SimpleService(BaseService):
 
     def check(self):
         """
-        :return:
+        check() prototype
+        :return: boolean
         """
-        return True
+        self.debug("Module", str(self.__module__), "doesn't implement check() function. Using default.")
+        if self._get_data() is None or len(self._get_data()) == 0:
+            return False
+        else:
+            return True
 
     def create(self):
         """
@@ -414,17 +402,45 @@ class SimpleService(BaseService):
 
 
 class UrlService(SimpleService):
+    # TODO add support for https connections
     def __init__(self, configuration=None, name=None):
         self.url = ""
         self.user = None
         self.password = None
+        self.proxies = {}
         SimpleService.__init__(self, configuration=configuration, name=name)
 
-    def __add_auth(self):
-        passman = urllib2.HTTPPasswordMgrWithDefaultRealm()
-        passman.add_password(None, self.url, self.user, self.password)
-        authhandler = urllib2.HTTPBasicAuthHandler(passman)
-        opener = urllib2.build_opener(authhandler)
+    def __add_openers(self):
+        # TODO add error handling
+        opener = urllib2.build_opener()
+
+        # Proxy handling
+        # TODO currently self.proxies isn't parsed from configuration file
+        if len(self.proxies) > 0:
+            for proxy in self.proxies:
+                url = proxy['url']
+                # TODO test this:
+                if "user" in proxy and "pass" in proxy:
+                    if url.lower().startswith('https://'):
+                        url = 'https://' + proxy['user'] + ':' + proxy['pass'] + '@' + url[8:]
+                    else:
+                        url = 'http://' + proxy['user'] + ':' + proxy['pass'] + '@' + url[7:]
+                # FIXME move proxy auth to sth like this:
+                #     passman = urllib2.HTTPPasswordMgrWithDefaultRealm()
+                #     passman.add_password(None, url, proxy['user'], proxy['password'])
+                #     opener.add_handler(urllib2.HTTPBasicAuthHandler(passman))
+
+                if url.lower().startswith('https://'):
+                    opener.add_handler(urllib2.ProxyHandler({'https': url}))
+                else:
+                    opener.add_handler(urllib2.ProxyHandler({'https': url}))
+
+        # HTTP Basic Auth
+        if self.user is not None and self.password is not None:
+            passman = urllib2.HTTPPasswordMgrWithDefaultRealm()
+            passman.add_password(None, self.url, self.user, self.password)
+            opener.add_handler(urllib2.HTTPBasicAuthHandler(passman))
+
         urllib2.install_opener(opener)
 
     def _get_raw_data(self):
@@ -434,7 +450,7 @@ class UrlService(SimpleService):
         """
         raw = None
         try:
-            f = urllib2.urlopen(self.url, timeout=self.update_every)
+            f = urllib2.urlopen(self.url, timeout=self.update_every * 2)
         except Exception as e:
             self.error(str(e))
             return None
@@ -470,13 +486,12 @@ class UrlService(SimpleService):
         except (KeyError, TypeError):
             pass
 
-        if self.user is not None and self.password is not None:
-            self.__add_auth()
+        self.__add_openers()
 
-        if self._get_data() is not None:
-            return True
-        else:
+        if self._get_data() is None or len(self._get_data()) == 0:
             return False
+        else:
+            return True
 
 
 class SocketService(SimpleService):
@@ -502,14 +517,17 @@ class SocketService(SimpleService):
                     # establish ipv6 or ipv4 connection.
                     for res in socket.getaddrinfo(self.host, self.port, socket.AF_UNSPEC, socket.SOCK_STREAM):
                         try:
+                            # noinspection SpellCheckingInspection
                             af, socktype, proto, canonname, sa = res
                             self._sock = socket.socket(af, socktype, proto)
-                        except socket.error as msg:
+                        except socket.error as e:
+                            self.debug("Cannot create socket:", str(e))
                             self._sock = None
                             continue
                         try:
                             self._sock.connect(sa)
-                        except socket.error as msg:
+                        except socket.error as e:
+                            self.debug("Cannot connect to socket:", str(e))
                             self._disconnect()
                             continue
                         self.__socket_config = res
@@ -520,7 +538,8 @@ class SocketService(SimpleService):
                         af, socktype, proto, canonname, sa = self.__socket_config
                         self._sock = socket.socket(af, socktype, proto)
                         self._sock.connect(sa)
-                    except socket.error as msg:
+                    except socket.error as e:
+                        self.debug("Cannot create or connect to socket:", str(e))
                         self._disconnect()
             else:
                 # connect to unix socket
@@ -665,7 +684,7 @@ class LogService(SimpleService):
                 self._last_position = 0
             elif os.path.getsize(self.log_path) == self._last_position:
                 self.debug("Log file hasn't changed. No new data.")
-                return None
+                return False
             with open(self.log_path, "r") as fp:
                 fp.seek(self._last_position)
                 for i, line in enumerate(fp):
@@ -701,13 +720,13 @@ class LogService(SimpleService):
             return False
 
     def create(self):
+        self._last_position = os.path.getsize(self.log_path)
         status = SimpleService.create(self)
-        self._last_position = 0
+        # self._last_position = 0
         return status
 
 
 class ExecutableService(SimpleService):
-    #command_whitelist = ['exim', 'postqueue']
     bad_substrings = ('&', '|', ';', '>', '<')
 
     def __init__(self, configuration=None, name=None):
@@ -748,9 +767,6 @@ class ExecutableService(SimpleService):
         except (KeyError, TypeError):
             self.error("No command specified. Using: '" + self.command + "'")
         self.command = self.command.split(' ')
-        #if self.command[0] not in self.command_whitelist:
-        #    self.error("Command is not whitelisted.")
-        #    return False
 
         for arg in self.command[1:]:
             if any(st in arg for st in self.bad_substrings):
@@ -763,8 +779,6 @@ class ExecutableService(SimpleService):
                 self.command[0] = prefix + base
                 if os.path.isfile(self.command[0]):
                     break
-                #if self._get_raw_data() is not None:
-                #    break
 
         if self._get_data() is None or len(self._get_data()) == 0:
             self.error("Command", self.command, "returned no data")