[Buildbot-commits] buildbot/buildbot/process base.py,1.58,1.59 builder.py,1.30,1.31

[Brian Warner]

Update of /cvsroot/buildbot/buildbot/buildbot/process
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv5665/buildbot/process

Modified Files:
	base.py builder.py 
Log Message:
Revision: arch at buildbot.sf.net--2004/buildbot--dev--0--patch-320
Creator:  Brian Warner <warner at lothar.com>

remove inappropriate already-attached warning

	* buildbot/process/builder.py (Builder.attached): remove the
	already-attached warning, this situation is normal. Add some
	comments explaining it.

--This line, and those below, will be ignored--
Files to commit:
   <can't compute list>

This list might be incomplete or outdated if editing the log
message was not invoked from an up-to-date changes buffer!


Index: base.py
===================================================================
RCS file: /cvsroot/buildbot/buildbot/buildbot/process/base.py,v
retrieving revision 1.58
retrieving revision 1.59
diff -u -d -r1.58 -r1.59
--- base.py	17 Aug 2005 02:15:37 -0000	1.58
+++ base.py	7 Oct 2005 18:37:21 -0000	1.59
@@ -258,6 +258,11 @@
         first Step. It returns a Deferred which will fire when the build
         finishes. This Deferred is guaranteed to never errback."""
 
+        # we are taking responsibility for watching the connection to the
+        # remote. This responsibility was held by the Builder until our
+        # startBuild was called, and will not return to them until we fire
+        # the Deferred returned by this method.
+
         log.msg("%s.startBuild" % self)
         self.build_status = build_status
         self.slavebuilder = slavebuilder

Index: builder.py
===================================================================
RCS file: /cvsroot/buildbot/buildbot/buildbot/process/builder.py,v
retrieving revision 1.30
retrieving revision 1.31
diff -u -d -r1.30 -r1.31
--- builder.py	31 Aug 2005 01:51:43 -0000	1.30
+++ builder.py	7 Oct 2005 18:37:21 -0000	1.31
@@ -307,11 +307,24 @@
         """
         for s in self.slaves.keys():
             if s.slave == slave:
-                # already attached to them. TODO: how does this ever get
-                # reached?
-                log.msg("%s.attached: WEIRD slave %s already attached"
-                        % (self, slave))
+                # already attached to them. This is fairly common, since
+                # attached() gets called each time we receive the builder
+                # list from the slave, and we ask for it each time we add or
+                # remove a builder. So if the slave is hosting builders
+                # A,B,C, and the config file changes A, we'll remove A and
+                # re-add it, triggering two builder-list requests, getting
+                # two redundant calls to attached() for B, and another two
+                # for C.
+                #
+                # Therefore, when we see that we're already attached, we can
+                # just ignore it. TODO: build a diagram of the state
+                # transitions here, I'm concerned about sb.attached() failing
+                # and leaving self.slaves[sb] stuck at 'attaching', and about
+                # the detached() message arriving while there's some
+                # transition pending such that the response to the transition
+                # re-vivifies self.slaves[sb]
                 return defer.succeed(self)
+
         sb = SlaveBuilder(self)
         self.slaves[sb] = "attaching"
         d = sb.attached(slave, remote, commands)
@@ -331,7 +344,8 @@
     def _not_attached(self, why, slave):
         # already log.err'ed by SlaveBuilder._attachFailure
         # TODO: make this .addSlaveEvent?
-        # TODO: remove from self.slaves
+        # TODO: remove from self.slaves (except that detached() should get
+        #       run first, right?)
         self.builder_status.addPointEvent(['failed', 'connect',
                                            slave.slave.slavename])
         # TODO: add an HTMLLogFile of the exception
@@ -467,6 +481,12 @@
         sb.finishBuild()
         if sb in self.slaves:
             self.slaves[sb] = "idle"
+        else:
+            # if the startBuild message failed because we lost the slave, our
+            # detacted() method will have already fired, removing the
+            # SlaveBuilder from self.slaves . This test is here to make sure
+            # we don't re-create the old self.slaves[sb] entry.
+            pass
 
         log.msg("re-queueing the BuildRequest")
         self.building.remove(build)