addBuildset now returns (bsid, brids)

The return value of this function is reflected through a few wrappers -
`master.addBuildset` and the various `BaseScheduler.addBuildset*`
methods.

master/buildbot/db/buildsets.py

@@ -36,6 +36,10 @@ def addBuildset(self, ssid, reason, properties, builderNames,
         each named builder, returning the resulting bsid via a Deferred.
         Arguments should be specified by keyword.
 
+        The return value is a tuple C{(bsid, brids)} where C{bsid} is the
+        inserted buildset ID and C{brids} is a list of inserted build request
+        IDs.
+
         @param ssid: id of the SourceStamp for this buildset
         @type ssid: integer
 
@@ -55,7 +59,7 @@ def addBuildset(self, ssid, reason, properties, builderNames,
 
         @param _reactor: for testing
 
-        @returns: buildset ID via a Deferred
+        @returns: buildset ID and buildrequest IDs, via a Deferred
         """
         def thd(conn):
             submitted_at = _reactor.seconds()
@@ -76,18 +80,24 @@ def thd(conn):
                          property_value=json.dumps([v,s]))
                     for k,(v,s) in properties.iteritems() ])
 
-            # and finish with a build request for each builder
-            conn.execute(self.db.model.buildrequests.insert(), [
-                dict(buildsetid=bsid, buildername=buildername,
-                     priority=0, claimed_at=0, claimed_by_name=None,
-                     claimed_by_incarnation=None, complete=0,
-                     results=-1, submitted_at=submitted_at,
-                     complete_at=None)
-                for buildername in builderNames ])
+            # and finish with a build request for each builder.  Note that
+            # sqlalchemy and the Python DBAPI do not provide a way to recover
+            # inserted IDs from a multi-row insert, so this is done one row at
+            # a time.
+            brids = []
+            ins = self.db.model.buildrequests.insert()
+            for buildername in builderNames:
+                r = conn.execute(ins,
+                    dict(buildsetid=bsid, buildername=buildername, priority=0,
+                        claimed_at=0, claimed_by_name=None,
+                        claimed_by_incarnation=None, complete=0, results=-1,
+                        submitted_at=submitted_at, complete_at=None))
+
+                brids.append(r.inserted_primary_key[0])
 
             transaction.commit()
 
-            return bsid
+            return (bsid, brids)
         return self.db.pool.do(thd)
 
     def getBuildset(self, bsid):

master/buildbot/interfaces.py

@@ -1004,14 +1004,11 @@ def getBuilder(name):
         """Retrieve the IBuilderControl object for the given Builder."""
 
 class IBuilderControl(Interface):
-    def submitBuildRequest(ss, reason, props=None, now=False):
+    def submitBuildRequest(ss, reason, props=None):
         """Create a BuildRequest, which will eventually cause a build of the
         given SourceStamp to be run on this builder. This returns a
         BuildRequestStatus object via a Deferred, which can be used to keep
-        track of the builds that are performed.
-
-        If now=True, and I have no slave attached, NoSlaveError will be
-        raised instead of queueing the request for later action."""
+        track of the builds that are performed."""
 
     def rebuildBuild(buildStatus, reason="<rebuild, no reason given>"):
         """Rebuild something we've already built before. This submits a

master/buildbot/master.py

@@ -757,15 +757,15 @@ def addBuildset(self, **kwargs):
         resulting builds.
         """
         d = self.db.buildsets.addBuildset(**kwargs)
-        def notify(bsid):
+        def notify((bsid,brids)):
             log.msg("added buildset %d to database" % bsid)
             # note that buildset additions are only reported on this master
             self._new_buildset_subs.deliver(bsid=bsid, **kwargs)
             # only deliver messages immediately if we're not polling
             if not self.db_poll_interval:
                 for bn in kwargs.get('builderNames', []):
                     self.buildRequestAdded(bsid=bsid, buildername=bn)
-            return bsid
+            return (bsid,brids)
         d.addCallback(notify)
         return d
 

master/buildbot/process/builder.py

@@ -24,7 +24,7 @@
 from buildbot import interfaces
 from buildbot.status.progress import Expectations
 from buildbot.status.builder import RETRY
-from buildbot.status.buildset import BuildSetStatus
+from buildbot.status.buildrequest import BuildRequestStatus
 from buildbot.process.properties import Properties
 from buildbot.process import buildrequest, slavebuilder
 from buildbot.process.slavebuilder import BUILDING
@@ -816,10 +816,9 @@ def add_buildset(ssid):
                     builderNames=[self.original.name],
                     ssid=ssid, reason=reason, properties=props)
         d.addCallback(add_buildset)
-        def get_brs(bsid):
-            bss = BuildSetStatus(bsid, self.master.master.status,
-                                 self.master.master.db)
-            brs = bss.getBuildRequests()[0]
+        def get_brs((bsid,brids)):
+            brs = BuildRequestStatus(self.original.name, brids[0],
+                                     self.master.master.status)
             return brs
         d.addCallback(get_brs)
         return d

master/buildbot/schedulers/base.py

@@ -258,7 +258,7 @@ def addBuildsetForLatest(self, reason='', external_idstring=None,
         @param properties: a properties object containing initial properties for
             the buildset
         @type properties: L{buildbot.process.properties.Properties}
-        @returns: buildset ID via Deferred
+        @returns: (buildset ID, buildrequest IDs) via Deferred
         """
         d = self.master.db.sourcestamps.createSourceStamp(
                 branch=branch, revision=None, repository=repository,
@@ -289,7 +289,7 @@ def addBuildsetForChanges(self, reason='', external_idstring=None,
         @param properties: a properties object containing initial properties for
             the buildset
         @type properties: L{buildbot.process.properties.Properties}
-        @returns: buildset ID via Deferred
+        @returns: (buildset ID, buildrequest IDs) via Deferred
         """
         assert changeids is not []
 
@@ -316,8 +316,9 @@ def addBuildsetForSourceStamp(self, ssid, reason='', external_idstring=None,
         Add a buildset for the given, already-existing sourcestamp.
 
         This method will add any properties provided to the scheduler
-        constructor to the buildset, and will call the master's addBuildset
-        method with the appropriate parameters.
+        constructor to the buildset, and will call the master's
+        L{BuildMaster.addBuildset} method with the appropriate parameters, and
+        return the same result.
 
         @param reason: reason for this buildset
         @type reason: unicode string
@@ -327,7 +328,7 @@ def addBuildsetForSourceStamp(self, ssid, reason='', external_idstring=None,
         @type properties: L{buildbot.process.properties.Properties}
         @param builderNames: builders to name in the buildset (defaults to
             C{self.builderNames})
-        @returns: buildset ID via Deferred
+        @returns: (buildset ID, buildrequest IDs) via Deferred
         """
         # combine properties
         if properties:

master/buildbot/schedulers/triggerable.py

@@ -46,7 +46,7 @@ def trigger(self, ssid, set_props=None):
                     properties=props)
         else:
             d = self.addBuildsetForLatest(reason=self.reason, properties=props)
-        def setup_waiter(bsid):
+        def setup_waiter((bsid,brids)):
             self._waiters[bsid] = d = defer.Deferred()
             self._updateWaiters()
             return d

master/buildbot/schedulers/trysched.py

@@ -211,7 +211,7 @@ def perspective_try(self, branch, revision, patch, repository, project,
                         reason=reason, properties=requested_props,
                         builderNames=builderNames))
         yield wfd
-        bsid = wfd.getResult()
+        (bsid,brids) = wfd.getResult()
 
         # return a remotely-usable BuildSetStatus object
         bss = BuildSetStatus(bsid, self.scheduler.master.status, db)

master/buildbot/test/fake/fakedb.py

@@ -515,8 +515,14 @@ def _newBsid(self):
 
     def addBuildset(self, **kwargs):
         bsid = kwargs['id'] = self._newBsid()
+        br_rows = []
+        for buildername in kwargs.pop('builderNames'):
+            br_rows.append(
+                    BuildRequest(buildsetid=bsid, buildername=buildername))
+        self.db.buildrequests.insertTestData(br_rows)
+
         self.buildsets[bsid] = kwargs
-        return defer.succeed(bsid)
+        return defer.succeed((bsid, [ br.id for br in br_rows ]))
 
     def subscribeToBuildset(self, schedulerid, buildsetid):
         self.buildset_subs.append((schedulerid, buildsetid))
@@ -617,6 +623,12 @@ def assertBuildset(self, bsid, expected_buildset, expected_sourcestamp):
         if buildset['properties']:
             buildset['properties'] = sorted(buildset['properties'].items())
 
+        # only add brids if we're expecting them (sometimes they're unknown)
+        if 'brids' in expected_buildset:
+            brids = [ br.id for br in self.db.buildrequests.reqs.values()
+                            if br.buildsetid == bsid ]
+            buildset['brids'] = brids
+
         if 'id' in ss:
             del ss['id']
         if not ss['changeids']:

master/buildbot/test/unit/test_db_buildsets.py

@@ -57,8 +57,11 @@ def test_addBuildset_simple(self):
             self.db.buildsets.addBuildset(ssid=234, reason='because',
                 properties={}, builderNames=['bldr'], external_idstring='extid',
                 _reactor=clock))
-        def check(bsid):
+        def check((bsid, brids)):
             def thd(conn):
+                # we should only have one brid
+                self.assertEqual(len(brids), 1)
+
                 # should see one buildset row
                 r = conn.execute(self.db.model.buildsets.select())
                 rows = [ (row.id, row.external_idstring, row.reason,
@@ -69,13 +72,14 @@ def thd(conn):
 
                 # and one buildrequests row
                 r = conn.execute(self.db.model.buildrequests.select())
-                rows = [ (row.buildsetid, row.buildername, row.priority,
-                          row.claimed_at, row.claimed_by_name,
-                          row.claimed_by_incarnation, row.complete, row.results,
-                          row.submitted_at, row.complete_at)
+
+                rows = [ (row.buildsetid, row.id, row.buildername,
+                    row.priority, row.claimed_at, row.claimed_by_name,
+                    row.claimed_by_incarnation, row.complete, row.results,
+                    row.submitted_at, row.complete_at)
                           for row in r.fetchall() ]
                 self.assertEqual(rows,
-                    [ ( bsid, 'bldr', 0, 0, None, None, 0,
+                    [ ( bsid, brids[0], 'bldr', 0, 0, None, None, 0,
                         -1, now, None) ])
             return self.db.pool.do(thd)
         d.addCallback(check)
@@ -87,8 +91,10 @@ def test_addBuildset_bigger(self):
         d.addCallback(lambda _ :
             self.db.buildsets.addBuildset(ssid=234, reason='because',
                                 properties=props, builderNames=['a', 'b']))
-        def check(bsid):
+        def check((bsid, brids)):
             def thd(conn):
+                self.assertEqual(len(brids), 2)
+
                 # should see one buildset row
                 r = conn.execute(self.db.model.buildsets.select())
                 rows = [ (row.id, row.external_idstring, row.reason,
@@ -107,10 +113,14 @@ def thd(conn):
 
                 # and two buildrequests rows (and don't re-check the default columns)
                 r = conn.execute(self.db.model.buildrequests.select())
-                rows = [ (row.buildsetid, row.buildername)
+                rows = [ (row.buildsetid, row.id, row.buildername)
                           for row in r.fetchall() ]
-                self.assertEqual(sorted(rows),
-                    [ ( bsid, 'a'), (bsid, 'b') ])
+
+                # we don't know which of the brids is assigned to which
+                # buildername, but either one will do
+                self.assertIn(sorted(rows), [
+                    [ ( bsid, brids[0], 'a'), (bsid, brids[1], 'b') ],
+                    [ ( bsid, brids[0], 'b'), (bsid, brids[1], 'a') ]])
             return self.db.pool.do(thd)
         d.addCallback(check)
         return d

master/buildbot/test/unit/test_master.py

@@ -60,18 +60,18 @@ def check(change):
     def test_buildset_subscription(self):
         self.master.db = mock.Mock()
         self.master.db.buildsets.addBuildset.return_value = \
-            defer.succeed(938593)
+            defer.succeed((938593, [19,20]))
 
         cb = mock.Mock()
         sub = self.master.subscribeToBuildsets(cb)
         self.assertIsInstance(sub, subscription.Subscription)
 
         d = self.master.addBuildset(ssid=999)
-        def check(bsid):
+        def check((bsid,brids)):
             # master called the right thing in the db component
             self.master.db.buildsets.addBuildset.assert_called_with(ssid=999)
             # addBuildset returned the right value
-            self.failUnless(bsid == 938593)
+            self.assertEqual((bsid,brids), (938593, [19,20]))
             # and the notification sub was called correctly
             cb.assert_called_with(bsid=938593, ssid=999)
         d.addCallback(check)