db: Add BuildRequestsModel dataclass

common/code_spelling_ignore_words.txt

@@ -204,6 +204,7 @@ buildrequestid
 buildrequestresults
 buildrequests
 buildrequestsconnectorcomponent
+buildrequestmodel
 buildresult
 buildroot
 build's

master/buildbot/data/buildrequests.py

@@ -29,22 +29,23 @@
     from typing import Sequence
 
     from buildbot.data.resultspec import ResultSpec
+    from buildbot.db.buildrequests import BuildRequestModel
 
 
-def _db2data(dbdict: dict, properties: dict | None):
+def _db2data(dbmodel: BuildRequestModel, properties: dict | None):
     return {
-        'buildrequestid': dbdict['buildrequestid'],
-        'buildsetid': dbdict['buildsetid'],
-        'builderid': dbdict['builderid'],
-        'priority': dbdict['priority'],
-        'claimed': dbdict['claimed'],
-        'claimed_at': dbdict['claimed_at'],
-        'claimed_by_masterid': dbdict['claimed_by_masterid'],
-        'complete': dbdict['complete'],
-        'results': dbdict['results'],
-        'submitted_at': dbdict['submitted_at'],
-        'complete_at': dbdict['complete_at'],
-        'waited_for': dbdict['waited_for'],
+        'buildrequestid': dbmodel.buildrequestid,
+        'buildsetid': dbmodel.buildsetid,
+        'builderid': dbmodel.builderid,
+        'priority': dbmodel.priority,
+        'claimed': dbmodel.claimed,
+        'claimed_at': dbmodel.claimed_at,
+        'claimed_by_masterid': dbmodel.claimed_by_masterid,
+        'complete': dbmodel.complete,
+        'results': dbmodel.results,
+        'submitted_at': dbmodel.submitted_at,
+        'complete_at': dbmodel.complete_at,
+        'waited_for': dbmodel.waited_for,
         'properties': properties,
     }
 
@@ -104,9 +105,7 @@ def get(self, resultSpec: ResultSpec, kwargs):
             return None
 
         filters = resultSpec.popProperties() if hasattr(resultSpec, 'popProperties') else []
-        properties = yield self.get_buildset_properties_filtered(
-            buildrequest['buildsetid'], filters
-        )
+        properties = yield self.get_buildset_properties_filtered(buildrequest.buildsetid, filters)
         return _db2data(buildrequest, properties)
 
     @defer.inlineCallbacks
@@ -163,7 +162,7 @@ def get(self, resultSpec, kwargs):
         results = []
         filters = resultSpec.popProperties() if hasattr(resultSpec, 'popProperties') else []
         for br in buildrequests:
-            properties = yield self.get_buildset_properties_filtered(br['buildsetid'], filters)
+            properties = yield self.get_buildset_properties_filtered(br.buildsetid, filters)
             results.append(_db2data(br, properties))
         return results
 
@@ -261,7 +260,7 @@ def completeBuildRequests(self, brids, results, complete_at=None):
             brdict = yield self.master.db.buildrequests.getBuildRequest(brid)
 
             if brdict:
-                bsid = brdict['buildsetid']
+                bsid = brdict.buildsetid
                 if bsid in seen_bsids:
                     continue
                 seen_bsids.add(bsid)

master/buildbot/data/buildsets.py

@@ -226,7 +226,7 @@ def maybeBuildsetComplete(self, bsid):
         # figure out the overall results of the buildset:
         cumulative_results = SUCCESS
         for brdict in brdicts:
-            cumulative_results = worst_status(cumulative_results, brdict['results'])
+            cumulative_results = worst_status(cumulative_results, brdict.results)
 
         # get a copy of the buildset
         bsdict = yield self.master.db.buildsets.getBuildset(bsid)

master/buildbot/data/masters.py

@@ -170,7 +170,7 @@ def _masterDeactivatedHousekeeping(self, masterid, name):
             complete=False, claimed=masterid
         )
         yield self.master.db.buildrequests.unclaimBuildRequests(
-            brids=[br['buildrequestid'] for br in buildrequests]
+            brids=[br.buildrequestid for br in buildrequests]
         )
 
     @defer.inlineCallbacks

master/buildbot/db/buildrequests.py

@@ -13,18 +13,27 @@
 #
 # Copyright Buildbot Team Members
 
+from __future__ import annotations
 
 import itertools
+from dataclasses import dataclass
+from typing import TYPE_CHECKING
 
 import sqlalchemy as sa
 from twisted.internet import defer
+from twisted.python import deprecate
 from twisted.python import log
+from twisted.python import versions
 
 from buildbot.db import NULL
 from buildbot.db import base
 from buildbot.process.results import RETRY
 from buildbot.util import datetime2epoch
 from buildbot.util import epoch2datetime
