launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/builderset-new-api into lp:launchpad.

Commit message:
Export BuilderSet.new() on the webservice.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/builderset-new-api/+merge/216861

Export BuilderSet.new() on the webservice. All pretty trivial, except for moving BuilderSet.new to a new interface restricted by launchpad.Admin like the view.
-- 
https://code.launchpad.net/~wgrant/launchpad/builderset-new-api/+merge/216861
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/builderset-new-api into lp:launchpad.

=== modified file 'lib/lp/buildmaster/configure.zcml'
--- lib/lp/buildmaster/configure.zcml	2013-11-20 17:10:19 +0000
+++ lib/lp/buildmaster/configure.zcml	2014-04-23 11:07:46 +0000
@@ -29,6 +29,9 @@
         provides="lp.buildmaster.interfaces.builder.IBuilderSet">
         <allow
             interface="lp.buildmaster.interfaces.builder.IBuilderSet"/>
+        <require
+            permission="launchpad.Admin"
+            interface="lp.buildmaster.interfaces.builder.IBuilderSetAdmin"/>
     </securedutility>
     <adapter
         provides="lp.services.webapp.interfaces.IBreadcrumb"

=== modified file 'lib/lp/buildmaster/doc/builder.txt'
--- lib/lp/buildmaster/doc/builder.txt	2013-11-28 12:23:01 +0000
+++ lib/lp/buildmaster/doc/builder.txt	2014-04-23 11:07:46 +0000
@@ -69,8 +69,10 @@
 
 The 'new' method will create a new builder in the database.
 
-    >>> bnew = builderset.new(
-    ...     [1], 'http://dummy.com:8221/', 'dummy', 'Dummy Title', 1)
+    >>> from lp.testing import admin_logged_in
+    >>> with admin_logged_in():
+    ...     bnew = builderset.new(
+    ...         [1], 'http://dummy.com:8221/', 'dummy', 'Dummy Title', 1)
     >>> bnew.name
     u'dummy'
 

=== modified file 'lib/lp/buildmaster/interfaces/builder.py'
--- lib/lp/buildmaster/interfaces/builder.py	2013-11-28 08:51:32 +0000
+++ lib/lp/buildmaster/interfaces/builder.py	2014-04-23 11:07:46 +0000
@@ -17,15 +17,18 @@
     ]
 
 from lazr.restful.declarations import (
+    call_with,
     collection_default_content,
     export_as_webservice_collection,
     export_as_webservice_entry,
+    export_factory_operation,
     export_read_operation,
     exported,
     operation_for_version,
     operation_parameters,
     operation_returns_collection_of,
     operation_returns_entry,
+    REQUEST_USER,
     )
 from lazr.restful.fields import (
     Reference,
@@ -165,7 +168,7 @@
                       'buildd-slave, e.g.: foobar-host.ppa')))
 
     active = exported(Bool(
-        title=_('Publicly Visible'), required=True, default=True,
+        title=_('Publicly Visible'), required=False, default=True,
         description=_('Whether or not to present the builder publicly.')))
 
     currentjob = Attribute("BuildQueue instance for job being processed.")
@@ -210,7 +213,25 @@
         """
 
 
-class IBuilderSet(Interface):
+class IBuilderSetAdmin(Interface):
+
+    @call_with(owner=REQUEST_USER)
+    @export_factory_operation(
+        IBuilder,
+        ['processors', 'url', 'name', 'title', 'active', 'virtualized',
+         'vm_host'])
+    @operation_for_version('devel')
+    def new(processors, url, name, title, owner, active=True,
+            virtualized=False, vm_host=None):
+        """Create a new builder.
+
+        The builder will be set to manual. An admin needs to verify its
+        configuration and set it to automatic before jobs will be
+        dispatched.
+        """
+
+
+class IBuilderSet(IBuilderSetAdmin):
     """Collections of builders.
 
     IBuilderSet provides access to all Builders in the system,
@@ -219,7 +240,6 @@
     methods that affect a single Builder should be on IBuilder.
     """
     export_as_webservice_collection(IBuilder)
-
     title = Attribute('Title')
 
     def __iter__():
@@ -235,18 +255,6 @@
     def getByName(name):
         """Retrieve a builder by name"""
 
-    def new(processors, url, name, title, owner, active=True,
-            virtualized=False, vm_host=None):
-        """Create a new Builder entry.
-
-        Additionally to the given arguments, builder are created with
-        'builderok' and 'manual' set to True.
-
-        It means that, once created, they will be presented as 'functional'
-        in the UI but will not receive any job until an administrator move
-        it to the automatic mode.
-        """
-
     def count():
         """Return the number of builders in the system."""
 

=== modified file 'lib/lp/buildmaster/tests/test_webservice.py'
--- lib/lp/buildmaster/tests/test_webservice.py	2013-11-28 08:51:32 +0000
+++ lib/lp/buildmaster/tests/test_webservice.py	2014-04-23 11:07:46 +0000
@@ -5,13 +5,21 @@
 
 __metaclass__ = type
 
+from zope.component import getUtility
+
+from lp.registry.interfaces.person import IPersonSet
+from lp.services.webapp.interfaces import OAuthPermission
 from lp.testing import (
+    admin_logged_in,
     api_url,
     logout,
     TestCaseWithFactory,
     )
 from lp.testing.layers import DatabaseFunctionalLayer
-from lp.testing.pages import LaunchpadWebServiceCaller
+from lp.testing.pages import (
+    LaunchpadWebServiceCaller,
+    webservice_for_person,
+    )
 
 
 class TestBuildersCollection(TestCaseWithFactory):
@@ -49,6 +57,27 @@
             ['quantum_builder1', 'quantum_builder2'],
             sorted(builder['name'] for builder in results['entries']))
 
+    def test_new(self):
+        person = self.factory.makePerson()
+        badmins = getUtility(IPersonSet).getByName('launchpad-buildd-admins')
+        webservice = webservice_for_person(
+            person, permission=OAuthPermission.WRITE_PRIVATE)
+        args = dict(
+            name='foo', processors=['/+processors/386'], title='foobar',
+            url='http://foo.buildd:8221/', virtualized=False,
+            api_version='devel')
+
+        response = webservice.named_post('/builders', 'new', **args)
+        self.assertEqual(401, response.status)
+
+        with admin_logged_in():
+            badmins.addMember(person, badmins)
+        response = webservice.named_post('/builders', 'new', **args)
+        self.assertEqual(201, response.status)
+
+        self.assertEqual(
+            'foobar', webservice.get('/builders/foo').jsonBody()['title'])
+
 
 class TestBuilderEntry(TestCaseWithFactory):
     layer = DatabaseFunctionalLayer

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2014-02-24 07:19:52 +0000
+++ lib/lp/testing/factory.py	2014-04-23 11:07:46 +0000
@@ -2760,9 +2760,10 @@
         if owner is None:
             owner = self.makePerson()
 
-        return getUtility(IBuilderSet).new(
-            processors, url, name, title, owner, active, virtualized, vm_host,
-            manual=manual)
+        with admin_logged_in():
+            return getUtility(IBuilderSet).new(
+                processors, url, name, title, owner, active,
+                virtualized, vm_host, manual=manual)
 
     def makeRecipeText(self, *branches):
         if len(branches) == 0: