Merge lp:~julian-edwards/launchpad/api-expose-builders into lp:launchpad

[Julian Edwards]

= Summary =
Expose IBuilder in the webservice

== Implementation details ==
The usual trivial-ish interface changes. /builders is a top-level collection.

== Tests ==
bin/test -cvvt xx-builders.txt

[Leonard Richardson (leonardr)]

This looks good. Just some nitpicky stuff:

On the web service, getByName is a named operation (or "operation"), not a method. Calling it a method makes it look like a local method call.

'Our "bob" builder was retrieved using the no-priv user, and he does not..' The "he" is ambiguous. s/he/no-priv/.

Do you want to iterate over a *sorted* list of attribute names? bob.lp_attributes is ultimately dependent on the order of attributes in the IBuilder definition. Something to think about.

You might want to distinguish between cprov's launchpad and no-priv's in the variable names. 'launchpad' and 'cprov_launchpad' or 'privileged_launchpad'.

[Leonard Richardson (leonardr)]

review: Approve

=== modified file 'lib/lp/buildmaster/configure.zcml'
--- lib/lp/buildmaster/configure.zcml	2010-07-26 15:52:22 +0000
+++ lib/lp/buildmaster/configure.zcml	2010-11-19 16:16:49 +0000
@@ -6,6 +6,7 @@
     xmlns="http://namespaces.zope.org/zope"
     xmlns:browser="http://namespaces.zope.org/browser"
     xmlns:i18n="http://namespaces.zope.org/i18n"
+    xmlns:webservice="http://namespaces.canonical.com/webservice"
     xmlns:xmlrpc="http://namespaces.zope.org/xmlrpc"
     i18n_domain="launchpad">
     <include package=".browser"/>
@@ -114,5 +115,6 @@
             interface="lp.buildmaster.interfaces.buildqueue.IBuildQueueSet"/>
     </securedutility>
 
+    <webservice:register module="lp.buildmaster.interfaces.webservice" />
 
 </configure>

=== modified file 'lib/lp/buildmaster/interfaces/builder.py'
--- lib/lp/buildmaster/interfaces/builder.py	2010-10-27 14:25:19 +0000
+++ lib/lp/buildmaster/interfaces/builder.py	2010-11-19 16:16:49 +0000
@@ -19,6 +19,16 @@
     'ProtocolVersionMismatch',
     ]
 
