Fixed style based changes

master/buildbot/plugins/__init__.py

@@ -17,15 +17,15 @@
 Buildbot plugin infrastructure
 """
 
-from buildbot import statistics as stats
+from buildbot import statistics
 from buildbot.interfaces import IBuildSlave
 from buildbot.interfaces import IBuildStep
 from buildbot.interfaces import IChangeSource
 from buildbot.interfaces import IScheduler
 from buildbot.plugins.db import get_plugins
 
 
-__all__ = ['changes', 'schedulers', 'buildslave', 'steps', 'util', 'reporters', 'stats']
+__all__ = ['changes', 'schedulers', 'buildslave', 'steps', 'util', 'reporters', 'statistics']
 
 
 # Names here match the names of the corresponding Buildbot module, hence

master/buildbot/process/build.py

@@ -301,6 +301,7 @@ def startBuild(self, build_status, expectations, slavebuilder):
         yield self.master.data.updates.setBuildStateString(self.buildid,
                                                            u'finished')
         yield self.master.data.updates.finishBuild(self.buildid, self.results)
+
         # mark the build as finished
         self.slavebuilder.buildFinished()
         slave.updateSlaveStatus(buildFinished=self)

master/buildbot/statistics/capture.py

@@ -16,6 +16,8 @@
 from twisted.internet import defer
 from twisted.internet import threads
 
+from buildbot import config
+
 
 class Capture(object):
 
@@ -25,20 +27,26 @@ class Capture(object):
 
     def __init__(self, routingKey, callback):
         self.routingKey = routingKey
-        self.callback = callback
+        self._callback = callback
         # parent service and buildmaster to be set when StatsService initialized
         self.parent_svcs = []
         self.master = None
 
-    def defaultContext(self, msg):
+    def _defaultContext(self, msg):
         return {
-            "builder_name": self.builder_name,
+            "builder_name": self._builder_name,
             "build_number": str(msg['number'])
         }
 
-    def consumer(self, routingKey, msg):
+    def consume(self, routingKey, msg):
         raise NotImplementedError
 
+    @defer.inlineCallbacks
+    def _store(self, post_data, series_name, context):
+        for svc in self.parent_svcs:
+            yield threads.deferToThread(svc.thd_postStatsValue, post_data, series_name,
+                                        context)
+
 
 class CaptureProperty(Capture):
 
@@ -48,8 +56,8 @@ class CaptureProperty(Capture):
     """
 
     def __init__(self, builder_name, property_name, callback=None):
-        self.builder_name = builder_name
-        self.property_name = property_name
+        self._builder_name = builder_name
+        self._property_name = property_name
         routingKey = ("builders", None, "builds", None, "finished")
 
         def default_callback(props, property_name):
@@ -61,24 +69,28 @@ def default_callback(props, property_name):
         Capture.__init__(self, routingKey, callback)
 
     @defer.inlineCallbacks
-    def consumer(self, routingKey, msg):
+    def consume(self, routingKey, msg):
         """
         Consumer for this (CaptureProperty) class. Gets the properties from data api and
         send them to the storage backends.
         """
         builder_info = yield self.master.data.get(("builders", msg['builderid']))
-        if self.builder_name == builder_info['name']:
+        if self._builder_name == builder_info['name']:
             properties = yield self.master.data.get(("builds", msg['buildid'], "properties"))
-            ret_val = self.callback(properties, self.property_name)
-            context = self.defaultContext(msg)
-            series_name = self.builder_name + "-" + self.property_name
+            try:
+                ret_val = self._callback(properties, self._property_name)
+            except KeyError:
+                config.error("CaptureProperty failed."
+                             " The property %s not found for build number %s on builder %s."
+                             % (self._property_name, msg['number'], self._builder_name))
+
+            context = self._defaultContext(msg)
+            series_name = self._builder_name + "-" + self._property_name
             post_data = {
-                "name": self.property_name,
+                "name": self._property_name,
                 "value": ret_val
             }
-            for svc in self.parent_svcs:
-                yield threads.deferToThread(svc.postStatsValue, post_data, series_name,
-                                            context)
+            self._store(post_data, series_name, context)
 
         else:
             yield defer.succeed(None)
