From 716fa6c76a2c10f8eea6bedae62d352a05917110 Mon Sep 17 00:00:00 2001
From: Pierre Tardy <tardyp@gmail.com>
Date: Wed, 12 Oct 2016 10:11:50 +0200
Subject: [PATCH] cleanup after reviews

---
 master/buildbot/util/service.py | 36 ++++++++++++++++++---------------
 master/buildbot/worker/hyper.py | 29 +++++++++++++-------------
 2 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/master/buildbot/util/service.py b/master/buildbot/util/service.py
index f2af04f58271..5ec3e4b02295 100644
--- a/master/buildbot/util/service.py
+++ b/master/buildbot/util/service.py
@@ -11,10 +11,10 @@
 # Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 #
 # Copyright Buildbot Team Members
-import hashlib
-
 from future.utils import itervalues
 
+import hashlib
+
 from twisted.application import service
 from twisted.internet import defer
 from twisted.internet import task
@@ -124,31 +124,35 @@ def getService(cls, parent, *args, **kwargs):
         if name in parent.namedServices:
             return defer.succeed(parent.namedServices[name])
         try:
-            o = cls(*args, **kwargs)
+            instance = cls(*args, **kwargs)
         except Exception:
             return defer.fail(failure.Failure())
-        o.name = name
-        d = o.setServiceParent(parent)
+        # The class is not required to initialized its name
+        # but we use the name to identify the instance in the parent service
+        # so we force it with the name we used
+        instance.name = name
+        d = instance.setServiceParent(parent)
 
         @d.addCallback
-        def returnObject(res):
+        def returnInstance(res):
             # we put the service on top of the list, so that it is stopped the last
             # This make sense as the singleton service is used as a dependency
             # for other service
-            parent.services.remove(o)
-            parent.services.insert(0, o)
-            return o
+            parent.services.remove(instance)
+            parent.services.insert(0, instance)
+            # hook the return value to the instance object
+            return instance
         return d
 
     @classmethod
     def getName(cls, *args, **kwargs):
-        h = hashlib.sha1()
-        for a in args:
-            h.update(str(a))
-        for a, b in kwargs.items():
-            h.update(str(a))
-            h.update(str(b))
-        return cls.__name__ + "_" + h.hexdigest()
+        _hash = hashlib.sha1()
+        for arg in args:
+            _hash.update(str(arg))
+        for k, v in kwargs.items():
+            _hash.update(str(k))
+            _hash.update(str(v))
+        return cls.__name__ + "_" + _hash.hexdigest()
 
 
 class BuildbotService(AsyncMultiService, config.ConfiguredMixin, util.ComparableMixin,
diff --git a/master/buildbot/worker/hyper.py b/master/buildbot/worker/hyper.py
index e43a24fcbc7a..a2d32bb2e9a4 100644
--- a/master/buildbot/worker/hyper.py
+++ b/master/buildbot/worker/hyper.py
@@ -53,9 +53,8 @@ class HyperLatentManager(service.SingletonService):
 
     def __init__(self, hyper_host, hyper_accesskey, hyper_secretkey):
         service.SingletonService.__init__(self)
-        self.name = self.getName(hyper_host, hyper_accesskey, hyper_secretkey)
         # Prepare the parameters for the Docker Client object.
-        self.client_args = {'clouds': {
+        self._client_args = {'clouds': {
             hyper_host: {
                 "accesskey": hyper_accesskey,
                 "secretkey": hyper_secretkey
@@ -63,17 +62,21 @@ def __init__(self, hyper_host, hyper_accesskey, hyper_secretkey):
         }}
 
     def startService(self):
-        self.threadPool = threadpool.ThreadPool(
+        self._threadPool = threadpool.ThreadPool(
             minthreads=1, maxthreads=self.MAX_THREADS, name='hyper')
-        self.threadPool.start()
-        self.client = Hyper(self.client_args)
+        self._threadPool.start()
+        self._client = Hyper(self._client_args)
+
+    @property
+    def client(self):
+        return self._client
 
     def stopService(self):
         self.client.close()
-        return self.threadPool.stop()
+        return self._threadPool.stop()
 
-    def runInThread(self, reactor, meth, *args, **kwargs):
-        return threads.deferToThreadPool(reactor, self.threadPool, meth, *args, **kwargs)
+    def deferToThread(self, reactor, meth, *args, **kwargs):
+        return threads.deferToThreadPool(reactor, self._threadPool, meth, *args, **kwargs)
 
 
 class HyperLatentWorker(AbstractLatentWorker):
@@ -81,8 +84,6 @@ class HyperLatentWorker(AbstractLatentWorker):
     instance = None
     ALLOWED_SIZES = ['s1', 's2', 's3', 's4',
                      'm1', 'm2', 'm3', 'l1', 'l2', 'l3']
-    threadPool = None
-    client = None
     image = None
     reactor = global_reactor
 
@@ -139,8 +140,8 @@ def createEnvironment(self):
                 "BUILDMASTER_PORT"] = self.masterFQDN.split(":")
         return result
 
-    def runInThread(self, meth, *args, **kwargs):
-        return self.manager.runInThread(self.reactor, meth, *args, **kwargs)
+    def deferToThread(self, meth, *args, **kwargs):
+        return self.manager.deferToThread(self.reactor, meth, *args, **kwargs)
 
     @defer.inlineCallbacks
     def start_instance(self, build):
@@ -148,7 +149,7 @@ def start_instance(self, build):
             raise ValueError('instance active')
 
         image = yield build.render(self.image)
-        yield self.runInThread(self._thd_start_instance, image)
+        yield self.deferToThread(self._thd_start_instance, image)
         defer.returnValue(True)
 
     def getContainerName(self):
@@ -193,7 +194,7 @@ def stop_instance(self, fast=False):
             # instance never attached, and it's because, somehow, we never
             # started.
             return defer.succeed(None)
-        return self.runInThread(self._thd_stop_instance, fast)
+        return self.deferToThread(self._thd_stop_instance, fast)
 
     @property
     def shortid(self):