+from buildbot.warnings import warn_deprecated
+
+if TYPE_CHECKING:
+    import datetime
 
 
 class AlreadyClaimedError(Exception):
@@ -35,7 +44,46 @@ class NotClaimedError(Exception):
     pass
 
 
-class BrDict(dict):
+@dataclass
+class BuildRequestModel:
+    buildrequestid: int
+    buildsetid: int
+    builderid: int
+    buildername: str
+    submitted_at: datetime.datetime
+    complete_at: datetime.datetime | None = None
+    complete: bool = False
+    results: int | None = None
+    waited_for: bool = False
+    priority: int = 0
+
+    claimed_at: datetime.datetime | None = None
+    claimed_by_masterid: int | None = None
+
+    @property
+    def claimed(self) -> bool:
+        return self.claimed_at is not None
+
+    # For backward compatibility from when SsDict inherited from Dict
+    def __getitem__(self, key: str):
+        warn_deprecated(
+            '3.12.0',
+            (
+                'BuildRequestsConnectorComponent '
+                'getBuildRequest, and getBuildRequests '
+                'no longer return BuildRequest as dictionnaries. '
+                'Usage of [] accessor is deprecated: please access the member directly'
+            ),
+        )
+
+        if hasattr(self, key):
+            return getattr(self, key)
+
+        raise KeyError(key)
+
+
+@deprecate.deprecated(versions.Version("buildbot", 3, 12, 0), BuildRequestModel)
+class BrDict(BuildRequestModel):
     pass
 
 
@@ -63,17 +111,16 @@ def _saSelectQuery(self):
             builder_tbl.c.name.label('buildername'),
         ).select_from(from_clause)
 
-    # returns a Deferred that returns a value
-    def getBuildRequest(self, brid):
-        def thd(conn):
+    def getBuildRequest(self, brid) -> defer.Deferred[BuildRequestModel | None]:
+        def thd(conn) -> BuildRequestModel | None:
             reqs_tbl = self.db.model.buildrequests
             q = self._saSelectQuery()
             q = q.where(reqs_tbl.c.id == brid)
             res = conn.execute(q)
             row = res.fetchone()
             rv = None
             if row:
-                rv = self._brdictFromRow(row, self.db.master.masterid)
+                rv = self._modelFromRow(row)
             res.close()
             return rv
 
@@ -90,10 +137,10 @@ def getBuildRequests(
         repository=None,
         resultSpec=None,
     ):
-        def deduplicateBrdict(brdicts):
-            return list(({b['buildrequestid']: b for b in brdicts}).values())
+        def deduplicateBrdict(brdicts: list[BuildRequestModel]) -> list[BuildRequestModel]:
+            return list(({b.buildrequestid: b for b in brdicts}).values())
 
-        def thd(conn):
+        def thd(conn) -> list[BuildRequestModel]:
             reqs_tbl = self.db.model.buildrequests
             claims_tbl = self.db.model.buildrequest_claims
             sstamps_tbl = self.db.model.sourcestamps
@@ -122,17 +169,11 @@ def thd(conn):
                 q = q.where(sstamps_tbl.c.repository == repository)
 
             if resultSpec is not None:
-                return deduplicateBrdict(
-                    resultSpec.thd_execute(
-                        conn, q, lambda r: self._brdictFromRow(r, self.db.master.masterid)
-                    )
-                )
+                return deduplicateBrdict(resultSpec.thd_execute(conn, q, self._modelFromRow))
 
             res = conn.execute(q)
 
-            return deduplicateBrdict([
-                self._brdictFromRow(row, self.db.master.masterid) for row in res.fetchall()
-            ])
+            return deduplicateBrdict([self._modelFromRow(row) for row in res.fetchall()])
 
         res = yield self.db.pool.do(thd)
         return res
@@ -165,8 +206,7 @@ def thd(conn):
 
         yield self.db.pool.do(thd)
 
-    # returns a Deferred that returns None
-    def unclaimBuildRequests(self, brids):
+    def unclaimBuildRequests(self, brids) -> defer.Deferred[None]:
         def thd(conn):
             transaction = conn.begin()
             claims_tbl = self.db.model.buildrequest_claims
@@ -264,31 +304,18 @@ def thd(conn):
         return self.db.pool.do(thd)
 
     @staticmethod
