Merge pull request #7601 from tdesveaux/typing/db/builders

typing: Add BuilderModel dataclass

master/buildbot/data/base.py

@@ -13,17 +13,23 @@
 #
 # Copyright Buildbot Team Members
 
+from __future__ import annotations
+
 import copy
 import enum
 import functools
 import re
 from collections import UserList
+from typing import TYPE_CHECKING
 
 from twisted.internet import defer
 
 from buildbot.data import exceptions
 from buildbot.util.twisted import async_to_deferred
 
+if TYPE_CHECKING:
+    from buildbot.db.builders import BuilderModel
+
 
 class EndpointKind(enum.Enum):
     SINGLE = 1
@@ -184,14 +190,14 @@ class NestedBuildDataRetriever:
         'worker_dict',
     )
 
-    def __init__(self, master, args):
+    def __init__(self, master, args) -> None:
         self.master = master
         self.args = args
         # False is used as special value as "not set". None is used as "not exists". This solves
         # the problem of multiple database queries in case entity does not exist.
         self.step_dict = False
         self.build_dict = False
-        self.builder_dict = False
+        self.builder_dict: BuilderModel | None | False = False
         self.log_dict = False
         self.worker_dict = False
 
@@ -244,7 +250,7 @@ async def get_build_dict(self):
                 return None
 
             self.build_dict = await self.master.db.builds.getBuildByNumber(
-                builderid=builder_dict['id'], number=self.args['build_number']
+                builderid=builder_dict.id, number=self.args['build_number']
             )
             return self.build_dict
 
@@ -268,7 +274,7 @@ async def get_build_id(self):
         return build_dict['id']
 
     @async_to_deferred
-    async def get_builder_dict(self):
+    async def get_builder_dict(self) -> BuilderModel | None:
         if self.builder_dict is not False:
             return self.builder_dict
 
@@ -296,14 +302,14 @@ async def get_builder_dict(self):
         return None
 
     @async_to_deferred
-    async def get_builder_id(self):
+    async def get_builder_id(self) -> int | None:
         if 'builderid' in self.args:
             return self.args['builderid']
 
         builder_dict = await self.get_builder_dict()
         if builder_dict is None:
             return None
-        return builder_dict['id']
+        return builder_dict.id
 
     @async_to_deferred
     async def get_log_dict(self):

master/buildbot/data/builders.py

@@ -13,12 +13,31 @@
 #
 # Copyright Buildbot Team Members
 
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
 
 from twisted.internet import defer
 
 from buildbot.data import base
 from buildbot.data import types
 
+if TYPE_CHECKING:
+    from buildbot.db.builders import BuilderModel
+
+
+def _db2data(builder: BuilderModel):
+    return {
+        "builderid": builder.id,
+        "name": builder.name,
+        "masterids": builder.masterids,
+        "description": builder.description,
+        "description_format": builder.description_format,
+        "description_html": builder.description_html,
+        "projectid": builder.projectid,
+        "tags": builder.tags,
+    }
+
 
 class BuilderEndpoint(base.BuildNestingMixin, base.Endpoint):
     kind = base.EndpointKind.SINGLE
@@ -34,22 +53,13 @@ def get(self, resultSpec, kwargs):
         if builderid is None:
             return None
 
-        bdict = yield self.master.db.builders.getBuilder(builderid)
-        if not bdict:
+        builder = yield self.master.db.builders.getBuilder(builderid)
+        if not builder:
             return None
         if 'masterid' in kwargs:
-            if kwargs['masterid'] not in bdict['masterids']:
+            if kwargs['masterid'] not in builder.masterids:
                 return None
-        return {
-            "builderid": builderid,
-            "name": bdict['name'],
-            "masterids": bdict['masterids'],
-            "description": bdict['description'],
-            "description_format": bdict["description_format"],
-            "description_html": bdict["description_html"],
-            "projectid": bdict['projectid'],
-            "tags": bdict['tags'],
-        }
+        return _db2data(builder)
 
 
 class BuildersEndpoint(base.Endpoint):
