- improve master/db/migrate/versions/037_buildrequests_builderid.py …

…as dustin suggested

 - various minor fix
 - Done:
   - DB API changes reflected in the doc
   - DATA API changes reflected in the doc
   - DATA API for buildrequest's dropping /builders/i:buildername/buildrequests or not (code drops, path still in place)
   - DATA API changes need to be reflected in the FakeData API
   - trigger steps now gets {builderid:buidrequestid} back from the sched.trigger call
   - Try Schedulers's RemoteBuildSetStatus.getBuildRequest should return builder names but returns builder ids
 - todo:
   - SchedulerMixin._addBuildsetReturnValue returns buildernames, but should return builderids
   - BuilderStatus.getPendingBuildRequestStatuses calls db.getBuildRequests (... buildername=...". It's Ok to just assert 0 in this method.
     The whole class will go away soon. Just don't leave it subtly broken

master/buildbot/data/buildrequests.py

@@ -12,7 +12,6 @@
 # Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 #
 # Copyright Buildbot Team Members
-import copy
 
 from buildbot.data import base
 from buildbot.data import types
@@ -69,7 +68,6 @@ class BuildRequestsEndpoint(Db2DataMixin, base.Endpoint):
     isCollection = True
     pathPatterns = """
         /buildrequests
-        /builders/i:buildername/buildrequests
         /builders/n:builderid/buildrequests
     """
     rootLinkName = 'buildrequests'
@@ -132,7 +130,6 @@ def generateEvent(self, brids, event):
         for _id in brids:
             # get the build and munge the result for the notification
             br = yield self.master.data.get(('buildrequests', str(_id)))
-            br = copy.deepcopy(br)
             self.produceEvent(br, event)
 
     @defer.inlineCallbacks

master/buildbot/data/buildsets.py

@@ -166,7 +166,7 @@ def addBuildset(self, waited_for, scheduler=None, sourcestamps=[], reason=u'',
 
         log.msg("added buildset %d to database" % bsid)
 
-        # if there are no builderNames, then this is done already, so send the
+        # if there are no builders, then this is done already, so send the
         # appropriate messages for that
         if not builderids:
             yield self.maybeBuildsetComplete(bsid, _reactor=_reactor)

master/buildbot/db/buildsets.py

@@ -105,7 +105,6 @@ def thd(conn):
             br_tbl = self.db.model.buildrequests
             ins = br_tbl.insert()
             for builderid in builderids:
-                # self.checkLength(br_tbl.c.buildername, buildername)
                 r = conn.execute(ins,
                                  dict(buildsetid=bsid, builderid=builderid, priority=0,
                                       claimed_at=0, claimed_by_name=None,

master/buildbot/db/migrate/versions/037_buildrequests_builderid.py

@@ -13,6 +13,7 @@
 #
 # Copyright Buildbot Team Members
 
+import hashlib
 import sqlalchemy as sa
 
 from migrate import changeset
@@ -42,22 +43,32 @@ def migrate_data(migrate_engine):
     buildrequests = sa.Table('buildrequests', metadata, autoload=True)
     builders = sa.Table('builders', metadata, autoload=True)
 
-    def findbuilderid(buildername):
-        tbl = builders
-
-        q = sa.select([tbl.c.id, tbl.c.name])
-        r = migrate_engine.execute(q)
-        row = r.fetchone()
-        r.close()
-
-        if row:
-            return row.id
+    bName2bID = dict()
+    q = sa.select([builders.c.id, builders.c.name])
+    for row in migrate_engine.execute(q).fetchall():
+        bName2bID[row.name] = row.id
+
+    def hashColumns(*args):
+        # copy paste from buildbot/db/base.py
+        def encode(x):
+            try:
+                return x.encode('utf8')
+            except AttributeError:
+                if x is None:
+                    return '\xf5'
+                return str(x)
+        return hashlib.sha1('\0'.join(map(encode, args))).hexdigest()
 
-        r = migrate_engine.execute(tbl.insert(), [{
-            'name': buildername,
-            'name_hash': buildername,   # XXX TODO !
-        }])
-        return r.inserted_primary_key[0]
+    def findbuilderid(buildername):
+        bid = bName2bID.get(buildername)
+        if bid is None: 
+            r = migrate_engine.execute(builders.insert(), [{
+                'name': buildername,
+                'name_hash': hashColumns(buildername),
+            }])
+            bid = r.inserted_primary_key[0]
+            bName2bID[buildername] = bid
+        return bid
 
     c = buildrequests.c
     q = sa.select([c.id, c.buildername])

master/buildbot/schedulers/trysched.py

@@ -243,7 +243,10 @@ class RemoteBuildSetStatus(pb.Referenceable):
     def __init__(self, master, bsid, brids):
         self.master = master
         self.bsid = bsid
-        self.brids = brids
+        self.brids = dict()
+        for builderid, brid in self.brids.iteritems():
+            builderDict = yield self.master.data.get(('builders', builderid))
+            self.brids[builderDict['name']] = brid
 
     def remote_getBuildRequests(self):
         return [(n, RemoteBuildRequest(self.master, n, brid))

master/buildbot/steps/trigger.py

@@ -160,14 +160,14 @@ def addBuildUrls(self, rclist):
                 results, brids = results
 
             if was_cb:  # errors were already logged in worstStatus
-                for buildername, br in brids.iteritems():
+                for builderid, br in brids.iteritems():
+                    builderDict = yield self.master.data.get(("builders", builderid))
                     builds = yield self.master.db.builds.getBuilds(buildrequestid=br)
                     for build in builds:
                         num = build['number']
-                        builderid = yield self.master.data.updates.findBuilderId(ascii2unicode(buildername))
                         url = self.master.status.getURLForBuild(builderid, num)
                         yield self.addURL("%s: %s #%d" % (statusToString(results),
-                                                          buildername, num), url)
+                                                          builderDict["name"], num), url)
 
     @defer.inlineCallbacks
     def run(self):

master/buildbot/test/unit/test_steps_trigger.py

@@ -113,8 +113,8 @@ def allSchedulers():
             return [a, b]
         m.allSchedulers = allSchedulers
 
-        a.brids = {'A': 11}
-        b.brids = {'B': 22}
+        a.brids = {77: 11}
+        b.brids = {78: 22}
 
         make_fake_br = lambda brid, builderid: fakedb.BuildRequest(
             id=brid, buildsetid=BRID_TO_BSID(brid), builderid=builderid)
@@ -203,13 +203,13 @@ def expectTriggeredLinks(self, *args):
                 (('b #22', 'baseurl/#buildrequests/22'), {}))
         if 'a' in args:
             self.exp_added_urls.append((('success: A #4011',
-                                         'baseurl/#builders/1/builds/4011'), {}))
+                                         'baseurl/#builders/77/builds/4011'), {}))
         if 'b' in args:
             self.exp_added_urls.append((('success: B #4022',
-                                         'baseurl/#builders/2/builds/4022'), {}))
+                                         'baseurl/#builders/78/builds/4022'), {}))
         if 'afailed' in args:
             self.exp_added_urls.append((('failure: A #4011',
-                                         'baseurl/#builders/1/builds/4011'), {}))
+                                         'baseurl/#builders/77/builds/4011'), {}))
 
     # tests
     def test_no_schedulerNames(self):

master/buildbot/test/util/scheduler.py

@@ -169,8 +169,10 @@ def makeFakeChange(self, **kwargs):
         return ch
 
     def _addBuildsetReturnValue(self, builderNames):
+        # XXX: TODO return builderids
         if builderNames is None:
             builderNames = self.sched.builderNames
+
         bsid = self._bsidGenerator.next()
         brids = dict(zip(builderNames, self._bridGenerator))
         return defer.succeed((bsid, brids))

master/docs/developer/database.rst

@@ -87,6 +87,7 @@ buildrequests
 
     * ``buildrequestid``
     * ``buildsetid``
+    * ``builderid``
     * ``buildername``
     * ``priority``
     * ``claimed`` (boolean, true if the request is claimed)
@@ -551,29 +552,29 @@ buildsets
     * ``complete_at`` (datetime object; time this buildset was completed)
     * ``results`` (aggregate result of this buildset; see :ref:`Build-Result-Codes`)
 
-    .. py:method:: addBuildset(sourcestamps, reason, properties, builderNames, external_idstring=None, parent_buildid=None, parent_relationship=None)
+    .. py:method:: addBuildset(sourcestamps, reason, properties, builderids, external_idstring=None, parent_buildid=None, parent_relationship=None)
 
         :param sourcestamps: sourcestamps for the new buildset; see below
         :type sourcestamps: list
         :param reason: reason for this buildset
         :type reason: short unicode string
         :param properties: properties for this buildset
         :type properties: dictionary, where values are tuples of (value, source)