+from lazr.restful.declarations import (
+    collection_default_content,
+    export_as_webservice_collection,
+    export_as_webservice_entry,
+    export_read_operation,
+    exported,
+    operation_parameters,
+    operation_returns_entry,
+    )
+
 from zope.interface import (
     Attribute,
     Interface,
@@ -38,6 +48,7 @@
 from lp.registry.interfaces.role import IHasOwner
 from lp.services.fields import (
     Description,
+    PersonChoice,
     Title,
     )
 
@@ -92,71 +103,74 @@
     machine status representation, including the field/properties:
     virtualized, builderok, status, failnotes and currentjob.
     """
+    export_as_webservice_entry()
+
     id = Attribute("Builder identifier")
+
     processor = Choice(
         title=_('Processor'), required=True, vocabulary='Processor',
         description=_('Build Slave Processor, used to identify '
                       'which jobs can be built by this device.'))
 
-    owner = Choice(
+    owner = exported(PersonChoice(
         title=_('Owner'), required=True, vocabulary='ValidOwner',
         description=_('Builder owner, a Launchpad member which '
-                      'will be responsible for this device.'))
+                      'will be responsible for this device.')))
 
-    url = TextLine(
+    url = exported(TextLine(
         title=_('URL'), required=True, constraint=builder_url_validator,
         description=_('The URL to the build machine, used as a unique '
                       'identifier. Includes protocol, host and port only, '
-                      'e.g.: http://farm.com:8221/'))
+                      'e.g.: http://farm.com:8221/')))
 
-    name = TextLine(
+    name = exported(TextLine(
         title=_('Name'), required=True, constraint=name_validator,
-        description=_('Builder Slave Name used for reference proposes'))
+        description=_('Builder Slave Name used for reference proposes')))
 
-    title = Title(
+    title = exported(Title(
         title=_('Title'), required=True,
-        description=_('The builder slave title. Should be just a few words.'))
+        description=_('The builder slave title. Should be just a few words.')))
 
-    description = Description(
+    description = exported(Description(
         title=_('Description'), required=False,
         description=_('The builder slave description, may be several '
                       'paragraphs of text, giving the highlights and '
-                      'details.'))
+                      'details.')))
 
-    virtualized = Bool(
+    virtualized = exported(Bool(
         title=_('Virtualized'), required=True, default=False,
         description=_('Whether or not the builder is a virtual Xen '
-                      'instance.'))
+                      'instance.')))
 
-    manual = Bool(
+    manual = exported(Bool(
         title=_('Manual Mode'), required=False, default=False,
         description=_('The auto-build system does not dispatch '
-                      'jobs automatically for slaves in manual mode.'))
+                      'jobs automatically for slaves in manual mode.')))
 
-    builderok = Bool(
+    builderok = exported(Bool(
         title=_('Builder State OK'), required=True, default=True,
-        description=_('Whether or not the builder is ok'))
+        description=_('Whether or not the builder is ok')))
 
-    failnotes = Text(
+    failnotes = exported(Text(
         title=_('Failure Notes'), required=False,
-        description=_('The reason for a builder not being ok'))
+        description=_('The reason for a builder not being ok')))
 
-    vm_host = TextLine(
+    vm_host = exported(TextLine(
         title=_('Virtual Machine Host'), required=False,
         description=_('The machine hostname hosting the virtual '
-                      'buildd-slave, e.g.: foobar-host.ppa'))
+                      'buildd-slave, e.g.: foobar-host.ppa')))
 
-    active = Bool(
+    active = exported(Bool(
         title=_('Publicly Visible'), required=True, default=True,
-        description=_('Whether or not to present the builder publicly.'))
+        description=_('Whether or not to present the builder publicly.')))
 
     slave = Attribute("xmlrpclib.Server instance corresponding to builder.")
 
     currentjob = Attribute("BuildQueue instance for job being processed.")
 
-    failure_count = Int(
+    failure_count = exported(Int(
         title=_('Failure Count'), required=False, default=0,
-        description=_("Number of consecutive failures for this builder."))
+       description=_("Number of consecutive failures for this builder.")))
 
     current_build_behavior = Field(
         title=u"The current behavior of the builder for the current job.",
@@ -333,6 +347,7 @@
     Methods on this interface should deal with the set of Builders:
     methods that affect a single Builder should be on IBuilder.
     """
+    export_as_webservice_collection(IBuilder)
 
     title = Attribute('Title')
 
@@ -342,6 +357,13 @@
     def __getitem__(name):
         """Retrieve a builder by name"""
 
+    @operation_parameters(
+        name=TextLine(title=_("Builder name"), required=True))
+    @operation_returns_entry(IBuilder)
+    @export_read_operation()
+    def getByName(name):
+        """Retrieve a builder by name"""
+
     def new(processor, url, name, title, description, owner,
             active=True, virtualized=False, vm_host=None):
         """Create a new Builder entry.
@@ -360,6 +382,7 @@
     def get(builder_id):
         """Return the IBuilder with the given builderid."""
 
+    @collection_default_content()
     def getBuilders():
         """Return all active configured builders."""
 

=== added file 'lib/lp/buildmaster/interfaces/webservice.py'
--- lib/lp/buildmaster/interfaces/webservice.py	1970-01-01 00:00:00 +0000
+++ lib/lp/buildmaster/interfaces/webservice.py	2010-11-19 16:16:49 +0000
@@ -0,0 +1,20 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""All the interfaces that are exposed through the webservice.
+
+There is a declaration in ZCML somewhere that looks like:
+  <webservice:register module="lp.soyuz.interfaces.webservice" />
+
+which tells `lazr.restful` that it should look for webservice exports here.
+"""
+
+__all__ = [
+    'IBuilder',
+    'IBuilderSet',
+    ]
+
+from lp.buildmaster.interfaces.builder import (
+    IBuilder,
+    IBuilderSet,
+    )

=== modified file 'lib/lp/buildmaster/model/builder.py'
--- lib/lp/buildmaster/model/builder.py	2010-11-17 16:22:45 +0000
+++ lib/lp/buildmaster/model/builder.py	2010-11-19 16:16:49 +0000
@@ -832,12 +832,16 @@
     def __iter__(self):
         return iter(Builder.select())
 
-    def __getitem__(self, name):
+    def getByName(self, name):
+        """See IBuilderSet."""
         try:
             return Builder.selectOneBy(name=name)
         except SQLObjectNotFound:
             raise NotFoundError(name)
 
+    def __getitem__(self, name):
+        return self.getByName(name)
+
     def new(self, processor, url, name, title, description, owner,
             active=True, virtualized=False, vm_host=None, manual=True):
         """See IBuilderSet."""

=== added file 'lib/lp/soyuz/stories/webservice/xx-builders.txt'
--- lib/lp/soyuz/stories/webservice/xx-builders.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/stories/webservice/xx-builders.txt	2010-11-19 16:16:49 +0000
@@ -0,0 +1,68 @@
+========
+Builders
+========
+
+The webservice exposes a top-level collection called "builders" which
+contains all the registered builders in the Launchpad build farm.
+
+    >>> nopriv_launchpad = launchpadlib_for(
+    ...     'builders test', 'no-priv', version='devel')
+    >>> builders = nopriv_launchpad.builders
+
+Iterating over the collection is possible:
+
+    >>> for builder in builders:
+    ...     print builder
+    http://api.launchpad.dev/devel/builders/bob
+    http://api.launchpad.dev/devel/builders/frog
+
+An individual builder can be retrieved by name by using the getByName()
+operation on "builders":
+
+    >>> bob = builders.getByName(name="bob")
+
+Each builder has a number of properties exposed:
+
+    >>> for attribute in bob.lp_attributes:
+    ...     print attribute
+    self_link
+    ...
+    active
+    builderok
+    description
+    failnotes
+    failure_count
+    manual
+    name
+    title
+    url
+    virtualized
+    vm_host
+
+
+Changing builder properties
+===========================
+
+If an authorized person (usually a member of the buildd-admins team)
+wishes to amend some property of a builder, the usual webservice 'lp_save'
+operation is used.
+
+Our "bob" builder was retrieved using the no-priv user, and no-priv does not
+have permission to save changes:
+
+    >>> print bob.active
+    True
+    >>> bob.active = False
+    >>> bob.lp_save()
+    Traceback (most recent call last):
+    ...
+    Unauthorized:...
+
+'cprov', who is a buildd-admin, is able to change the data:
+
+    >>> cprov_launchpad = launchpadlib_for(
+    ...     'builders test', 'cprov', version='devel')
+    >>> bob = cprov_launchpad.builders.getByName(name="bob")
+    >>> bob.active = False
+    >>> bob.lp_save()
+