launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:stormify-buildqueue into launchpad:master.

Commit message:
Convert BuildQueue to Storm

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/387246
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:stormify-buildqueue into launchpad:master.

diff --git a/lib/lp/buildmaster/interactor.py b/lib/lp/buildmaster/interactor.py
index 03283d5..d036aad 100644
--- a/lib/lp/buildmaster/interactor.py
+++ b/lib/lp/buildmaster/interactor.py
@@ -633,7 +633,7 @@ class BuilderInteractor(object):
         """
         builder_status = slave_status["builder_status"]
         if builder_status == "BuilderStatus.ABORTING":
-            logtail = "Waiting for slave process to be terminated"
+            logtail = u"Waiting for slave process to be terminated"
         elif slave_status.get("logtail") is not None:
             # slave_status["logtail"] is normally an xmlrpc_client.Binary
             # instance, and the contents might include invalid UTF-8 due to
@@ -644,7 +644,7 @@ class BuilderInteractor(object):
             # PostgreSQL text columns can't contain \0 characters, and since
             # we only use this for web UI display purposes there's no point
             # in going through contortions to store them.
-            logtail = logtail.replace("\0", "")
+            logtail = logtail.replace(u"\0", u"")
         else:
             logtail = None
         return logtail
diff --git a/lib/lp/buildmaster/manager.py b/lib/lp/buildmaster/manager.py
index 8b3a3a3..a4b96e3 100644
--- a/lib/lp/buildmaster/manager.py
+++ b/lib/lp/buildmaster/manager.py
@@ -260,7 +260,7 @@ class PrefetchedBuilderFactory(BaseBuilderFactory):
         """See `BaseBuilderFactory`."""
         transaction.abort()
         builders_and_current_bqs = list(IStore(Builder).using(
-            Builder, LeftJoin(BuildQueue, BuildQueue.builderID == Builder.id)
+            Builder, LeftJoin(BuildQueue, BuildQueue.builder == Builder.id)
             ).find((Builder, BuildQueue)))
         getUtility(IBuilderSet).preloadProcessors(
             [b for b, _ in builders_and_current_bqs])
diff --git a/lib/lp/buildmaster/model/builder.py b/lib/lp/buildmaster/model/builder.py
index 473b923..93326ee 100644
--- a/lib/lp/buildmaster/model/builder.py
+++ b/lib/lp/buildmaster/model/builder.py
@@ -286,7 +286,7 @@ class BuilderSet(object):
             Sum(BuildQueue.estimated_duration),
             Processor,
             Coalesce(BuildQueue.virtualized, True)),
-            Processor.id == BuildQueue.processorID,
+            Processor.id == BuildQueue.processor_id,
             BuildQueue.status == BuildQueueStatus.WAITING).group_by(
                 Processor, Coalesce(BuildQueue.virtualized, True))
 
diff --git a/lib/lp/buildmaster/model/buildqueue.py b/lib/lp/buildmaster/model/buildqueue.py
index ed5f2cb..ded1382 100644
--- a/lib/lp/buildmaster/model/buildqueue.py
+++ b/lib/lp/buildmaster/model/buildqueue.py
@@ -16,13 +16,6 @@ from operator import attrgetter
 
 import pytz
 import six
-from sqlobject import (
-    BoolCol,
-    ForeignKey,
-    IntCol,
-    IntervalCol,
-    StringCol,
-    )
 from storm.expr import (
     And,
     Desc,
@@ -31,8 +24,11 @@ from storm.expr import (
     SQL,
     )
 from storm.properties import (
+    Bool,
     DateTime,
     Int,
+    TimeDelta,
+    Unicode,
     )
 from storm.references import Reference
 from storm.store import Store
@@ -43,6 +39,7 @@ from zope.component import (
 from zope.interface import implementer
 from zope.security.proxy import removeSecurityProxy
 
+from lp.app.errors import NotFoundError
 from lp.buildmaster.enums import (
     BuildFarmJobType,
     BuildQueueStatus,
@@ -61,9 +58,9 @@ from lp.services.database.constants import (
     DEFAULT,
     UTC_NOW,
     )
-from lp.services.database.enumcol import EnumCol
+from lp.services.database.enumcol import DBEnum
 from lp.services.database.interfaces import IStore
-from lp.services.database.sqlbase import SQLBase
+from lp.services.database.stormbase import StormBase
 from lp.services.features import getFeatureFlag
 from lp.services.propertycache import (
     cachedproperty,
@@ -92,31 +89,37 @@ def specific_build_farm_job_sources():
 
 
 @implementer(IBuildQueue)
-class BuildQueue(SQLBase):
-    _table = "BuildQueue"
-    _defaultOrder = "id"
+class BuildQueue(StormBase):
+    __storm_table__ = "BuildQueue"
+    __storm_order__ = "id"
 
     def __init__(self, build_farm_job, estimated_duration=DEFAULT,
                  virtualized=DEFAULT, processor=DEFAULT, lastscore=None):
-        super(BuildQueue, self).__init__(
-            _build_farm_job=build_farm_job, virtualized=virtualized,
-            processor=processor, estimated_duration=estimated_duration,
-            lastscore=lastscore)
+        super(BuildQueue, self).__init__()
+        self._build_farm_job = build_farm_job
+        self.estimated_duration = estimated_duration
+        self.virtualized = virtualized
+        self.processor = processor
+        self.lastscore = lastscore
         if lastscore is None and self.specific_build is not None:
             self.score()
 
+    id = Int(primary=True)
+
     _build_farm_job_id = Int(name='build_farm_job')
     _build_farm_job = Reference(_build_farm_job_id, 'BuildFarmJob.id')
-    status = EnumCol(enum=BuildQueueStatus, default=BuildQueueStatus.WAITING)
+    status = DBEnum(enum=BuildQueueStatus, default=BuildQueueStatus.WAITING)
     date_started = DateTime(tzinfo=pytz.UTC)
 
-    builder = ForeignKey(dbName='builder', foreignKey='Builder', default=None)
-    logtail = StringCol(dbName='logtail', default=None)
-    lastscore = IntCol(dbName='lastscore', default=0)
-    manual = BoolCol(dbName='manual', default=False)
-    estimated_duration = IntervalCol()
-    processor = ForeignKey(dbName='processor', foreignKey='Processor')
-    virtualized = BoolCol(dbName='virtualized')
+    builder_id = Int(name='builder', default=None)
+    builder = Reference(builder_id, 'Builder.id')
+    logtail = Unicode(name='logtail', default=None)
+    lastscore = Int(name='lastscore', default=0)
+    manual = Bool(name='manual', default=False)
+    estimated_duration = TimeDelta()
+    processor_id = Int(name='processor')
+    processor = Reference(processor_id, 'Processor.id')
+    virtualized = Bool(name='virtualized')
 
     @cachedproperty
     def specific_build(self):
@@ -251,17 +254,20 @@ class BuildQueueSet(object):
 
     def get(self, buildqueue_id):
         """See `IBuildQueueSet`."""
-        return BuildQueue.get(buildqueue_id)
+        bq = IStore(BuildQueue).get(BuildQueue, buildqueue_id)
+        if bq is None:
+            raise NotFoundError(buildqueue_id)
+        return bq
 
     def getByBuilder(self, builder):
         """See `IBuildQueueSet`."""
-        return BuildQueue.selectOneBy(builder=builder)
+        return IStore(BuildQueue).find(BuildQueue, builder=builder).one()
 
     def preloadForBuilders(self, builders):
         # Populate builders' currentjob cachedproperty.
-        queues = load_referencing(BuildQueue, builders, ['builderID'])
+        queues = load_referencing(BuildQueue, builders, ['builder_id'])
         queue_builders = dict(
-            (queue.builderID, queue) for queue in queues)
+            (queue.builder_id, queue) for queue in queues)
         for builder in builders:
             cache = get_property_cache(builder)
             cache.currentjob = queue_builders.get(builder.id, None)
@@ -274,7 +280,7 @@ class BuildQueueSet(object):
             BuildQueue,
             BuildQueue._build_farm_job_id.is_in(
                 [removeSecurityProxy(b).build_farm_job_id for b in builds])))
-        load_related(Builder, bqs, ['builderID'])
+        load_related(Builder, bqs, ['builder_id'])
         prefetched_data = dict(
             (removeSecurityProxy(buildqueue)._build_farm_job_id, buildqueue)
             for buildqueue in bqs)
diff --git a/lib/lp/buildmaster/queuedepth.py b/lib/lp/buildmaster/queuedepth.py
index 66f1986..c49a3ff 100644
--- a/lib/lp/buildmaster/queuedepth.py
+++ b/lib/lp/buildmaster/queuedepth.py
@@ -181,7 +181,7 @@ def get_pending_jobs_clauses(bq):
             (BuildQueue.lastscore = %s AND BuildQueue.id < %s))
         AND buildqueue.virtualized = %s
         """ % sqlvalues(
-            BuildQueueStatus.WAITING, bq.lastscore, bq.lastscore, bq,
+            BuildQueueStatus.WAITING, bq.lastscore, bq.lastscore, bq.id,
             bq.virtualized)
     processor_clause = """
         AND (
diff --git a/lib/lp/buildmaster/tests/test_buildqueue.py b/lib/lp/buildmaster/tests/test_buildqueue.py
index d50e5a0..08ceb55 100644
--- a/lib/lp/buildmaster/tests/test_buildqueue.py
+++ b/lib/lp/buildmaster/tests/test_buildqueue.py
@@ -6,14 +6,15 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 from datetime import timedelta
 
-from storm.sqlobject import SQLObjectNotFound
+from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
+from lp.app.errors import NotFoundError
 from lp.buildmaster.enums import (
     BuildQueueStatus,
     BuildStatus,
     )
-from lp.buildmaster.model.buildqueue import BuildQueue
+from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
 from lp.services.database.interfaces import IStore
 from lp.soyuz.enums import (
     ArchivePurpose,
@@ -115,7 +116,7 @@ class TestBuildCancellation(TestCaseWithFactory):
 
     def assertCancelled(self, build, bq):
         self.assertEqual(BuildStatus.CANCELLED, build.status)
-        self.assertRaises(SQLObjectNotFound, BuildQueue.get, bq.id)
+        self.assertRaises(NotFoundError, getUtility(IBuildQueueSet).get, bq.id)
 
     def test_binarypackagebuild_cancel(self):
         build = self.factory.makeBinaryPackageBuild()