@@ -90,27 +102,45 @@ class CaptureBuildTimes(Capture):
     Capture methods for capturing build start times.
     """
 
-    def __init__(self, builder_name, callback):
-        self.builder_name = builder_name
+    def __init__(self, builder_name, callback, time_type):
+        self._builder_name = builder_name
         routingKey = ("builders", None, "builds", None, "finished")
+        self._time_type = time_type
         Capture.__init__(self, routingKey, callback)
 
     @defer.inlineCallbacks
-    def consumer(self, routingKey, msg):
+    def consume(self, routingKey, msg):
         """
         Consumer for CaptureBuildStartTime. Gets the build start time.
         """
         builder_info = yield self.master.data.get(("builders", msg['builderid']))
-        if self.builder_name == builder_info['name']:
-            ret_val = self.callback(*self.retValParams(msg))
-            context = self.defaultContext(msg)
+        if self._builder_name == builder_info['name']:
+            try:
+                ret_val = self._callback(*self._retValParams(msg))
+            except Exception as e:
+                # catching generic exceptions is okay here since we propagate it
+                config.error(self._err_msg(msg) + " Exception raised: " + type(e).__name__ +
+                             " with message: " + e.message)
+            context = self._defaultContext(msg)
             post_data = {
                 self._time_type: ret_val
             }
-            series_name = self.builder_name + "-build-times"
-            for svc in self.parent_svcs:
-                yield threads.deferToThread(svc.postStatsValue, post_data, series_name,
-                                            context)
+            series_name = self._builder_name + "-build-times"
+            self._store(post_data, series_name, context)
+
+        else:
+            yield defer.succeed(None)
+
+    def _err_msg(self, build_data):
+        if self._time_type == "start-time":
+            capture_class = "CaptureBuildStartTime"
+        elif self._time_type == "end-time":
+            capture_class = "CaptureBuildEndTime"
+        if self._time_type == "duration":
+            capture_class = "CaptureBuildDuration "
+        msg = "%s failed on build %s on builder %s." % (capture_class, build_data['number'],
+                                                        self._builder_name)
+        return msg
 
 
 class CaptureBuildStartTime(CaptureBuildTimes):
@@ -124,10 +154,10 @@ def default_callback(start_time):
             return start_time.isoformat()
         if not callback:
             callback = default_callback
-        self._time_type = "start-time"
-        CaptureBuildTimes.__init__(self, builder_name, callback)
+        time_type = "start-time"
+        CaptureBuildTimes.__init__(self, builder_name, callback, time_type)
 
-    def retValParams(self, msg):
+    def _retValParams(self, msg):
         return [msg['started_at']]
 
 
@@ -142,10 +172,10 @@ def default_callback(end_time):
             return end_time.isoformat()
         if not callback:
             callback = default_callback
-        self._time_type = "end-time"
-        CaptureBuildTimes.__init__(self, builder_name, callback)
+        time_type = "end-time"
+        CaptureBuildTimes.__init__(self, builder_name, callback, time_type)
 
-    def retValParams(self, msg):
+    def _retValParams(self, msg):
         return [msg['complete_at']]
 
 
@@ -171,10 +201,10 @@ def default_callback(start_time, end_time):
 
         if not callback:
             callback = default_callback
-        self._time_type = "duration"
-        CaptureBuildTimes.__init__(self, builder_name, callback)
+        time_type = "duration"
+        CaptureBuildTimes.__init__(self, builder_name, callback, time_type)
 
-    def retValParams(self, msg):
+    def _retValParams(self, msg):
         return [msg['started_at'], msg['complete_at']]
 
 
@@ -185,24 +215,34 @@ class CaptureData(Capture):
     """
 
     def __init__(self, data_name, builder_name, callback=None):
-        self.data_name = data_name
-        self.builder_name = builder_name
+        self._data_name = data_name
+        self._builder_name = builder_name
+
+        def identity(x):
+            return x
 
         if not callback:
-            callback = lambda x: x
+            callback = identity
 
         routingKey = ("stats-yieldMetricsValue", "stats-yield-data")
         Capture.__init__(self, routingKey, callback)
 
     @defer.inlineCallbacks
-    def consumer(self, routingKey, msg):
+    def consume(self, routingKey, msg):
+        """
+        Consumer for this (CaptureData) class. Gets the data sent from yieldMetricsValue and
+        sends it to the storage backends.
+        """
         build_data = msg['build_data']
         builder_info = yield self.master.data.get(("builders", build_data['builderid']))
-        if self.builder_name == builder_info['name'] and self.data_name == msg['data_name']:
-            ret_val = self.callback(msg['post_data'])
-            context = self.defaultContext(build_data)
+        if self._builder_name == builder_info['name'] and self._data_name == msg['data_name']:
+            try:
+                ret_val = self._callback(msg['post_data'])
+            except Exception as e:
+                config.error("CaptureData failed for build %s of builder %s."
+                             " Exception generated: %s with message %s"
+                             % (msg['number'], self._builder_name, type(e).__name__, e.message))
             post_data = ret_val