@@ -66,19 +76,7 @@ def get(self, resultSpec, kwargs):
         bdicts = yield self.master.db.builders.getBuilders(
             masterid=kwargs.get('masterid', None), projectid=kwargs.get('projectid', None)
         )
-        return [
-            {
-                "builderid": bd['id'],
-                "name": bd['name'],
-                "masterids": bd['masterids'],
-                "description": bd['description'],
-                "description_format": bd['description_format'],
-                "description_html": bd['description_html'],
-                "projectid": bd['projectid'],
-                "tags": bd['tags'],
-            }
-            for bd in bdicts
-        ]
+        return [_db2data(bd) for bd in bdicts]
 
     def get_kwargs_from_graphql(self, parent, resolve_info, args):
         if parent is not None:
@@ -139,17 +137,17 @@ def updateBuilderList(self, masterid, builderNames):
         # figure out what to remove and remove it
         builderNames_set = set(builderNames)
         for bldr in builders:
-            if bldr['name'] not in builderNames_set:
-                builderid = bldr['id']
+            if bldr.name not in builderNames_set:
+                builderid = bldr.id
                 yield self.master.db.builders.removeBuilderMaster(
                     masterid=masterid, builderid=builderid
                 )
                 self.master.mq.produce(
                     ('builders', str(builderid), 'stopped'),
-                    {"builderid": builderid, "masterid": masterid, "name": bldr['name']},
+                    {"builderid": builderid, "masterid": masterid, "name": bldr.name},
                 )
             else:
-                builderNames_set.remove(bldr['name'])
+                builderNames_set.remove(bldr.name)
 
         # now whatever's left in builderNames_set is new
         for name in builderNames_set:

master/buildbot/data/forceschedulers.py

@@ -85,7 +85,7 @@ def get(self, resultSpec, kwargs):
             bdict = yield self.master.db.builders.getBuilder(builderid)
         for sched in self.master.allSchedulers():
             if isinstance(sched, forcesched.ForceScheduler):
-                if builderid is not None and bdict['name'] not in sched.builderNames:
+                if builderid is not None and bdict.name not in sched.builderNames:
                     continue
                 ret.append(forceScheduler2Data(sched))
         return ret

master/buildbot/data/logchunks.py

@@ -48,7 +48,7 @@ def get_info():
         if log_dict['type'] == 's':
             log_prefix = ''
             if builder_dict is not None:
-                log_prefix += f'Builder: {builder_dict["name"]}\n'
+                log_prefix += f'Builder: {builder_dict.name}\n'
             if build_dict is not None:
                 log_prefix += f'Build number: {build_dict["number"]}\n'
             if worker_dict is not None:
@@ -58,7 +58,7 @@ def get_info():
 
         informative_parts = []
         if builder_dict is not None:
-            informative_parts += [builder_dict['name']]
+            informative_parts += [builder_dict.name]
         if build_dict is not None:
             informative_parts += ['build', str(build_dict['number'])]
         if step_dict is not None:

master/buildbot/data/masters.py

@@ -50,7 +50,7 @@ def get(self, resultSpec, kwargs):
         # this builder
         if 'builderid' in kwargs:
             builder = yield self.master.db.builders.getBuilder(builderid=kwargs['builderid'])
-            if not builder or kwargs['masterid'] not in builder['masterids']:
+            if not builder or kwargs['masterid'] not in builder.masterids:
                 return None
         m = yield self.master.db.masters.getMaster(kwargs['masterid'])
         return _db2data(m) if m else None
@@ -70,7 +70,7 @@ def get(self, resultSpec, kwargs):
         if 'builderid' in kwargs:
             builder = yield self.master.db.builders.getBuilder(builderid=kwargs['builderid'])
             if builder:
-                masterids = set(builder['masterids'])
+                masterids = set(builder.masterids)
                 masterlist = [m for m in masterlist if m['id'] in masterids]
             else:
                 masterlist = []

master/buildbot/db/builders.py

@@ -13,13 +13,45 @@
 #
 # Copyright Buildbot Team Members
 
+from __future__ import annotations
 
 from collections import defaultdict
