Skip to content

Commit

Permalink
Revert "Merge branch 'SlaveInfo' of git://github.com/jaredgrubb/build…
Browse files Browse the repository at this point in the history
…bot"

This reverts commit 1c59734, reversing
changes made to bf5c148.

See #880 for details on why
this was reverted.

Conflicts:
	master/buildbot/status/slave.py
	master/docs/relnotes/index.rst
  • Loading branch information
djmitche committed Dec 23, 2013
1 parent 0c33350 commit f518276
Show file tree
Hide file tree
Showing 11 changed files with 36 additions and 569 deletions.
36 changes: 20 additions & 16 deletions master/buildbot/buildslave/base.py
Expand Up @@ -178,7 +178,22 @@ def _lockReleased(self):
return # oh well..
self.botmaster.maybeStartBuildsForSlave(self.slavename)

def _saveSlaveInfoDict(self, slaveinfo):
def _applySlaveInfo(self, info):
if not info:
return

self.slave_status.setAdmin(info.get("admin"))
self.slave_status.setHost(info.get("host"))
self.slave_status.setAccessURI(info.get("access_uri"))
self.slave_status.setVersion(info.get("version"))

def _saveSlaveInfoDict(self):
slaveinfo = {
'admin': self.slave_status.getAdmin(),
'host': self.slave_status.getHost(),
'access_uri': self.slave_status.getAccessURI(),
'version': self.slave_status.getVersion(),
}
return self.master.db.buildslaves.updateBuildslave(
name=self.slavename,
slaveinfo=slaveinfo,
Expand All @@ -192,16 +207,10 @@ def applyInfo(buildslave):
if buildslave is None:
return

self.updateSlaveInfo(**buildslave['slaveinfo'])
self._applySlaveInfo(buildslave.get('slaveinfo'))

return d

def updateSlaveInfo(self, **kwargs):
self.slave_status.updateInfo(**kwargs)

def getSlaveInfo(self, key, default=None):
return self.slave_status.getInfo(key, default)

def setServiceParent(self, parent):
# botmaster needs to set before setServiceParent which calls startService
self.botmaster = parent
Expand All @@ -211,7 +220,6 @@ def setServiceParent(self, parent):
def startService(self):
self.updateLocks()
self.startMissingTimer()
self.slave_status.addInfoWatcher(self._saveSlaveInfoDict)
d = self._getSlaveInfo()
d.addCallback(lambda _: service.MultiService.startService(self))
return d
Expand Down Expand Up @@ -266,7 +274,6 @@ def reconfigService(self, new_config):
new_config)

def stopService(self):
self.slave_status.removeInfoWatcher(self._saveSlaveInfoDict)
if self.registration:
self.registration.unregister()
self.registration = None
Expand Down Expand Up @@ -471,12 +478,7 @@ def _commands_unavailable(why):
def _accept_slave(res):
self.slave_status.setConnected(True)

self.slave_status.updateInfo(
admin=state.get("admin"),
host=state.get("host"),
access_uri=state.get("access_uri"),
version=state.get("version"),
)
self._applySlaveInfo(state)

self.slave_commands = state.get("slave_commands")
self.slave_environ = state.get("slave_environ")
Expand All @@ -494,6 +496,8 @@ def _accept_slave(res):
self.stopMissingTimer()
self.botmaster.master.status.slaveConnected(self.slavename)

d.addCallback(lambda _: self._saveSlaveInfoDict())

d.addCallback(lambda _: self.updateSlave())

