Skip to content

Commit

Permalink
Remove remoteShutdown's buildslave argument
Browse files Browse the repository at this point in the history
The connection already has that object from the constructor.
  • Loading branch information
djmitche committed Sep 28, 2013
1 parent 869077f commit da678e4
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 17 deletions.
2 changes: 1 addition & 1 deletion master/buildbot/buildslave/base.py
Expand Up @@ -538,7 +538,7 @@ def shutdown(self):
log.msg("no remote; slave is already shut down")
return

yield self.conn.remoteShutdown(self)
yield self.conn.remoteShutdown()

def maybeShutdown(self):
"""Shut down this slave if it has been asked to shut down gracefully,
Expand Down
2 changes: 1 addition & 1 deletion master/buildbot/buildslave/protocols/base.py
Expand Up @@ -58,7 +58,7 @@ def remoteSetBuilderList(self, builders):
def startCommands(self, remoteCommand, builderName, commandId, commandName, args):
raise NotImplementedError

def remoteShutdown(self, buildslave):
def remoteShutdown(self):
raise NotImplementedError

def remoteStartBuild(self, builderName):
Expand Down
8 changes: 4 additions & 4 deletions master/buildbot/buildslave/protocols/pb.py
Expand Up @@ -159,7 +159,7 @@ def doKeepalive(self):
return self.mind.callRemote('print', message="keepalive")

@defer.inlineCallbacks
def remoteShutdown(self, buildslave):
def remoteShutdown(self):
# First, try the "new" way - calling our own remote's shutdown
# method. The method was only added in 0.8.3, so ignore NoSuchMethod
# failures.
Expand All @@ -184,13 +184,13 @@ def check_connlost(f):
# remote builder, which will cause the slave buildbot process to exit.
def old_way():
d = None
for b in buildslave.slavebuilders.values():
for b in self.buildslave.slavebuilders.values():
if b.remote:
d = b.mind.callRemote("shutdown")
break

if d:
log.msg("Shutting down (old) slave: %s" % buildslave.slavename)
log.msg("Shutting down (old) slave: %s" % self.buildslave.slavename)
# The remote shutdown call will not complete successfully since the
# buildbot process exits almost immediately after getting the
# shutdown request.
Expand All @@ -199,7 +199,7 @@ def old_way():
# shutdown as expected.
def _errback(why):
if why.check(pb.PBConnectionLost):
log.msg("Lost connection to %s" % buildslave.slavename)
log.msg("Lost connection to %s" % self.buildslave.slavename)
else:
log.err("Unexpected error when trying to shutdown %s" % buildslave.slavename)
d.addErrback(_errback)
Expand Down
6 changes: 4 additions & 2 deletions master/buildbot/test/unit/test_buildslave_base.py
Expand Up @@ -18,6 +18,7 @@
from twisted.internet import defer, task, reactor
from buildbot import config, locks
from buildbot.buildslave import base
from buildbot.buildslave.protocols import base as protocols_base
from buildbot.test.fake import fakemaster, fakedb, bslavemanager
from buildbot.test.fake.botmaster import FakeBotMaster

Expand All @@ -38,7 +39,8 @@ def createBuildslave(self, name='bot', password='pass', attached=False, **kwargs
slave = self.ConcreteBuildSlave(name, password, **kwargs)
slave.master = self.master
slave.botmaster = self.botmaster
if attached: slave.conn = mock.Mock()
if attached:
slave.conn = mock.Mock(spec=protocols_base.Connection)
return slave

def test_constructor_minimal(self):
Expand Down Expand Up @@ -327,7 +329,7 @@ def test_slave_shutdown(self):
yield slave.startService()

yield slave.shutdown()
slave.conn.remoteShutdown.assert_called_with(slave)
slave.conn.remoteShutdown.assert_called_with()

@defer.inlineCallbacks
def test_slave_shutdown_not_connected(self):
Expand Down
6 changes: 2 additions & 4 deletions master/buildbot/test/unit/test_buildslave_protocols_pb.py
Expand Up @@ -165,10 +165,8 @@ def test_doKeepalive(self):
def test_remoteShutdown(self):
self.mind.callRemote.return_value = defer.succeed(None)
conn = pb.Connection(self.master, self.buildslave, self.mind)
buildslave = None
# "buildslave" required to shutdown slave in "old way", since
# this feature deprecated and hard to test in reality it's not tested
conn.remoteShutdown(buildslave)
# note that we do not test the "old way", as it is now *very* old.
conn.remoteShutdown()

self.mind.callRemote.assert_called_with('shutdown')

Expand Down
2 changes: 1 addition & 1 deletion master/buildbot/test/util/protocols.py
Expand Up @@ -54,7 +54,7 @@ def startCommands(self, remoteCommand, builderName, commandId,

def test_sig_remoteShutdown(self):
@self.assertArgSpecMatches(self.conn.remoteShutdown)
def remoteShutdown(self, buildslave):
def remoteShutdown(self):
pass

def test_sig_remoteStartBuild(self):
Expand Down
6 changes: 2 additions & 4 deletions master/docs/developer/cls-protocols.rst
Expand Up @@ -69,13 +69,11 @@ to know about protocol calls or handle protocol specific exceptions.
Start command on slave
.. py:method:: remoteShutdown(buildslave)
.. py:method:: remoteShutdown()
:param buildslave: buildbot.buildslave.base.BuildSlave instance
:returns: Deferred
Shutdown slave, "buildslave" required to shutdown old slaves (saved for
backward compatability)
Shutdown the slave, causing its process to halt permanently.
.. py:method:: remoteStartBuild(builderName)
Expand Down

0 comments on commit da678e4

Please sign in to comment.