Addition of build input properties to BuildBot (a.k.a. custom build properties)

Here is a post to buildbot-devel that lead to this ticket being created. It also describes the various files attached to the ticket...

Roch Gadsdon <rochg@bakbone.co.uk> writes:

Hello list, I have a few thoughts / questions on custom build properties.

Hi there.. sorry to be so slow in responding to this.

Thanks for replying Brian - really appreciate it. Sorry to take so long to get back - it's definitely not disinterest.

I have opened a trac ticket with both Paul's original patch and my further patches for the try functionality. I know the code I wrote is not yet sufficiently robust, but it would be interesting to see if this looks like a valid approach or there are better ways. I am still new to Python and BuildBot? so please just say if it doesn't look right.

Here's a summary of the current code:

Paul's original patch (custombuildproperties.diff):

- Added a dictionary called custom_props to BuildRequest?. The properties in this dictionary are added to the existing "build properties" with self.setProperty - Added code to read a description of available build properties from the master config file. These descriptions are stored in the BuildMaster? class - Added code to the force-build page to retrieve the property descriptions, generate form fields for the user to fill out, and include the properties in the BuildRequest? once the build is started

This worked for the forced builds when custom properties were defined, but not for other types of build so I have added:

- Code in html.py to guard against no custom properties being defined (html.py.diff) - Code to BuildSet? to optionally accept a custom_props dictionary at creation and pass it to the BuildRequest? it creates ((buildset.py.diff)

Then to add support for "try" builds I:

- Made TryOptions? interpret a single new option called "customproperties" that accepted a comma separated list of properties e.g. "tests=quick,dependencies=no". The comma separated list gets converted into a dictionary (runner.py.diff) - Modified the Try class so that when it makes the remote "try" call it includes the custom properties dictionary (tryclient.py.diff) - Modified Try_Userpass_Perspective to expect a custom properties dictionary and pass it to the BuildSet? it creates (scheduler.py.diff)

I have also included shell.py.diff on the ticket, which is an example of how we use the the input properties in our BuildBot? setup.

Yeah, I definitely think there's a need for this sort of thing. I'm tempted to think that we might want to distinguish between these things you've added which are inputs to the build process from the existing "build properties" which could be considered as strictly outputs of the build. Maybe the new input things should be called "parameters" or "arguments" or something. Or maybe it's reasonable to use not bother with this input-vs-output distinction and just call them all "properties" and treat them identically.

Thinking of this type of custom property as a read-only input sounds like a nice idea. I'd be happy to change to that. Discouraging/preventing the modification of the inputs to a method/task/whatever is usually a good thing.

Thinking about things now, I guess we would also need to preserve the input properties so that a build with input properties could be exactly repeated with the "Rebuild" button. I'd have to look some more at the code to understand how to do that.

I've wanted this personally for a while: I've been slowing working on a "buildbot force" command that would be like "buildbot try" but without the patch, because when I'm bringing up a new buildslave and discovering what libraries are missing, etc, it's convenient to be able to retrigger builds without making spurious source code changes (or going to the web page's Force Build button, or using 'buildbot debugclient'). And I'd really like to be able to say "buildbot try buildbot.test.test_web" to just do a subset of the tests.

It works out really nicely for us. Running a subset of the tests while you're developing is really handy.

I will place a copy of this message on the trac ticket as well. Would you prefer future discussion to be on this list or on the ticket? I would think the list is better as anyone else can read and contribute, but I'll fit in with whatever you prefer.

Thanks again,

-- Roch

Original custom build properties patch from Paul Gain

Resilience fixes to guard against no custom properties being defined

Fix to make BuildSet? work with BuildRequest? now that BuildRequest? supports custom properties

Interpret command line custom properties in TryOptions?

Included custom properties when making remote "try" call

Accept custom properties in Try_Userpass_Perspective

Example of the use of custom properties. Not intended for the BuildBot? code base.

I'm not sure if the master is the right place to hook up custom build properties. I'd rather expect those to be on the builder.

In what I'm doing right now, I actually have to set properties based on the state of the scheduler, but even just having the buildset part of the patches would get me 50% there. (I do ask the scheduler for the status of the source tree on a particular slave to decide whether to check-out or not.)

Partial updates for patches to run against 0.7.6 (no web interface yet)

This patch breaks the ability to start a build from the debugclient GUI. The GUI sends a 'requestBuild()' call, and then the master crashes with this stack trace:

[2008-01-29 10:04:36] [Broker,0,10.93.3.61] <Build builder1>.startBuild
[2008-01-29 10:04:36] [Broker,0,10.93.3.61] Unhandled error in Deferred:
[2008-01-29 10:04:36] [Broker,0,10.93.3.61] Unhandled Error
        Traceback (most recent call last):
          File "/usr/local/Intelerad/3rd_Party/python-2.5/lib/python2.5/site-packages/twisted/spread/pb.py", line 522, in expressionReceived
            method(*sexp[1:])
          File "/usr/local/Intelerad/3rd_Party/python-2.5/lib/python2.5/site-packages/twisted/spread/pb.py", line 891, in proto_answer
            d.callback(self.unserialize(netResult))
          File "/usr/local/Intelerad/3rd_Party/python-2.5/lib/python2.5/site-packages/twisted/internet/defer.py", line 239, in callback
            self._startRunCallbacks(result)
          File "/usr/local/Intelerad/3rd_Party/python-2.5/lib/python2.5/site-packages/twisted/internet/defer.py", line 304, in _startRunCallbacks
            self._runCallbacks()
        --- <exception caught here> ---
          File "/usr/local/Intelerad/3rd_Party/python-2.5/lib/python2.5/site-packages/twisted/internet/defer.py", line 317, in _runCallbacks
            self.result = callback(self.result, *args, **kw)
          File "/home/gward/lib/python/buildbot/process/builder.py", line 619, in _startBuild_2
            d = build.startBuild(bs, self.expectations, sb)
          File "/home/gward/lib/python/buildbot/process/base.py", line 297, in startBuild
            self.setupStatus(build_status)
          File "/home/gward/lib/python/buildbot/process/base.py", line 277, in setupStatus
            for key,userProp in cp.iteritems():
        exceptions.AttributeError: 'str' object has no attribute 'iteritems'

This all stems from DebugPerspective? and debugclient not knowing anything about custom properties. I'll attach a patch that fixes this shortly.

Teach DebugPerspective? and debugclient about custom build properties

Patch to allow custom properties to be propagated from web force build form (i.e. an HTMLResource), for 0.7.6. Assumes that customprops076.tgz has been applied

These patches look great, but Roch says he's going to do a bit more work on them, so I haven't added them to my darcs repo.

as a note to self, when I pull these changes, I need to pull greg's corresponding patch on #56.

Can we get one single patch for this bug? Or can I get a commandline that explains what to do with the tgz? I took that with me on vacation, just to find out that I can read it, but I didn't find a way for a program to read it.

I'd really like to try this out, and extend it to work with schedulers (which is what I need), but so far, I just failed to get a tree.

+1.

I especially need to be able to pass custom build properties from each scheduler, too.

I just recorded the patch Aaron sent to the list on 3/15 as #85:customprops.patch, with a few changes:

• in constructors, set self.custom_props to {} immediately if it has its default value of None. This just avoids having to apply duck typing everywhere else
• use {}.items instead of {}.iteritems -- I think the latter is newer than the minimum Python version, and the dictionary isn't that large anyway.
• resolved lots of conflicts with the record of the patch in #227 (which adds parameters to the same constructors as this patch does .. oops!)
• fixed a unit test error in the try client (the client was assuming that configcustom_props? existed)

Pike, dossy, hopefully you can now work off the latest development tarball (or just 'darcs get' my repository).

Brian, this changes the call signature for the remote call to "try". Do we need to worry about inter-version compatibility of the tryclient and the buildmaster?

Also, please don't pull this yet, as it still needs

• unit tests
• documentation

which I will try to take care of soon. Others are invited to help (especially with documentation!).

See also

• #87:customprops-fix-debugclient.patch (gward's patch)
• #87:param-tweaks.patch (minor tweaks)

I believe that wraps up all of the attachments to this ticket; customprops_web.diff is included in #87:customprops.patch.

Let's sum up the work left to do:

• custom props from schedulers other than the try and triggerable schedulers (Pike, dossy, can you do this?)
• unit tests (me)
• documentation (me, if necessary)
• compatibility determination, milestone (brian)

also, from the mailing list:

• Persist the custom properties so that builds are exactly repeatable
• Decide if we want to distinguish custom properties from other build properties
• Display custom props in the web interface (Aaron)

I'm working on this; see

http://darcs.r.igoro.us/darcsweb.cgi?r=properties;a=summary

Should be a half-dozen patches or so, then I'll request review.

OK, the branch linked in the previous comment now supports lots of properties:

• cproperties? (global)
• Scheduler(..properties={..}) (properties from schedulers)
• BuildSlave(..properties={..}) (properties from buildslaves)
• arbitrary property input in debugging, try, and triggered buildsets

This is *not* backward-compatible with the custom properties stuff, however:

• The config is different
• Try and debugging remote connections. will no longer match
• The force() parameter useful for subclassing has changed. I hope to implement this myself.

At this point, here's what's left:

• documentation (I haven't touched the docs at all)
• display properties in the web interface
• allow users to specify properties in the web interface

I wrote the documentation and some code to display properties. Could those of you cc'd on this ticket take a look and let me know what you think? Also, somebody please step up and take on the ability to specify build properties from the web interface?

First off, yay. I should be able to use this for the stuff that I need it for. Web-interface might be a plus for me, or not, I have to hack so many new views into it, I guess I don't mind one more.

For a review, I'd like to see one diff for review instead of a tree. It's a little icky to find out what's new and what's old, in particular for someone like me that's not using darcs in any own project.

Stuff I have seen so far, only glimpsing:

I missed comments in properties.py.

I'm not sure if the "bash-like" variable expansion is cool, or over-shooting, or half-missed. I didn't find the reference to the shell expansion stuff helpful, and I consider myself to be doing rather lots in shells.

It's an odd-edge-case backwards compat issue, too:

>>> "%(foo:+bar)s" % {"foo:+bar":"hi"}
'hi'

Don't ask me who'd do that, but it's worth noting.

So much for now, back to fx3rc1.

all #124 bugs in properties branch

Replying to Pike:

For a review, I'd like to see one diff for review instead of a tree. It's a little icky to find out what's new and what's old, in particular for someone like me that's not using darcs in any own project.

Ask and ye shall receive.

I missed comments in properties.py.

Fair enough. I've added a few in #124:properties_py_comments.patch.

I'm not sure if the "bash-like" variable expansion is cool, or over-shooting, or half-missed. I didn't find the reference to the shell expansion stuff helpful, and I consider myself to be doing rather lots in shells.

The documentation certainly doesn't rely on the shell documentation to give the meaning of the expansions. The most useful one is :-, since it allows you to deal with cases where a property is not set, e.g., 'scheduler' (if a build is forced, then it has no scheduler).

It's an odd-edge-case backwards compat issue, too: {{{

"%(foo:+bar)s" % {"foo:+bar":"hi"}

'hi' }}} Don't ask me who'd do that, but it's worth noting.

Worth noting, sure, but given that custom props haven't hit brian's tree yet, and none of the built-in props match this pattern, I doubt it will be a problem.

in the devel tree as #124:.*

Dustin, does "in devel tree" imply an ETA of this landing for a particular release?

We're working on redoing the l10n builds at mozilla, and it'd either be custom properties or some hackery to emulate those. On the other hand, this is a pretty hefty patch to pre-load on our production masters.

No promises as to acceptance in general. This particular patch has been sitting in devel for a while, so I will probably push it to Brian soon. It will be in the next release, barring unforseen problems.

The devel releases also have snapshot tarballs that you could build against, if you'd like to get the latest and greatest patches.

http://darcs.r.igoro.us/buildbot/dustin/dist/

The intent of the devel tree is to be a place where popular patches like this one can come to hang out before hitting brian's tree, whil still being useful to folks who aren't interested in applying long lists of patches.

I will admit that the devel tree would work better in git, where I could continually rebase it onto brian's tree -- as it is, if I want to maintain a patch that's not in brian's tree, I need to add more patches on top of it for every conflict.

OK, we've got quite a CC list now. I put this in my "tobrian" tree to push to the main (release) tree. There seems to be some problem with that communication at the moment, but it will certainly be resolved by 0.7.8.