d.addCallback(lambda _:
Expand Down
19 changes: 0 additions & 19 deletions master/buildbot/process/properties.py
Expand Up @@ -395,14 +395,6 @@ def getRenderingFor(self, build):
return {}


class _SlaveInfoDict(util.ComparableMixin, object):
implements(IRenderable)

def getRenderingFor(self, build):
slave = build.getBuild().slavebuilder.slave
return slave.slave_status.getInfoAsDict()


class _Lazy(util.ComparableMixin, object):
implements(IRenderable)

Expand Down Expand Up @@ -482,17 +474,6 @@ def _parse_src(arg):
codebase = attr = repl = None
return _SourceStampDict(codebase), attr, repl

@staticmethod
def _parse_slave(arg):
try:
key, repl = arg.split(":", 1)
except ValueError:
key, repl = arg, None
if not Interpolate.identifier_re.match(key):
config.error("Keyword must be alphanumeric for slave-info Interpolation '%s'" % arg)
key = repl = None
return _SlaveInfoDict(), key, repl

def _parse_kw(self, arg):
try:
kw, repl = arg.split(":", 1)
Expand Down
70 changes: 12 additions & 58 deletions master/buildbot/status/slave.py
Expand Up @@ -15,18 +15,19 @@

import time

import copy

from buildbot import interfaces
from buildbot.util import ascii2unicode
from buildbot.util import json
from buildbot.util.eventual import eventually
from zope.interface import implements


class SlaveStatus:
implements(interfaces.ISlaveStatus)

admin = None
host = None
access_uri = None
version = None
connected = False
graceful_shutdown = False
paused = False
Expand All @@ -37,24 +38,22 @@ def __init__(self, name):
self.runningBuilds = []
self.graceful_callbacks = []
self.pause_callbacks = []
self.info = {}
self.info_change_callbacks = []
self.connect_times = []

def getName(self):
return self.name

def getAdmin(self):
return self.getInfo('admin')
return self.admin

def getHost(self):
return self.getInfo('host')
return self.host

def getAccessURI(self):
return self.getInfo('access_uri')
return self.access_uri

def getVersion(self):
return self.getInfo('version')
return self.version

def isConnected(self):
return self.connected
Expand All @@ -73,16 +72,16 @@ def getConnectCount(self):
return len([t for t in self.connect_times if t > then])

def setAdmin(self, admin):
self.updateInfo(admin=admin)
self.admin = ascii2unicode(admin)

def setHost(self, host):
self.updateInfo(host=host)
self.host = ascii2unicode(host)

def setAccessURI(self, access_uri):
self.updateInfo(access_uri=access_uri)
self.access_uri = access_uri

def setVersion(self, version):
self.updateInfo(version=version)
self.version = version

def setConnected(self, isConnected):
self.connected = isConnected
Expand Down Expand Up @@ -140,50 +139,6 @@ def removeGracefulWatcher(self, watcher):
if watcher in self.graceful_callbacks:
self.graceful_callbacks.remove(watcher)

def getInfoAsDict(self):
return copy.deepcopy(self.info)

def setInfoDict(self, info):
self.info = info

def getInfo(self, key, default=None):
return self.info.get(key, default)

def updateInfo(self, **kwargs):
# round-trip the value through json to 'normalize' it and
# to ensure bad values dont get stuffed into the dictionary
new_values = json.loads(json.dumps(kwargs))

for special_key in ['admin', 'host']:
if special_key in new_values:
new_values[special_key] = ascii2unicode(new_values[special_key])

# try to see if anything changed (so we should inform watchers)
for k, v in new_values.iteritems():
if k not in self.info:
break
if self.info[k] != v:
break
else:
# nothing changed so just bail now
return

self.info.update(new_values)

for watcher in self.info_change_callbacks:
eventually(watcher, self.getInfoAsDict())

def hasInfo(self, key):
return key in self.info

def addInfoWatcher(self, watcher):
if not watcher in self.info_change_callbacks:
self.info_change_callbacks.append(watcher)

def removeInfoWatcher(self, watcher):
if watcher in self.info_change_callbacks:
self.info_change_callbacks.remove(watcher)

def asDict(self):
result = {}
# Constant
Expand All @@ -196,5 +151,4 @@ def asDict(self):
result['version'] = self.getVersion()
result['connected'] = self.isConnected()
result['runningBuilds'] = [b.asDict() for b in self.getRunningBuilds()]
result['info'] = self.getInfoAsDict()
return result
19 changes: 0 additions & 19 deletions master/buildbot/steps/master.py
Expand Up @@ -200,25 +200,6 @@ def start(self):
self.finished(SUCCESS)


class SetSlaveInfo(BuildStep):
name = 'SetSlaveInfo'
description = ['Setting']
descriptionDone = ['Set']

def start(self):
update = self.getSlaveInfoUpdate()

if update and isinstance(update, dict):
self.build.slavebuilder.slave.updateSlaveInfo(update)

self.step_status.setText(self.describe(done=True))
self.finished(SUCCESS)

def getSlaveInfoUpdate(self):
# should subclass; return a dictionary to update info with
return {}


class LogRenderable(BuildStep):
name = 'LogRenderable'
description = ['Logging']
Expand Down
13 changes: 2 additions & 11 deletions master/buildbot/test/fake/fakedb.py
Expand Up @@ -25,7 +25,6 @@
from buildbot.db import buildrequests
from buildbot.util import datetime2epoch
from buildbot.util import json
from copy import deepcopy
from twisted.internet import defer
from twisted.internet import reactor

Expand Down Expand Up @@ -865,17 +864,10 @@ def insertTestData(self, rows):
})

