Refactor some mildly confusing ifs

buildbot · Nov 5, 2014 · 290c92e · 290c92e
1 parent f4a4618
commit 290c92e
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 22 deletions.
diff --git a/master/buildbot/schedulers/basic.py b/master/buildbot/schedulers/basic.py
@@ -134,16 +134,11 @@ def gotChange(self, change, important):
 
         timer_name = self.getTimerNameForChange(change)
 
-        # if we have a treeStableTimer, then record the change's importance
-        # and:
+        # if we have a treeStableTimer
         # - for an important change, start the timer
         # - for an unimportant change, reset the timer if it is running
-        d = self.master.db.schedulers.classifyChanges(
-            self.objectid, {change.number: important})
 
-        def fix_timer(_):
-            if not important and not self._stable_timers[timer_name]:
-                return
+        if important or self._stable_timers[timer_name]:
             if self._stable_timers[timer_name]:
                 self._stable_timers[timer_name].cancel()
 
@@ -152,8 +147,10 @@ def fire_timer():
                 d.addErrback(log.err, "while firing stable timer")
             self._stable_timers[timer_name] = self._reactor.callLater(
                 self.treeStableTimer, fire_timer)
-        d.addCallback(fix_timer)
-        return d
+
+        # record the change's importance
+        return self.master.db.schedulers.classifyChanges(
+            self.objectid, {change.number: important})
 
     @defer.inlineCallbacks
     def scanExistingClassifiedChanges(self):
@@ -188,13 +185,11 @@ def getChangeClassificationsForTimer(self, objectid, timer_name):
     @util.deferredLocked('_stable_timers_lock')
     @defer.inlineCallbacks
     def stableTimerFired(self, timer_name):
-        # if the service has already been stopped then just bail out
-        if not self._stable_timers[timer_name]:
+        # delete this now-fired timer, if the service has already been stopped
+        # then just bail out
+        if not self._stable_timers.pop(timer_name, None):
             return
 
-        # delete this now-fired timer
-        del self._stable_timers[timer_name]
-
         classifications = \
             yield self.getChangeClassificationsForTimer(self.objectid,
                                                         timer_name)

diff --git a/master/buildbot/schedulers/timed.py b/master/buildbot/schedulers/timed.py
@@ -304,17 +304,15 @@ def __init__(self, name, builderNames, minute=0, hour='*',
     def preStartConsumingChanges(self):
         return defer.succeed(None)
 
+    @defer.inlineCallbacks
     def startTimedSchedulerService(self):
         if self.onlyIfChanged:
-            d = self.preStartConsumingChanges()
-
-            d.addCallback(lambda _:
-                          self.startConsumingChanges(fileIsImportant=self.fileIsImportant,
-                                                     change_filter=self.change_filter,
-                                                     onlyImportant=self.onlyImportant))
-            return d
+            yield self.preStartConsumingChanges()
+            yield self.startConsumingChanges(fileIsImportant=self.fileIsImportant,
+                                             change_filter=self.change_filter,
+                                             onlyImportant=self.onlyImportant)
         else:
-            return self.master.db.schedulers.flushChangeClassifications(self.objectid)
+            yield self.master.db.schedulers.flushChangeClassifications(self.objectid)
 
     def gotChange(self, change, important):
         # both important and unimportant changes on our branch are recorded, as