Merge pull request buildbot#7608 from tdesveaux/typing/db/builddata

typing: convert db.build_data.BuildDataDict to dataclass

master/buildbot/data/build_data.py

@@ -13,26 +13,30 @@
 #
 # 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.build_data import BuildDataModel
 
-class Db2DataMixin:
-    def db2data(self, dbdict):
-        data = {
-            'buildid': dbdict['buildid'],
-            'name': dbdict['name'],
-            'value': dbdict['value'],
-            'length': dbdict['length'],
-            'source': dbdict['source'],
-        }
-        return defer.succeed(data)
+
+def _db2data(model: BuildDataModel):
+    return {
+        'buildid': model.buildid,
+        'name': model.name,
+        'value': model.value,
+        'length': model.length,
+        'source': model.source,
+    }
 
 
-class BuildDatasNoValueEndpoint(Db2DataMixin, base.BuildNestingMixin, base.Endpoint):
+class BuildDatasNoValueEndpoint(base.BuildNestingMixin, base.Endpoint):
     kind = base.EndpointKind.COLLECTION
     pathPatterns = """
         /builders/n:builderid/builds/n:build_number/data
@@ -48,11 +52,11 @@ def get(self, resultSpec, kwargs):
 
         results = []
         for dbdict in build_datadicts:
-            results.append((yield self.db2data(dbdict)))
+            results.append(_db2data(dbdict))
         return results
 
 
-class BuildDataNoValueEndpoint(Db2DataMixin, base.BuildNestingMixin, base.Endpoint):
+class BuildDataNoValueEndpoint(base.BuildNestingMixin, base.Endpoint):
     kind = base.EndpointKind.SINGLE
     pathPatterns = """
         /builders/n:builderid/builds/n:build_number/data/i:name
@@ -67,7 +71,7 @@ def get(self, resultSpec, kwargs):
 
         build_datadict = yield self.master.db.build_data.getBuildDataNoValue(buildid, name)
 
-        return (yield self.db2data(build_datadict)) if build_datadict else None
+        return _db2data(build_datadict) if build_datadict else None
 
 
 class BuildDataEndpoint(base.BuildNestingMixin, base.Endpoint):
@@ -88,9 +92,9 @@ def get(self, resultSpec, kwargs):
             return None
 
         return {
-            'raw': dbdict['value'],
+            'raw': dbdict.value,
             'mime-type': 'application/octet-stream',
-            'filename': dbdict['name'],
+            'filename': dbdict.name,
         }
 
 

master/buildbot/db/build_data.py

@@ -13,14 +13,48 @@
 #
 # Copyright Buildbot Team Members
 
+
+from __future__ import annotations
+
+from dataclasses import dataclass
+
 import sqlalchemy as sa
 from twisted.internet import defer
+from twisted.python import deprecate
+from twisted.python import versions
 
 from buildbot.db import NULL
 from buildbot.db import base
+from buildbot.warnings import warn_deprecated
+
+
+@dataclass
+class BuildDataModel:
+    buildid: int
+    name: str
+    length: int
+    source: str
+    value: bytes | None
+
+    # For backward compatibility
+    def __getitem__(self, key: str):
+        warn_deprecated(
+            '3.12.0',
+            (
+                'BuildDataConnectorComponent getBuildData, getBuildDataNoValue, and getAllBuildDataNoValues '
+                'no longer return BuildData as dictionnaries. '
+                'Usage of [] accessor is deprecated: please access the member directly'
+            ),
+        )
+
+        if hasattr(self, key):
+            return getattr(self, key)
+
+        raise KeyError(key)
 
 
-class BuildDataDict(dict):
+@deprecate.deprecated(versions.Version("buildbot", 3, 12, 0), BuildDataModel)
+class BuildDataDict(BuildDataModel):
     pass
 
 
@@ -73,7 +107,7 @@ def thd(conn):
 
     @defer.inlineCallbacks
     def getBuildData(self, buildid, name):