-            series_name = self.builder_name + "-" + self.data_name
-            for svc in self.parent_svcs:
-                yield threads.deferToThread(svc.postStatsValue, post_data, series_name,
-                                            context)
+            series_name = self._builder_name + "-" + self._data_name
+            context = self._defaultContext(build_data)
+            self._store(post_data, series_name, context)

master/buildbot/statistics/stats_service.py

@@ -54,7 +54,7 @@ def registerConsumers(self):
             for cap in svc.captures:
                 cap.parent_svcs.append(svc)
                 cap.master = self.master
-                consumer = yield self.master.mq.startConsuming(cap.consumer, cap.routingKey)
+                consumer = yield self.master.mq.startConsuming(cap.consume, cap.routingKey)
                 self.consumers.append(consumer)
 
     @defer.inlineCallbacks
@@ -83,9 +83,10 @@ def yieldMetricsValue(self, data_name, post_data, buildid):
         build_data = yield self.master.data.get(('builds', buildid))
         routingKey = ("stats-yieldMetricsValue", "stats-yield-data")
 
-        msg = dict()
-        msg['data_name'] = data_name
-        msg['post_data'] = post_data
-        msg['build_data'] = build_data
+        msg = {
+            'data_name': data_name,
+            'post_data': post_data,
+            'build_data': build_data
+        }
 
         self.master.mq.produce(routingKey, msg)

master/buildbot/statistics/storage_backends/__init__.py

@@ -0,0 +1,19 @@
+# This file is part of Buildbot.  Buildbot is free software: you can
+# redistribute it and/or modify it under the terms of the GNU General Public
+# License as published by the Free Software Foundation, version 2.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program; if not, write to the Free Software Foundation, Inc., 51
+# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+# Copyright Buildbot Team Members
+
+from buildbot.statistics.storage_backends.base import StatsStorageBase
+from buildbot.statistics.storage_backends.influxdb_client import InfluxStorageService
+
+__all__ = ['StatsStorageBase', 'InfluxStorageService']

master/buildbot/statistics/storage_backends/base.py

@@ -0,0 +1,27 @@
+# This file is part of Buildbot.  Buildbot is free software: you can
+# redistribute it and/or modify it under the terms of the GNU General Public
+# License as published by the Free Software Foundation, version 2.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program; if not, write to the Free Software Foundation, Inc., 51
+# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+# Copyright Buildbot Team Members
+
+from twisted.internet import defer
+
+
+class StatsStorageBase(object):
+
+    """
+    Base class for sub service responsible for passing on stats data to
+    a storage backend
+    """
+
+    def thd_postStatsValue(self, post_data, series_name, context=None):
+        return defer.succeed(None)

master/buildbot/statistics/storage_backends.py → master/buildbot/statistics/storage_backends/influxdb_client.py

@@ -13,28 +13,17 @@
 #
 # Copyright Buildbot Team Members
 
-from twisted.internet import defer
 from twisted.python import log
 
 from buildbot import config
+from buildbot.statistics.storage_backends import StatsStorageBase
 
 try:
     from influxdb import InfluxDBClient
 except ImportError:
     InfluxDBClient = None
 
 
-class StatsStorageBase(object):
-
-    """
-    Base class for sub service responsible for passing on stats data to
-    a storage backend
-    """
-
-    def postStatsValue(self, name, value, series_name, context={}):
-        return defer.succeed(None)
-
-
 class InfluxStorageService(StatsStorageBase):
 
     """
@@ -58,17 +47,20 @@ def __init__(self, url, port, user, password, db, captures,
                                      self.password, self.db)
         self.inited = True
 
-    def postStatsValue(self, post_data, series_name, context={}):
+    def thd_postStatsValue(self, post_data, series_name, context=None):
         if not self.inited:
             log.err("Service {0} not initialized".format(self.name))
             return
+
+        data = {
+            'name': series_name,
+            'fields': post_data
+        }
+
         log.msg("Sending data to InfluxDB")
         log.msg("post_data: {0!r}".format(post_data))
-        log.msg("context: {0!r}".format(context))
+        if context:
+            log.msg("context: {0!r}".format(context))
+            data['tags'] = context
 
-        data = {}
-        data['name'] = series_name
-        data['fields'] = post_data
-        data['tags'] = context
-        points = [data]
-        self.client.write_points(points)
+        self.client.write_points([data])