launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/improve-livefs-scoring into lp:launchpad.

Commit message:
Allow setting a relative build score on live filesystems.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1452543 in Launchpad itself: "livefs builds default to low scores, with no way to adjust defaults"
  https://bugs.launchpad.net/launchpad/+bug/1452543

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/improve-livefs-scoring/+merge/327639
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/improve-livefs-scoring into lp:launchpad.

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py	2017-06-16 10:02:47 +0000
+++ lib/lp/security.py	2017-07-18 16:38:02 +0000
@@ -3095,6 +3095,11 @@
             user.in_commercial_admin or user.in_admin)
 
 
+class ModerateLiveFS(ModerateArchive):
+    """Restrict changing the build score on live filesystems."""
+    usedfor = ILiveFS
+
+
 class AdminLiveFS(AuthorizationBase):
     """Restrict changing build settings on live filesystems.
 

=== modified file 'lib/lp/soyuz/browser/livefs.py'
--- lib/lp/soyuz/browser/livefs.py	2015-05-07 10:52:10 +0000
+++ lib/lp/soyuz/browser/livefs.py	2017-07-18 16:38:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """LiveFS views."""
@@ -181,6 +181,7 @@
         'owner',
         'name',
         'require_virtualized',
+        'relative_build_score',
         ])
     distro_series = Choice(
         vocabulary='BuildableDistroSeries', title=u'Distribution series')
@@ -282,11 +283,14 @@
 
     label = title
 
-    field_names = ['require_virtualized']
+    field_names = ['require_virtualized', 'relative_build_score']
 
     @property
     def initial_values(self):
-        return {'require_virtualized': self.context.require_virtualized}
+        return {
+            'require_virtualized': self.context.require_virtualized,
+            'relative_build_score': self.context.relative_build_score,
+            }
 
 
 class LiveFSEditView(LiveFSMetadataValidatorMixin, BaseLiveFSEditView):

=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml	2016-11-12 22:25:52 +0000
+++ lib/lp/soyuz/configure.zcml	2017-07-18 16:38:02 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -951,12 +951,16 @@
             permission="launchpad.View"
             interface=".interfaces.livefs.ILiveFSView
                        .interfaces.livefs.ILiveFSEditableAttributes
+                       .interfaces.livefs.ILiveFSModerateAttributes
                        .interfaces.livefs.ILiveFSAdminAttributes"/>
         <require
             permission="launchpad.Edit"
             interface=".interfaces.livefs.ILiveFSEdit"
             set_schema=".interfaces.livefs.ILiveFSEditableAttributes"/>
         <require
+            permission="launchpad.Moderate"
+            set_schema=".interfaces.livefs.ILiveFSModerateAttributes"/>
+        <require
             permission="launchpad.Admin"
             set_schema=".interfaces.livefs.ILiveFSAdminAttributes"/>
     </class>

=== modified file 'lib/lp/soyuz/interfaces/livefs.py'
--- lib/lp/soyuz/interfaces/livefs.py	2015-09-15 20:16:47 +0000
+++ lib/lp/soyuz/interfaces/livefs.py	2017-07-18 16:38:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Live filesystem interfaces."""
@@ -10,6 +10,7 @@
     'DuplicateLiveFSName',
     'ILiveFS',
     'ILiveFSEditableAttributes',
+    'ILiveFSEditableAttributes',
     'ILiveFSSet',
     'ILiveFSView',
     'LIVEFS_FEATURE_FLAG',
@@ -249,6 +250,19 @@
         key_type=TextLine(), required=True, readonly=False))
 
 