def getBuildslaves(self):
return defer.succeed([{
'name': s['name'],
'slaveid': s['slaveid'],
} for s in self.buildslaves])
return defer.succeed([])

def getBuildslaveByName(self, name):
buildslave = self._getBuildslaveByName(name)
if buildslave is not None:
# XX: make a deep-copy to avoid side effects
buildslave = deepcopy(buildslave)
return defer.succeed(buildslave)
return defer.succeed(self._getBuildslaveByName(name))

def _getBuildslaveByName(self, name):
for slave in self.buildslaves:
Expand All @@ -884,7 +876,6 @@ def _getBuildslaveByName(self, name):
return None

def updateBuildslave(self, name, slaveinfo):
slaveinfo = deepcopy(slaveinfo)
slave = self._getBuildslaveByName(name)
if slave is None:
self.insertTestData([
Expand Down
43 changes: 2 additions & 41 deletions master/buildbot/test/unit/test_buildslave_base.py
Expand Up @@ -22,7 +22,6 @@
from buildbot.test.fake import fakemaster
from buildbot.test.fake import pbmanager
from buildbot.test.fake.botmaster import FakeBotMaster
from buildbot.util import eventual
from twisted.internet import defer
from twisted.internet import reactor
from twisted.internet import task
Expand All @@ -42,9 +41,6 @@ def setUp(self):
self.patch(reactor, 'callLater', self.clock.callLater)
self.patch(reactor, 'seconds', self.clock.seconds)

def tearDown(self):
self.clock.pump([0])

def createBuildslave(self, name='bot', password='pass', **kwargs):
slave = self.ConcreteBuildSlave(name, password, **kwargs)
slave.master = self.master
Expand Down Expand Up @@ -266,8 +262,7 @@ def test_startService_getSlaveInfo_fromDb(self):
'admin': 'TheAdmin',
'host': 'TheHost',
'access_uri': 'TheURI',
'version': 'TheVersion',
'key': 'value',
'version': 'TheVersion'
})
])
slave = self.createBuildslave()
Expand All @@ -278,35 +273,6 @@ def test_startService_getSlaveInfo_fromDb(self):
self.assertEqual(slave.slave_status.getHost(), 'TheHost')
self.assertEqual(slave.slave_status.getAccessURI(), 'TheURI')
self.assertEqual(slave.slave_status.getVersion(), 'TheVersion')
self.assertEqual(slave.slave_status.getInfo('key'), 'value')

@defer.inlineCallbacks
def test_startService_setSlaveInfo_UpdatesDb(self):
self.master.db.insertTestData([
fakedb.Buildslave(name='bot', info={
'admin': 'TheAdmin',
'host': 'TheHost',
'access_uri': 'TheURI',
'version': 'TheVersion',
'key': 'value',
})
])
slave = self.createBuildslave()
yield slave.startService()

# change a value
slave.slave_status.updateInfo(key='new-value')
self.clock.pump([0]) # we overrode the reactor, so gotta force the calls
yield eventual.flushEventualQueue()

# and the db is updated too:
slave_db = yield self.master.db.buildslaves.getBuildslaveByName("bot")

self.assertEqual(slave_db['slaveinfo']['admin'], 'TheAdmin')
self.assertEqual(slave_db['slaveinfo']['host'], 'TheHost')
self.assertEqual(slave_db['slaveinfo']['access_uri'], 'TheURI')
self.assertEqual(slave_db['slaveinfo']['version'], 'TheVersion')
self.assertEqual(slave_db['slaveinfo']['key'], 'new-value')

def createRemoteBot(self):
class Bot():
Expand Down Expand Up @@ -437,8 +403,7 @@ def test_attached_slaveInfoUpdates(self):
'admin': 'WrongAdmin',
'host': 'WrongHost',
'access_uri': 'WrongURI',
'version': 'WrongVersion',
'key': 'value',
'version': 'WrongVersion'
})
])
slave = self.createBuildslave()
Expand All @@ -457,10 +422,6 @@ def test_attached_slaveInfoUpdates(self):
self.assertEqual(slave.slave_status.getHost(), 'TheHost')
self.assertEqual(slave.slave_status.getAccessURI(), 'TheURI')
self.assertEqual(slave.slave_status.getVersion(), 'TheVersion')
self.assertEqual(slave.slave_status.getInfo('key'), 'value')

self.clock.pump([0]) # we overrode the reactor, so gotta force the calls
yield eventual.flushEventualQueue()

# and the db is updated too:
buildslave = yield self.master.db.buildslaves.getBuildslaveByName("bot")
Expand Down

0 comments on commit f518276

Please sign in to comment.