On Tue, 2010-04-06 at 15:51 +0000, Brad Crittenden wrote: > Review: Needs Information > Hi William, > > This branch looks great. As we discussed on IRC please run it past a Soyuz team member and I'll approve it. Thanks for the review, Brad. > > === modified file 'lib/lp/buildmaster/doc/builder.txt' > > --- lib/lp/buildmaster/doc/builder.txt 2010-03-24 10:24:22 +0000 > > +++ lib/lp/buildmaster/doc/builder.txt 2010-04-03 04:39:18 +0000 > > @@ -2,36 +2,34 @@ > > Builder Class > > ============= > > > > -This test aims to meet the requirements of > > - for the Builder class, > > -which represents the Buildd Slave entity. > > - > > -Need auxiliar methods from zope toolchain: > > +The Builder class represents a slave machine in the build farm. These > > +slaves are used to execute untrusted code -- for example when building > > +packages. > > + > > +There are several builders in the sample data. Let's examine the first. > > + > > + >>> from lp.buildmaster.model.builder import Builder > > + >>> builder = Builder.get(1) > > + > > +As expected, it implements IBuilder. > > Nice changes. > > > +Builders can take on different behaviors depending on the type of build > > +they are currently processing. Each builder provides an attribute > > +(current_build_behavior) to which all the build-type specific behavior > > +for the current build is delegated. In the sample data, bob's current > > +behavior is the behavior for dealing with binary packages > > How about: > "behavior is dealing with binary packages." > (Note full-stop.) > > > >>> from zope.security.proxy import isinstance > > >>> from lp.soyuz.model.binarypackagebuildbehavior import ( > > @@ -40,85 +38,77 @@ > > ... builder.current_build_behavior, BinaryPackageBuildBehavior) > > True > > > > -Confirm we can get the slave xmlrpc interface > > +A builder has an XML-RPC proxy in the 'slave' attribute, which allows > > +us to easily call methods on the slave machines. > > > > >>> s = builder.slave > > > > -Confirm that the urlbase is correct in that slave. (If the protocol changes, > > -this may change too) > > +The base URL of the proxy matches the builder's URL. > > > > >>> s.urlbase == builder.url > > True > > > > -Check if the instance corresponds to the declared interface: > > - > > - >>> verifyObject(IBuilder, builder) > > - True > > - > > > > BuilderSet > > ========== > > > > -Now perform the tests for the Builder ContentSet class, BuilderSet. > > - > > -Check if it can be imported: > > - > > +Builders and groups thereof are managed through a utility, IBuilderSet. > > + > > + >>> from zope.component import getUtility > > >>> from lp.buildmaster.interfaces.builder import IBuilderSet > > - > > -Check we can use the set as a utility: > > - > > >>> builderset = getUtility(IBuilderSet) > > - > > -Check if the instance returned as utility corresponds to its > > -respective interface: > > - > > >>> verifyObject(IBuilderSet, builderset) > > True > > > > -Check if the instance is iterable: > > +Iterating over a BuilderSet yields all registered builders. > > > > >>> for b in builderset: > > - ... b.id > > - 1 > > + ... print b.name > > + bob > > + frog > > + > > +count() return the number of builder instance we have stored: > > typo: instances Fixed. This was in the original text, so I've also reworded it slightly. > > + > > + >>> builderset.count() > > 2 > > > > -Check if the __getitem__ method: > > - > > - >>> builderset['bob'].name > > - u'bob' > > - > > -Check now the specific method in the utility as new(): > > +Builders can be retrieved by name. > > + > > + >>> print builderset['bob'].name > > + bob > > + >>> print builderset['bad'] > > + None > > + > > +And also by ID. > > + > > + >>> print builderset.get(2).name > > + frog > > + >>> print builderset.get(100).name > > + Traceback (most recent call last): > > + ... > > + SQLObjectNotFound: Object not found > > + > > +The 'new' method will create a new builder in the database. > > > > >>> bnew = builderset.new(1, 'http://dummy.com:8221/', 'dummy', > > ... 'Dummy Title', 'eh ?', 1) > > >>> bnew.name > > u'dummy' > > > > -Check get() which returns a correspondent Builder instance to a given id: > > - > > - >>> builderset.get(bnew.id).name > > - u'dummy' > > - > > -Or raises an SQLObjectNotFound exception: > > - > > - >>> builderset.get(100) > > - Traceback (most recent call last): > > - ... > > - SQLObjectNotFound: Object not found > > - > > -count() return the number of builder instance we have stored: > > - > > - >>> builderset.count() > > - 3 > > - > > -getBuilder() method returns all the builders available. It seems the > > -same than the own instance but we have plans to turn it aware of some > > -attributes of builder instance as: builderok and trust. > > - > > - >>> for b in builderset.getBuilders(): > > - ... b.name > > - u'bob' > > - u'dummy' > > +'getBuilders' returns builders with the 'active' flag set, ordered by > > +virtualisation status, architecture, then name. > > We've agreed on en_US (don't hate me) so > /virtualisation/virtualization/ Also (begrudgingly) fixed. > > + > > + >>> for b in builderset.getBuilders(): > > + ... b.name > > Please use 'print b.name' so the unicodiness is hidden. Fixed. > > + u'bob' > > + u'dummy' > > + u'frog' > > + >>> login('