From 8ba19979db030a0727cbc9ae4c5cee10e252d9da Mon Sep 17 00:00:00 2001 From: Thomas Desveaux Date: Wed, 22 May 2024 08:38:02 +0200 Subject: [PATCH 1/3] db: Add ProjectModel dataclass --- master/buildbot/db/projects.py | 55 ++++++-- master/buildbot/test/__init__.py | 6 + master/buildbot/test/fakedb/projects.py | 18 ++- .../buildbot/test/unit/data/test_projects.py | 17 +-- master/buildbot/test/unit/db/test_projects.py | 121 +++++++++--------- master/buildbot/test/util/validation.py | 10 -- master/docs/spelling_wordlist.txt | 1 + .../db-introduce-ProjectModel.change | 1 + 8 files changed, 133 insertions(+), 96 deletions(-) create mode 100644 newsfragments/db-introduce-ProjectModel.change diff --git a/master/buildbot/db/projects.py b/master/buildbot/db/projects.py index 74b697113f3..19ef2f32ff1 100644 --- a/master/buildbot/db/projects.py +++ b/master/buildbot/db/projects.py @@ -13,10 +13,41 @@ # # Copyright Buildbot Team Members +from __future__ import annotations + +from dataclasses import dataclass from twisted.internet import defer from buildbot.db import base +from buildbot.warnings import warn_deprecated + + +@dataclass +class ProjectModel: + id: int + name: str + slug: str + description: str | None + description_format: str | None + description_html: str | None + + # For backward compatibility + def __getitem__(self, key: str): + warn_deprecated( + '4.1.0', + ( + 'ProjectsConnectorComponent ' + 'get_project, get_projects, and get_active_projects ' + 'no longer return Project as dictionnaries. ' + 'Usage of [] accessor is deprecated: please access the member directly' + ), + ) + + if hasattr(self, key): + return getattr(self, key) + + raise KeyError(key) class ProjectsConnectorComponent(base.DBConnectorComponent): @@ -44,7 +75,7 @@ def thd(conn): rv = None if row: - rv = self._project_dict_from_row(row) + rv = self._model_from_row(row) res.close() return rv @@ -57,7 +88,7 @@ def thd(conn): q = tbl.select() q = q.order_by(tbl.c.name) res = conn.execute(q) - return [self._project_dict_from_row(row) for row in res.fetchall()] + return [self._model_from_row(row) for row in res.fetchall()] return self.db.pool.do(thd) @@ -70,7 +101,7 @@ def thd(conn): q = projects_tbl.select().join(builders_tbl).join(bm_tbl).order_by(projects_tbl.c.name) res = conn.execute(q) - return [self._project_dict_from_row(row) for row in res.fetchall()] + return [self._model_from_row(row) for row in res.fetchall()] return self.db.pool.do(thd) @@ -91,12 +122,12 @@ def thd(conn): return self.db.pool.do(thd) - def _project_dict_from_row(self, row): - return { - "id": row.id, - "name": row.name, - "slug": row.slug, - "description": row.description, - "description_format": row.description_format, - "description_html": row.description_html, - } + def _model_from_row(self, row): + return ProjectModel( + id=row.id, + name=row.name, + slug=row.slug, + description=row.description, + description_format=row.description_format, + description_html=row.description_html, + ) diff --git a/master/buildbot/test/__init__.py b/master/buildbot/test/__init__.py index d29f192a72c..fad93f2bd5d 100644 --- a/master/buildbot/test/__init__.py +++ b/master/buildbot/test/__init__.py @@ -204,3 +204,9 @@ r"to deadlocks in the child\.", category=DeprecationWarning, ) + +warnings.filterwarnings( + "ignore", + r".*ProjectsConnectorComponent get_project, get_projects, and get_active_projects no longer return Project as dictionnaries.*", + category=DeprecatedApiWarning, +) diff --git a/master/buildbot/test/fakedb/projects.py b/master/buildbot/test/fakedb/projects.py index de3ceef8625..20cc3305b69 100644 --- a/master/buildbot/test/fakedb/projects.py +++ b/master/buildbot/test/fakedb/projects.py @@ -16,6 +16,7 @@ from twisted.internet import defer +from buildbot.db.projects import ProjectModel from buildbot.test.fakedb.base import FakeDBComponent from buildbot.test.fakedb.row import Row @@ -85,13 +86,13 @@ def find_project_id(self, name, auto_create=True): def get_project(self, projectid): if projectid in self.projects: - return defer.succeed(self._row2dict(self.projects[projectid])) + return defer.succeed(self._model_from_row(self.projects[projectid])) return defer.succeed(None) def get_projects(self): rv = [] for project in self.projects.values(): - rv.append(self._row2dict(project)) + rv.append(self._model_from_row(project)) return rv def get_active_projects(self): @@ -110,7 +111,7 @@ def get_active_projects(self): for id, project in self.projects.items(): if id not in active_projectids: continue - rv.append(self._row2dict(project)) + rv.append(self._model_from_row(project)) return rv def update_project_info( @@ -125,5 +126,12 @@ def update_project_info( project["description_html"] = description_html return defer.succeed(None) - def _row2dict(self, row): - return row.copy() + def _model_from_row(self, row): + return ProjectModel( + id=row['id'], + name=row['name'], + slug=row['slug'], + description=row['description'], + description_format=row['description_format'], + description_html=row['description_html'], + ) diff --git a/master/buildbot/test/unit/data/test_projects.py b/master/buildbot/test/unit/data/test_projects.py index 8084637828f..2caaa593dfa 100644 --- a/master/buildbot/test/unit/data/test_projects.py +++ b/master/buildbot/test/unit/data/test_projects.py @@ -21,6 +21,7 @@ from buildbot.data import projects from buildbot.data import resultspec +from buildbot.db.projects import ProjectModel from buildbot.test import fakedb from buildbot.test.fake import fakemaster from buildbot.test.reactor import TestReactorMixin @@ -155,13 +156,13 @@ def test_update_project_info(self): self.assertEqual( projects, [ - { - "id": 13, - "name": "fake_project", - "slug": "slug13", - "description": "project13 desc", - "description_format": "format", - "description_html": "html desc", - } + ProjectModel( + id=13, + name="fake_project", + slug="slug13", + description="project13 desc", + description_format="format", + description_html="html desc", + ) ], ) diff --git a/master/buildbot/test/unit/db/test_projects.py b/master/buildbot/test/unit/db/test_projects.py index 33585216fd3..c0189f8a3cf 100644 --- a/master/buildbot/test/unit/db/test_projects.py +++ b/master/buildbot/test/unit/db/test_projects.py @@ -20,7 +20,6 @@ from buildbot.test import fakedb from buildbot.test.util import connector_component from buildbot.test.util import interfaces -from buildbot.test.util import validation def project_key(builder): @@ -65,17 +64,17 @@ def test_update_project_info(self): 7, "slug7", "project7 desc", "format", "html desc" ) dbdict = yield self.db.projects.get_project(7) - validation.verifyDbDict(self, 'projectdict', dbdict) + self.assertIsInstance(dbdict, projects.ProjectModel) self.assertEqual( dbdict, - { - "id": 7, - "name": "fake_project7", - "slug": "slug7", - "description": "project7 desc", - "description_format": "format", - "description_html": "html desc", - }, + projects.ProjectModel( + id=7, + name="fake_project7", + slug="slug7", + description="project7 desc", + description_format="format", + description_html="html desc", + ), ) @defer.inlineCallbacks @@ -84,14 +83,14 @@ def test_find_project_id_new(self): dbdict = yield self.db.projects.get_project(id) self.assertEqual( dbdict, - { - "id": id, - "name": "fake_project", - "slug": "fake_project", - "description": None, - "description_format": None, - "description_html": None, - }, + projects.ProjectModel( + id=id, + name="fake_project", + slug="fake_project", + description=None, + description_format=None, + description_html=None, + ), ) @defer.inlineCallbacks @@ -113,17 +112,17 @@ def test_get_project(self): fakedb.Project(id=7, name='fake_project'), ]) dbdict = yield self.db.projects.get_project(7) - validation.verifyDbDict(self, 'projectdict', dbdict) + self.assertIsInstance(dbdict, projects.ProjectModel) self.assertEqual( dbdict, - { - "id": 7, - "name": "fake_project", - "slug": "fake_project", - "description": None, - "description_format": None, - "description_html": None, - }, + projects.ProjectModel( + id=7, + name="fake_project", + slug="fake_project", + description=None, + description_format=None, + description_html=None, + ), ) @defer.inlineCallbacks @@ -140,35 +139,35 @@ def test_get_projects(self): ]) dblist = yield self.db.projects.get_projects() for dbdict in dblist: - validation.verifyDbDict(self, 'projectdict', dbdict) + self.assertIsInstance(dbdict, projects.ProjectModel) self.assertEqual( sorted(dblist, key=project_key), sorted( [ - { - "id": 7, - "name": "fake_project7", - "slug": "fake_project7", - "description": None, - "description_format": None, - "description_html": None, - }, - { - "id": 8, - "name": "fake_project8", - "slug": "fake_project8", - "description": None, - "description_format": None, - "description_html": None, - }, - { - "id": 9, - "name": "fake_project9", - "slug": "fake_project9", - "description": None, - "description_format": None, - "description_html": None, - }, + projects.ProjectModel( + id=7, + name="fake_project7", + slug="fake_project7", + description=None, + description_format=None, + description_html=None, + ), + projects.ProjectModel( + id=8, + name="fake_project8", + slug="fake_project8", + description=None, + description_format=None, + description_html=None, + ), + projects.ProjectModel( + id=9, + name="fake_project9", + slug="fake_project9", + description=None, + description_format=None, + description_html=None, + ), ], key=project_key, ), @@ -192,18 +191,18 @@ def test_get_active_projects(self): ]) dblist = yield self.db.projects.get_active_projects() for dbdict in dblist: - validation.verifyDbDict(self, 'projectdict', dbdict) + self.assertIsInstance(dbdict, projects.ProjectModel) self.assertEqual( dblist, [ - { - "id": 2, - "name": "fake_project2", - "slug": "fake_project2", - "description": None, - "description_format": None, - "description_html": None, - } + projects.ProjectModel( + id=2, + name="fake_project2", + slug="fake_project2", + description=None, + description_format=None, + description_html=None, + ) ], ) diff --git a/master/buildbot/test/util/validation.py b/master/buildbot/test/util/validation.py index 42115a408d3..1ebd715e4ed 100644 --- a/master/buildbot/test/util/validation.py +++ b/master/buildbot/test/util/validation.py @@ -342,16 +342,6 @@ def validate(self, name, arg_object): message['sourcestamps'] = Selector() message['sourcestamps'].add(None, DictValidator(**_sourcestamp)) -# project -dbdict['projectdict'] = DictValidator( - id=IntValidator(), - name=StringValidator(), - slug=StringValidator(), - description=NoneOk(StringValidator()), - description_format=NoneOk(StringValidator()), - description_html=NoneOk(StringValidator()), -) - # builder message['builders'] = Selector() diff --git a/master/docs/spelling_wordlist.txt b/master/docs/spelling_wordlist.txt index 9ca3a66cc15..01ab4d9e317 100644 --- a/master/docs/spelling_wordlist.txt +++ b/master/docs/spelling_wordlist.txt @@ -662,6 +662,7 @@ prev PrivateTemporaryDirectory procmail prois +ProjectModel propertiesDict propKey proxying diff --git a/newsfragments/db-introduce-ProjectModel.change b/newsfragments/db-introduce-ProjectModel.change new file mode 100644 index 00000000000..ac3388eb3a4 --- /dev/null +++ b/newsfragments/db-introduce-ProjectModel.change @@ -0,0 +1 @@ +:class:`ProjectsConnectorComponent` `get_project`, `get_projects`, and `get_active_projects` now return a :class`ProjectModel` instead of a dictionary of Project data \ No newline at end of file From 281cb092610423289198c6d417fef7b5d9840c30 Mon Sep 17 00:00:00 2001 From: Thomas Desveaux Date: Wed, 22 May 2024 08:46:50 +0200 Subject: [PATCH 2/3] Remove usage of ProjectModel [] accessor --- master/buildbot/data/projects.py | 24 ++++++++++++------- master/buildbot/test/__init__.py | 6 ----- master/buildbot/test/unit/db/test_projects.py | 2 +- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/master/buildbot/data/projects.py b/master/buildbot/data/projects.py index 2158f089bdf..371f38c6599 100644 --- a/master/buildbot/data/projects.py +++ b/master/buildbot/data/projects.py @@ -13,21 +13,27 @@ # # 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.projects import ProjectModel + -def project_db_to_data(dbdict, active=None): +def project_db_to_data(model: ProjectModel, active=None): return { - "projectid": dbdict["id"], - "name": dbdict["name"], - "slug": dbdict["slug"], - "description": dbdict["description"], - "description_format": dbdict["description_format"], - "description_html": dbdict["description_html"], + "projectid": model.id, + "name": model.name, + "slug": model.slug, + "description": model.description, + "description_format": model.description_format, + "description_html": model.description_html, "active": active, } @@ -69,8 +75,8 @@ def get(self, result_spec, kwargs): # This is not optimized case which is assumed to be infrequently required dbdicts_all = yield self.master.db.projects.get_projects() dbdicts_active = yield self.master.db.projects.get_active_projects() - ids_active = set(dbdict["id"] for dbdict in dbdicts_active) - dbdicts = [dbdict for dbdict in dbdicts_all if dbdict["id"] not in ids_active] + ids_active = set(dbdict.id for dbdict in dbdicts_active) + dbdicts = [dbdict for dbdict in dbdicts_all if dbdict.id not in ids_active] return [project_db_to_data(dbdict, active=active) for dbdict in dbdicts] diff --git a/master/buildbot/test/__init__.py b/master/buildbot/test/__init__.py index fad93f2bd5d..d29f192a72c 100644 --- a/master/buildbot/test/__init__.py +++ b/master/buildbot/test/__init__.py @@ -204,9 +204,3 @@ r"to deadlocks in the child\.", category=DeprecationWarning, ) - -warnings.filterwarnings( - "ignore", - r".*ProjectsConnectorComponent get_project, get_projects, and get_active_projects no longer return Project as dictionnaries.*", - category=DeprecatedApiWarning, -) diff --git a/master/buildbot/test/unit/db/test_projects.py b/master/buildbot/test/unit/db/test_projects.py index c0189f8a3cf..27eec9daec4 100644 --- a/master/buildbot/test/unit/db/test_projects.py +++ b/master/buildbot/test/unit/db/test_projects.py @@ -23,7 +23,7 @@ def project_key(builder): - return builder['id'] + return builder.id class Tests(interfaces.InterfaceTests): From 460eda74804b47b1755cdf809ec78802e4a32aa2 Mon Sep 17 00:00:00 2001 From: Thomas Desveaux Date: Wed, 22 May 2024 08:53:13 +0200 Subject: [PATCH 3/3] db: Add type hints to ProjectsConnectorComponent --- master/buildbot/db/projects.py | 30 +++++++++++++------------ master/buildbot/test/fakedb/projects.py | 19 ++++++++++------ 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/master/buildbot/db/projects.py b/master/buildbot/db/projects.py index 19ef2f32ff1..182718ca870 100644 --- a/master/buildbot/db/projects.py +++ b/master/buildbot/db/projects.py @@ -51,7 +51,7 @@ def __getitem__(self, key: str): class ProjectsConnectorComponent(base.DBConnectorComponent): - def find_project_id(self, name, auto_create=True): + def find_project_id(self, name: str, auto_create: bool = True) -> defer.Deferred[int | None]: name_hash = self.hashColumns(name) return self.findSomethingId( tbl=self.db.model.projects, @@ -64,9 +64,8 @@ def find_project_id(self, name, auto_create=True): autoCreate=auto_create, ) - @defer.inlineCallbacks - def get_project(self, projectid): - def thd(conn): + def get_project(self, projectid: int) -> defer.Deferred[ProjectModel | None]: + def thd(conn) -> ProjectModel | None: q = self.db.model.projects.select().where( self.db.model.projects.c.id == projectid, ) @@ -79,11 +78,10 @@ def thd(conn): res.close() return rv - return (yield self.db.pool.do(thd)) + return self.db.pool.do(thd) - # returns a Deferred that returns a value - def get_projects(self): - def thd(conn): + def get_projects(self) -> defer.Deferred[list[ProjectModel]]: + def thd(conn) -> list[ProjectModel]: tbl = self.db.model.projects q = tbl.select() q = q.order_by(tbl.c.name) @@ -92,9 +90,8 @@ def thd(conn): return self.db.pool.do(thd) - # returns a Deferred that returns a value - def get_active_projects(self): - def thd(conn): + def get_active_projects(self) -> defer.Deferred[list[ProjectModel]]: + def thd(conn) -> list[ProjectModel]: projects_tbl = self.db.model.projects builders_tbl = self.db.model.builders bm_tbl = self.db.model.builder_masters @@ -107,9 +104,14 @@ def thd(conn): # returns a Deferred that returns a value def update_project_info( - self, projectid, slug, description, description_format, description_html - ): - def thd(conn): + self, + projectid: int, + slug: str, + description: str | None, + description_format: str | None, + description_html: str | None, + ) -> defer.Deferred[None]: + def thd(conn) -> None: q = self.db.model.projects.update().where(self.db.model.projects.c.id == projectid) conn.execute( q.values( diff --git a/master/buildbot/test/fakedb/projects.py b/master/buildbot/test/fakedb/projects.py index 20cc3305b69..fd9b4929171 100644 --- a/master/buildbot/test/fakedb/projects.py +++ b/master/buildbot/test/fakedb/projects.py @@ -13,6 +13,7 @@ # # Copyright Buildbot Team Members +from __future__ import annotations from twisted.internet import defer @@ -66,8 +67,7 @@ def insert_test_data(self, rows): "description_html": row.description_html, } - # Returns Deferred that yields a number - def find_project_id(self, name, auto_create=True): + def find_project_id(self, name: str, auto_create: bool = True) -> defer.Deferred[int | None]: for m in self.projects.values(): if m['name'] == name: return defer.succeed(m['id']) @@ -84,18 +84,18 @@ def find_project_id(self, name, auto_create=True): } return defer.succeed(id) - def get_project(self, projectid): + def get_project(self, projectid: int) -> defer.Deferred[ProjectModel | None]: if projectid in self.projects: return defer.succeed(self._model_from_row(self.projects[projectid])) return defer.succeed(None) - def get_projects(self): + def get_projects(self) -> list[ProjectModel]: rv = [] for project in self.projects.values(): rv.append(self._model_from_row(project)) return rv - def get_active_projects(self): + def get_active_projects(self) -> list[ProjectModel]: rv = [] active_builderids = { @@ -115,8 +115,13 @@ def get_active_projects(self): return rv def update_project_info( - self, projectid, slug, description, description_format, description_html - ): + self, + projectid: int, + slug: str, + description: str | None, + description_format: str | None, + description_html: str | None, + ) -> defer.Deferred[None]: if projectid not in self.projects: return defer.succeed(None) project = self.projects[projectid]