+from dataclasses import dataclass
+from dataclasses import field
 
 import sqlalchemy as sa
 from twisted.internet import defer
 
 from buildbot.db import base
+from buildbot.warnings import warn_deprecated
+
+
+@dataclass
+class BuilderModel:
+    id: int
+    name: str
+    description: str | None = None
+    description_format: str | None = None
+    description_html: str | None = None
+    projectid: int | None = None
+    tags: list[str] = field(default_factory=list)
+    masterids: list[int] = field(default_factory=list)
+
+    # For backward compatibility
+    def __getitem__(self, key: str):
+        warn_deprecated(
+            '3.12.0',
+            (
+                'BuildersConnectorComponent getBuilder and getBuilders '
+                'no longer return Builder as dictionnaries. '
+                'Usage of [] accessor is deprecated: please access the member directly'
+            ),
+        )
+
+        if hasattr(self, key):
+            return getattr(self, key)
+
+        raise KeyError(key)
 
 
 class BuildersConnectorComponent(base.DBConnectorComponent):
@@ -83,16 +115,12 @@ def thd(conn):
 
         return (yield self.db.pool.do(thd))
 
-    def getBuilder(self, builderid):
-        d = self.getBuilders(_builderid=builderid)
-
-        @d.addCallback
-        def first(bldrs):
-            if bldrs:
-                return bldrs[0]
-            return None
-
-        return d
+    @defer.inlineCallbacks
+    def getBuilder(self, builderid: int):
+        bldrs: list[BuilderModel] = yield self.getBuilders(_builderid=builderid)
+        if bldrs:
+            return bldrs[0]
+        return None
 
     # returns a Deferred that returns None
     def addBuilderMaster(self, builderid=None, masterid=None):
@@ -116,8 +144,13 @@ def thd(conn, no_recurse=False):
 
         return self.db.pool.do(thd)
 
-    def getBuilders(self, masterid=None, projectid=None, _builderid=None):
-        def thd(conn):
+    def getBuilders(
+        self,
+        masterid: int | None = None,
+        projectid: int | None = None,
+        _builderid: int | None = None,
+    ) -> defer.Deferred[list[BuilderModel]]:
+        def thd(conn) -> list[BuilderModel]:
             bldr_tbl = self.db.model.builders
             bm_tbl = self.db.model.builder_masters
             builders_tags_tbl = self.db.model.builders_tags
@@ -160,24 +193,22 @@ def thd(conn):
                 bldr_id_to_tags[bldr_id].append(tag)
 
             # now group those by builderid, aggregating by masterid
-            rv = []
-            last = None
+            rv: list[BuilderModel] = []
+            last: BuilderModel | None = None
             for row in conn.execute(q).fetchall():
-                # pylint: disable=unsubscriptable-object
-                if not last or row['id'] != last['id']:
-                    last = {
-                        "id": row.id,
-                        "name": row.name,
-                        "masterids": [],
-                        "description": row.description,
-                        "description_format": row.description_format,
-                        "description_html": row.description_html,
-                        "projectid": row.projectid,
-                        "tags": bldr_id_to_tags[row.id],
-                    }
+                if not last or row['id'] != last.id:
+                    last = BuilderModel(
+                        id=row.id,
+                        name=row.name,
+                        description=row.description,
+                        description_format=row.description_format,
+                        description_html=row.description_html,
+                        projectid=row.projectid,
+                        tags=bldr_id_to_tags[row.id],
+                    )
                     rv.append(last)
                 if row['masterid']:
-                    last['masterids'].append(row['masterid'])
+                    last.masterids.append(row['masterid'])
             return rv
 
         return self.db.pool.do(thd)

master/buildbot/process/buildrequest.py

@@ -217,7 +217,7 @@ def _make_br(cls, brid, brdict, master):
         buildrequest.id = brid
         buildrequest.bsid = brdict['buildsetid']
         builder = yield master.db.builders.getBuilder(brdict['builderid'])
-        buildrequest.buildername = builder['name']
+        buildrequest.buildername = builder.name
         buildrequest.builderid = brdict['builderid']
         buildrequest.priority = brdict['priority']
         dt = brdict['submitted_at']