[Buildbot-commits] buildbot/buildbot scheduler.py,1.12,1.13 master.py,1.89,1.90

[Brian Warner]

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

Modified Files:
	scheduler.py master.py 
Log Message:
Revision: arch at buildbot.sf.net--2004/buildbot--dev--0--patch-452
Creator:  Brian Warner <warner at monolith.lothar.com>

fix a bug: config-file reload would half-clobber all Schedulers

	* buildbot/master.py (BuildMaster): remove the .schedulers
	attribute, replacing it with an allSchedulers() method that looks
	for all IService children that implement IScheduler. Having only
	one parent/child relationship means fewer opportunities for bugs.
	(BuildMaster.allSchedulers): new method
	(BuildMaster.loadConfig_Schedulers): update to use allSchedulers,
	also fix ugly bug that caused any config-file reload to
	half-forget about the earlier Schedulers, causing an exception
	when a Change arrived and was handed to a half-connected
	Scheduler. The exception was in scheduler.py line 54ish:
	  self.parent.submitBuildSet(bs)
	  exceptions.AttributeError: 'NoneType' object has no attribute
	  'submitBuildSet'
	(BuildMaster.addChange): update to use allSchedulers()

	* buildbot/scheduler.py (BaseScheduler.__implements__): fix this
	to work properly with twisted-1.3.0, where you must explicitly
	include the __implements__ from parent classes
	(BaseScheduler.__repr__): make it easier to distinguish distinct
	instances
	(BaseUpstreamScheduler.__implements__): same

	* buildbot/status/builder.py (Status.getSchedulers): update to
	use allSchedulers()
	* buildbot/test/test_run.py (Run.testMaster): same
	* buildbot/test/test_dependencies.py (Dependencies.findScheduler): same
	* buildbot/test/test_config.py (ConfigTest.testSchedulers): same,
	make sure Scheduler instances are left alone when an identical
	config file is reloaded
	(ConfigElements.testSchedulers): make sure Schedulers are properly
	comparable

	* Makefile (TRIALARGS): my local default Twisted version is now
	2.1.0, update the trial arguments accordingly


Index: master.py
===================================================================
RCS file: /cvsroot/buildbot/buildbot/buildbot/master.py,v
retrieving revision 1.89
retrieving revision 1.90
diff -u -d -r1.89 -r1.90
--- master.py	26 Nov 2005 02:14:31 -0000	1.89
+++ master.py	3 Jan 2006 09:26:40 -0000	1.90
@@ -591,7 +591,6 @@
 
         self.statusTargets = []
 
-        self.schedulers = []
         self.bots = []
         # this ChangeMaster is a dummy, only used by tests. In the real
         # buildmaster, where the BuildMaster instance is activated
@@ -752,8 +751,7 @@
         # Schedulers are service.MultiServices and thus iterable.
         assert type(schedulers) in (list, tuple)
         for s in schedulers:
-            assert (interfaces.IScheduler(s, None)
-                    or interfaces.IUpstreamScheduler(s, None))
+            assert interfaces.IScheduler(s, None)
         assert type(status) in (list, tuple)
         for s in status:
             assert interfaces.IStatusReceiver(s, None)
@@ -907,14 +905,22 @@
         d.addCallback(addNewOnes)
         return d
 
+    def allSchedulers(self):
+        # TODO: when twisted-1.3 compatibility is dropped, switch to the
+        # providedBy form, because it's faster (no actual adapter lookup)
+        return [child for child in self
+                #if interfaces.IScheduler.providedBy(child)]
+                if interfaces.IScheduler(child, None)]
+
+
     def loadConfig_Schedulers(self, newschedulers):
-        old = [s for s in self.schedulers if s not in newschedulers]
-        [self.schedulers.remove(s) for s in old]
-        dl = [defer.maybeDeferred(s.disownServiceParent) for s in old]
+        oldschedulers = self.allSchedulers()
+        removed = [s for s in oldschedulers if s not in newschedulers]
+        added = [s for s in newschedulers if s not in oldschedulers]
+        dl = [defer.maybeDeferred(s.disownServiceParent) for s in removed]
         def addNewOnes(res):
-            [s.setServiceParent(self)
-             for s in newschedulers if s not in self.schedulers]
-            self.schedulers = newschedulers
+            for s in added:
+                s.setServiceParent(self)
         d = defer.DeferredList(dl, fireOnOneErrback=1)
         d.addCallback(addNewOnes)
         return d
@@ -1004,7 +1010,7 @@
 
 
     def addChange(self, change):
-        for s in self.schedulers:
+        for s in self.allSchedulers():
             s.addChange(change)
 
     def submitBuildSet(self, bs):

Index: scheduler.py
===================================================================
RCS file: /cvsroot/buildbot/buildbot/buildbot/scheduler.py,v
retrieving revision 1.12
retrieving revision 1.13
diff -u -d -r1.12 -r1.13
--- scheduler.py	22 Dec 2005 21:00:01 -0000	1.12
+++ scheduler.py	3 Jan 2006 09:26:40 -0000	1.13
@@ -21,14 +21,16 @@
     if implements:
         implements(interfaces.IScheduler)
     else:
-        __implements__ = interfaces.IScheduler,
+        __implements__ = (interfaces.IScheduler,
+                          service.MultiService.__implements__)
 
     def __init__(self, name):
         service.MultiService.__init__(self)
         self.name = name
 
     def __repr__(self):
-        return "<Scheduler '%s'>" % self.name
+        # TODO: why can't id() return a positive number? %d is ugly.
+        return "<Scheduler '%s' at %d>" % (self.name, id(self))
 
     def submit(self, bs):
         self.parent.submitBuildSet(bs)
@@ -40,7 +42,8 @@
     if implements:
         implements(interfaces.IUpstreamScheduler)
     else:
-        __implements__ = interfaces.IUpstreamScheduler,
+        __implements__ = (interfaces.IUpstreamScheduler,
+                          BaseScheduler.__implements__)
 
     def __init__(self, name):
         BaseScheduler.__init__(self, name)