+class ILiveFSModerateAttributes(Interface):
+    """Restricted `ILiveFS` attributes.
+
+    These attributes need launchpad.View to see, and launchpad.Moderate to
+    change.
+    """
+    relative_build_score = exported(Int(
+        title=_("Relative build score"), required=True, readonly=False,
+        description=_(
+            "A delta to apply to all build scores for the live filesystem.  "
+            "Builds with a higher score will build sooner.")))
+
+
 class ILiveFSAdminAttributes(Interface):
     """`ILiveFS` attributes that can be edited by admins.
 
@@ -262,7 +276,7 @@
 
 class ILiveFS(
     ILiveFSView, ILiveFSEdit, ILiveFSEditableAttributes,
-    ILiveFSAdminAttributes):
+    ILiveFSModerateAttributes, ILiveFSAdminAttributes):
     """A buildable live filesystem image."""
 
     # XXX cjwatson 2014-05-06 bug=760849: "beta" is a lie to get WADL

=== modified file 'lib/lp/soyuz/model/livefs.py'
--- lib/lp/soyuz/model/livefs.py	2015-11-26 15:46:38 +0000
+++ lib/lp/soyuz/model/livefs.py	2017-07-18 16:38:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -111,6 +111,8 @@
 
     require_virtualized = Bool(name='require_virtualized')
 
+    relative_build_score = Int(name='relative_build_score', allow_none=False)
+
     def __init__(self, registrant, owner, distro_series, name,
                  metadata, require_virtualized, date_created):
         """Construct a `LiveFS`."""
@@ -123,6 +125,7 @@
         self.name = name
         self.metadata = metadata
         self.require_virtualized = require_virtualized
+        self.relative_build_score = 0
         self.date_created = date_created
         self.date_last_modified = date_created
 

=== modified file 'lib/lp/soyuz/model/livefsbuild.py'
--- lib/lp/soyuz/model/livefsbuild.py	2016-08-12 12:56:41 +0000
+++ lib/lp/soyuz/model/livefsbuild.py	2017-07-18 16:38:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -250,7 +250,9 @@
         self.buildqueue_record.cancel()
 
     def calculateScore(self):
-        return 2510 + self.archive.relative_build_score
+        return (
+            2510 + self.archive.relative_build_score +
+            self.livefs.relative_build_score)
 
     def getMedianBuildDuration(self):
         """Return the median duration of our successful builds."""

=== modified file 'lib/lp/soyuz/tests/test_livefs.py'
--- lib/lp/soyuz/tests/test_livefs.py	2016-08-12 12:56:41 +0000
+++ lib/lp/soyuz/tests/test_livefs.py	2017-07-18 16:38:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test live filesystems."""
@@ -17,6 +17,7 @@
 import transaction
 from zope.component import getUtility
 from zope.event import notify
+from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
 
 from lp.buildmaster.enums import (
@@ -44,6 +45,7 @@
 from lp.testing import (
     ANONYMOUS,
     api_url,
+    celebrity_logged_in,
     login,
     logout,
     person_logged_in,
@@ -109,6 +111,16 @@
         self.assertSqlAttributeEqualsDate(
             livefs, "date_last_modified", UTC_NOW)
 
+    def test_relative_build_score(self):
+        # Buildd admins can change the relative build score of a LiveFS, but
+        # ordinary users cannot.
+        livefs = self.factory.makeLiveFS()
+        with person_logged_in(livefs.owner):
+            self.assertRaises(
+                Unauthorized, setattr, livefs, "relative_build_score", 100)
+        with celebrity_logged_in("buildd_admin"):
+            livefs.relative_build_score = 100
+
     def test_requestBuild(self):
         # requestBuild creates a new LiveFSBuild.
         livefs = self.factory.makeLiveFS()
@@ -150,8 +162,9 @@
         self.assertEqual(2510, queue_record.lastscore)
 
     def test_requestBuild_relative_build_score(self):
-        # Offsets for archives are respected.
+        # Offsets for archives and livefses are respected.
         livefs = self.factory.makeLiveFS()
+        removeSecurityProxy(livefs).relative_build_score = 50
         archive = self.factory.makeArchive(owner=livefs.owner)
         removeSecurityProxy(archive).relative_build_score = 100
         distroarchseries = self.factory.makeDistroArchSeries(
@@ -161,7 +174,7 @@
             PackagePublishingPocket.RELEASE)
         queue_record = build.buildqueue_record
         queue_record.score()
-        self.assertEqual(2610, queue_record.lastscore)
+        self.assertEqual(2660, queue_record.lastscore)
 
     def test_requestBuild_rejects_repeats(self):
         # requestBuild refuses if there is already a pending build.