-        :param builderNames: builders specified by this buildset
-        :type builderNames: list of strings
+        :param builderids: builderids specified by this buildset
+        :type builderids: list of int
         :param external_idstring: external key to identify this buildset; defaults to None
         :type external_idstring: unicode string
         :param datetime submitted_at: time this buildset was created; defaults to the current time
         :param int parent_buildid: optional build id that is the parent for this buildset
         :param unicode parent_relationship: relationship identifier for the parent, this is is configured relationship between the parent build, and the childs buildsets
         :returns: buildset ID and buildrequest IDs, via a Deferred
 
-        Add a new Buildset to the database, along with BuildRequests for each named builder, returning the resulting bsid via a Deferred.
+        Add a new Buildset to the database, along with BuildRequests for each builder, returning the resulting bsid via a Deferred.
         Arguments should be specified by keyword.
 
         Each sourcestamp in the list of sourcestamps can be given either as an integer, assumed to be a sourcestamp ID, or a dictionary of keyword arguments to be passed to :py:meth:`~buildbot.db.sourcestamps.SourceStampsConnectorComponent.findSourceStampId`.
 
-        The return value is a tuple ``(bsid, brids)`` where ``bsid`` is the inserted buildset ID and ``brids`` is a dictionary mapping buildernames to build request IDs.
+        The return value is a tuple ``(bsid, brids)`` where ``bsid`` is the inserted buildset ID and ``brids`` is a dictionary mapping builderids to build request IDs.
 
     .. py:method:: completeBuildset(bsid, results[, complete_at=XX])
 

master/docs/developer/rtype-buildset.rst

@@ -50,14 +50,14 @@ All update methods are available as attributes of ``master.data.updates``.
 
 .. py:class:: buildbot.data.buildsets.BuildsetResourceType
 
-    .. py:method:: addBuildset(scheduler=None, sourcestamps=[], reason='', properties={}, builderNames=[], external_idstring=None, parent_buildid=None, parent_relationship=None)
+    .. py:method:: addBuildset(scheduler=None, sourcestamps=[], reason='', properties={}, builderids=[], external_idstring=None, parent_buildid=None, parent_relationship=None)
 
         :param string scheduler: the name of the scheduler creating this buildset
         :param list sourcestamps: sourcestamps for the new buildset; see below
         :param unicode reason: the reason for this build
         :param properties: properties to set on this buildset
         :type properties: dictionary with unicode keys and (source, property value) values
-        :param list builderNames: names of the builders for which build requests should be created
+        :param list builderids: names of the builderids for which build requests should be created
         :param unicode external_idstring: arbitrary identifier to recognize this buildset later
         :param int parent_buildid: optional build id that is the parent for this buildset
         :param unicode parent_relationship: relationship identifier for the parent, this is is configured relationship between the parent build, and the childs buildsets
@@ -66,7 +66,6 @@ All update methods are available as attributes of ``master.data.updates``.
         .. warning:
 
             The ``scheduler`` parameter will be replaced with a ``schedulerid`` parameter in future releases.
-            The ``builderNames`` parameter will be replaced with a ``builderIds`` parameter in future releases.
 
         Create a new buildset and corresponding buildrequests based on the given parameters.
         This is the low-level interface for scheduling builds.