-    def _brdictFromRow(row, master_masterid):
-        claimed = False
-        claimed_by_masterid = None
-        claimed_at = None
-        if row.claimed_at is not None:
-            claimed_at = row.claimed_at
-            claimed = True
-            claimed_by_masterid = row.masterid
-
-        submitted_at = epoch2datetime(row.submitted_at)
-        complete_at = epoch2datetime(row.complete_at)
-        claimed_at = epoch2datetime(claimed_at)
-
-        return BrDict(
+    def _modelFromRow(row):
+        return BuildRequestModel(
             buildrequestid=row.id,
             buildsetid=row.buildsetid,
             builderid=row.builderid,
             buildername=row.buildername,
-            priority=row.priority,
-            claimed=claimed,
-            claimed_at=claimed_at,
-            claimed_by_masterid=claimed_by_masterid,
+            submitted_at=epoch2datetime(row.submitted_at),
+            complete_at=epoch2datetime(row.complete_at),
             complete=bool(row.complete),
             results=row.results,
-            submitted_at=submitted_at,
-            complete_at=complete_at,
             waited_for=bool(row.waited_for),
+            priority=row.priority,
+            claimed_at=epoch2datetime(row.claimed_at),
+            claimed_by_masterid=row.masterid,
         )

master/buildbot/process/buildrequest.py

@@ -13,14 +13,20 @@
 #
 # Copyright Buildbot Team Members
 
+from __future__ import annotations
+
 import calendar
+from typing import TYPE_CHECKING
 
 from twisted.internet import defer
 
 from buildbot.data import resultspec
 from buildbot.process import properties
 from buildbot.process.results import SKIPPED
 
+if TYPE_CHECKING:
+    from buildbot.db.buildrequests import BuildRequestModel
+
 
 class BuildRequestCollapser:
     # brids is a list of the new added buildrequests id
@@ -63,12 +69,14 @@ def collapse(self):
             bldrdict = yield self.master.data.get(('builders', builderid))
             # Get the builder object
             bldr = self.master.botmaster.builders.get(bldrdict['name'])
+            if not bldr:
+                continue
             # Get the Collapse BuildRequest function (from the configuration)
-            collapseRequestsFn = bldr.getCollapseRequestsFn() if bldr else None
+            collapseRequestsFn = bldr.getCollapseRequestsFn()
             unclaim_brs = yield self._getUnclaimedBrs(builderid)
 
             # short circuit if there is no merging to do
-            if not collapseRequestsFn or not unclaim_brs:
+            if not unclaim_brs:
                 continue
 
             for unclaim_br in unclaim_brs:
@@ -193,9 +201,9 @@ class BuildRequest:
     sources = {}
 
     @classmethod
-    def fromBrdict(cls, master, brdict):
+    def fromBrdict(cls, master, brdict: BuildRequestModel):
         """
-        Construct a new L{BuildRequest} from a dictionary as returned by
+        Construct a new L{BuildRequest} from a L{BuildRequestModel} as returned by
         L{BuildRequestsConnectorComponent.getBuildRequest}.
 
         This method uses a cache, which may result in return of stale objects;
@@ -208,30 +216,29 @@ def fromBrdict(cls, master, brdict):
         @returns: L{BuildRequest}, via Deferred
         """
         cache = master.caches.get_cache("BuildRequests", cls._make_br)
-        return cache.get(brdict['buildrequestid'], brdict=brdict, master=master)
+        return cache.get(brdict.buildrequestid, brdict=brdict, master=master)
 
     @classmethod
     @defer.inlineCallbacks
-    def _make_br(cls, brid, brdict, master):
+    def _make_br(cls, brid: int, brdict: BuildRequestModel, master):
         buildrequest = cls()
         buildrequest.id = brid
-        buildrequest.bsid = brdict['buildsetid']
-        builder = yield master.db.builders.getBuilder(brdict['builderid'])
-        buildrequest.buildername = builder.name
-        buildrequest.builderid = brdict['builderid']
-        buildrequest.priority = brdict['priority']
-        dt = brdict['submitted_at']
+        buildrequest.bsid = brdict.buildsetid
+        buildrequest.buildername = brdict.buildername
+        buildrequest.builderid = brdict.builderid
+        buildrequest.priority = brdict.priority
+        dt = brdict.submitted_at
         buildrequest.submittedAt = dt and calendar.timegm(dt.utctimetuple())
         buildrequest.master = master
-        buildrequest.waitedFor = brdict['waited_for']
+        buildrequest.waitedFor = brdict.waited_for
 
         # fetch the buildset to get the reason
-        buildset = yield master.db.buildsets.getBuildset(brdict['buildsetid'])
+        buildset = yield master.db.buildsets.getBuildset(brdict.buildsetid)
         assert buildset  # schema should guarantee this
         buildrequest.reason = buildset['reason']
 
         # fetch the buildset properties, and convert to Properties
-        buildset_properties = yield master.db.buildsets.getBuildsetProperties(brdict['buildsetid'])
+        buildset_properties = yield master.db.buildsets.getBuildsetProperties(brdict.buildsetid)
 
         buildrequest.properties = properties.Properties.fromDict(buildset_properties)