WIP: keepalive code completely moved to Connection, some test fail fo…

…r now

master/buildbot/buildslave/base.py

@@ -47,15 +47,13 @@ class AbstractBuildSlave(config.ReconfigurableServiceMixin,
     subclassed to add extra functionality."""
 
     implements(IBuildSlave)
-    keepalive_timer = None
-    keepalive_interval = None
 
     # reconfig slaves after builders
     reconfig_priority = 64
 
     def __init__(self, name, password, max_builds=None,
                  notify_on_missing=[], missing_timeout=3600,
-                 properties={}, locks=None, keepalive_interval=3600):
+                 properties={}, locks=None):
         """
         @param name: botname this machine will supply when it connects
         @param password: password this machine will supply when
@@ -105,7 +103,6 @@ def __init__(self, name, password, max_builds=None,
                     'notify_on_missing arg %r is not a string' % (i,))
         self.missing_timeout = missing_timeout
         self.missing_timer = None
-        self.keepalive_interval = keepalive_interval
 
         # a protocol connection, if we're currently connected
         self.conn = None
@@ -239,9 +236,6 @@ def reconfigService(self, new_config):
         self.max_builds = new.max_builds
         self.access = new.access
         self.notify_on_missing = new.notify_on_missing
-        self.keepalive_interval = new.keepalive_interval
-        if self.conn:
-            self.conn.updateKeepaliveInterval(new.keepalive_interval)
 
         if self.missing_timeout != new.missing_timeout:
             running_missing_timer = self.missing_timer
@@ -290,15 +284,6 @@ def stopMissingTimer(self):
             self.missing_timer.cancel()
             self.missing_timer = None
 
-    def stopKeepaliveTimer(self):
-        if self.conn:
-            self.conn.stopKeepaliveTimer()
-
-    def startKeepaliveTimer(self):
-        if self.conn:
-            self.conn.startKeepaliveTimer()
-            log.msg("Starting buildslave keepalive timer for '%s'" % self.slavename)
-
     def isConnected(self):
         return self.conn
 
@@ -373,7 +358,6 @@ def attached(self, conn):
         # We want to know when the graceful shutdown flag changes
         self.slave_status.addGracefulWatcher(self._gracefulChanged)
         self.conn = conn
-        self.conn.keepalive_interval = getattr(self, "keepalive_interval", None)
 
         d = defer.succeed(None)
 
@@ -399,8 +383,6 @@ def _got_info(info):
             d1.addCallback(_got_info)
             return d1
 
-        d.addCallback(lambda _: self.startKeepaliveTimer())
-
         @d.addCallback
         def _accept_slave(res):
             self.slave_status.setConnected(True)
@@ -444,8 +426,6 @@ def detached(self):
         self.slave_status.setConnected(False)
         log.msg("BuildSlave.detached(%s)" % self.slavename)
         self.botmaster.master.status.slaveDisconnected(self.slavename)
-        if self.conn:
-            self.conn.stopKeepaliveTimer()
         self.releaseLocks()
 
         # notify watchers, but do so in the next reactor iteration so that

master/buildbot/buildslave/protocols/pb.py

@@ -75,12 +75,14 @@ def _getPerspective(self, mind, buildslaveName):
 
 class Connection(base.Connection, pb.Avatar):
 
+    # TODO: configure keepalive_interval in c['protocols']['pb']['keepalive_interval']
+    keepalive_timer = None
+    keepalive_interval = 3600
+
     def __init__(self, master, buildslave, mind):
         base.Connection.__init__(self, master, buildslave)
         self.mind = mind
-        keepalive_timer = None
-        keepalive_interval = None
-
+        self.startKeepaliveTimer()
 
     # methods called by the PBManager
 
@@ -206,11 +208,6 @@ def _errback(why):
     def remoteStartBuild(self):
         return self.mind.callRemote('startBuild')
 
-    def updateKeepaliveInterval(self, keepalive_interval):
-        self.keepalive_interval = keepalive_interval
-        self.stopKeepaliveTimer()
-        self.startKeepaliveTimer()
-
     def stopKeepaliveTimer(self):
         if self.keepalive_timer:
             self.keepalive_timer.cancel()

master/buildbot/test/unit/test_buildslave_base.py

@@ -50,7 +50,6 @@ def test_constructor_minimal(self):
         self.assertEqual(bs.missing_timeout, 3600)
         self.assertEqual(bs.properties.getProperty('slavename'), 'bot')
         self.assertEqual(bs.access, [])
-        self.assertEqual(bs.keepalive_interval, 3600)
 
     def test_constructor_full(self):
         lock1, lock2 = mock.Mock(name='lock1'), mock.Mock(name='lock2')
@@ -59,14 +58,13 @@ def test_constructor_full(self):
                 notify_on_missing=['me@me.com'],
                 missing_timeout=120,
                 properties={'a':'b'},
-                locks=[lock1, lock2],
-                keepalive_interval=60)
+                locks=[lock1, lock2])
+
         self.assertEqual(bs.max_builds, 2)
         self.assertEqual(bs.notify_on_missing, ['me@me.com'])
         self.assertEqual(bs.missing_timeout, 120)
         self.assertEqual(bs.properties.getProperty('a'), 'b')
         self.assertEqual(bs.access, [lock1, lock2])
-        self.assertEqual(bs.keepalive_interval, 60)
 
     def test_constructor_notify_on_missing_not_list(self):
         bs = self.ConcreteBuildSlave('bot', 'pass',
@@ -98,14 +96,12 @@ def test_reconfigService_attrs(self):
                 max_builds=2,
                 notify_on_missing=['me@me.com'],
                 missing_timeout=120,
-                properties={'a':'b'},
-                keepalive_interval=60)
+                properties={'a':'b'})
         new = self.ConcreteBuildSlave('bot', 'pass',
                 max_builds=3,
                 notify_on_missing=['her@me.com'],
                 missing_timeout=121,
-                properties={'a':'c'},
-                keepalive_interval=61)
+                properties={'a':'c'})
 
         old.updateSlave = mock.Mock(side_effect=lambda : defer.succeed(None))
 
@@ -115,7 +111,6 @@ def test_reconfigService_attrs(self):
         self.assertEqual(old.notify_on_missing, ['her@me.com'])
         self.assertEqual(old.missing_timeout, 121)
         self.assertEqual(old.properties.getProperty('a'), 'c')
-        self.assertEqual(old.keepalive_interval, 61)
         self.assertEqual(old.registration.updates, ['bot'])
         self.assertTrue(old.updateSlave.called)