-        def thd(conn):
+        def thd(conn) -> BuildDataModel | None:
             build_data_table = self.db.model.build_data
 
             q = build_data_table.select().where(
@@ -83,14 +117,14 @@ def thd(conn):
             row = res.fetchone()
             if not row:
                 return None
-            return self._row2dict(conn, row)
+            return self._model_from_row(row, value=row.value)
 
         res = yield self.db.pool.do(thd)
         return res
 
     @defer.inlineCallbacks
     def getBuildDataNoValue(self, buildid, name):
-        def thd(conn):
+        def thd(conn) -> BuildDataModel | None:
             build_data_table = self.db.model.build_data
 
             q = sa.select(
@@ -104,14 +138,14 @@ def thd(conn):
             row = res.fetchone()
             if not row:
                 return None
-            return self._row2dict_novalue(conn, row)
+            return self._model_from_row(row, value=None)
 
         res = yield self.db.pool.do(thd)
         return res
 
     @defer.inlineCallbacks
     def getAllBuildDataNoValues(self, buildid):
-        def thd(conn):
+        def thd(conn) -> list[BuildDataModel]:
             build_data_table = self.db.model.build_data
 
             q = sa.select(
@@ -122,7 +156,7 @@ def thd(conn):
             )
             q = q.where(build_data_table.c.buildid == buildid)
 
-            return [self._row2dict_novalue(conn, row) for row in conn.execute(q).fetchall()]
+            return [self._model_from_row(row, value=None) for row in conn.execute(q).fetchall()]
 
         res = yield self.db.pool.do(thd)
         return res
@@ -166,16 +200,11 @@ def thd(conn):
         res = yield self.db.pool.do(thd)
         return res
 
-    def _row2dict(self, conn, row):
-        return BuildDataDict(
+    def _model_from_row(self, row, value: bytes | None):
+        return BuildDataModel(
             buildid=row.buildid,
             name=row.name,
-            value=row.value,
             length=row.length,
             source=row.source,
-        )
-
-    def _row2dict_novalue(self, conn, row):
-        return BuildDataDict(
-            buildid=row.buildid, name=row.name, value=None, length=row.length, source=row.source
+            value=value,
         )

master/buildbot/test/fakedb/build_data.py

@@ -13,8 +13,11 @@
 #
 # Copyright Buildbot Team Members
 
+from __future__ import annotations
+
 from twisted.internet import defer
 
+from buildbot.db.build_data import BuildDataModel
 from buildbot.test.fakedb.base import FakeDBComponent
 from buildbot.test.fakedb.row import Row
 
@@ -68,26 +71,26 @@ def setBuildData(self, buildid, name, value, source):
         }
 
     # returns a Deferred
-    def getBuildData(self, buildid, name):
+    def getBuildData(self, buildid, name) -> defer.Deferred[BuildDataModel | None]:
         row = self._get_build_data_row(buildid, name)
         if row is not None:
-            return defer.succeed(self._row2dict(row))
+            return defer.succeed(self._model_from_row(row, value=row.get('value')))
         return defer.succeed(None)
 
     # returns a Deferred
-    def getBuildDataNoValue(self, buildid, name):
+    def getBuildDataNoValue(self, buildid, name) -> defer.Deferred[BuildDataModel | None]:
         row = self._get_build_data_row(buildid, name)
         if row is not None:
-            return defer.succeed(self._row2dict_novalue(row))
+            return defer.succeed(self._model_from_row(row, value=None))
         return defer.succeed(None)
 
     # returns a Deferred
-    def getAllBuildDataNoValues(self, buildid):
+    def getAllBuildDataNoValues(self, buildid) -> defer.Deferred[list[BuildDataModel]]:
         ret = []
         for row in self.build_data.values():
             if row['buildid'] != buildid:
                 continue
-            ret.append(self._row2dict_novalue(row))
+            ret.append(self._model_from_row(row, value=None))
 
         return defer.succeed(ret)
 
@@ -115,13 +118,11 @@ def deleteOldBuildData(self, older_than_timestamp):
 
         return defer.succeed(count_before - count_after)
 
-    def _row2dict(self, row):
-        ret = row.copy()
-        del ret['id']
-        return ret
-
-    def _row2dict_novalue(self, row):
-        ret = row.copy()
-        del ret['id']
-        ret['value'] = None
-        return ret
+    def _model_from_row(self, row, value: bytes | None):
+        return BuildDataModel(
+            buildid=row['buildid'],
+            name=row['name'],
+            length=row['length'],
+            source=row['source'],
+            value=value,
+        )

master/buildbot/test/unit/data/test_build_data.py

@@ -18,6 +18,7 @@
 from twisted.trial import unittest
 
 from buildbot.data import build_data
+from buildbot.db.build_data import BuildDataModel
 from buildbot.test import fakedb
 from buildbot.test.fake import fakemaster
 from buildbot.test.reactor import TestReactorMixin
@@ -343,11 +344,11 @@ def test_set_build_data(self):
         result = yield self.master.db.build_data.getBuildData(2, 'name1')
         self.assertEqual(
             result,
-            {
-                'buildid': 2,
-                'name': 'name1',
-                'value': b'value1',
-                'length': 6,
-                'source': 'source1',
-            },
+            BuildDataModel(
+                buildid=2,
+                name='name1',
+                value=b'value1',
+                length=6,
+                source='source1',
+            ),
         )