Opened 13 years ago

Closed 12 years ago

Last modified 11 years ago

#87 closed enhancement (fixed)

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

Reported by: rochg Owned by:
Priority: major Milestone: 0.7.8
Version: 0.7.6 Keywords:
Cc: Pike, dustin, dossy, hollowman, gward, redsymbol, sergey, christian.unger@…

Description

This ticket covers adding build input properties to BuildBot. These properties allow the user to vary the behaviour of a build by specifying a set of properties that can be interpreted by the user's build system or BuildBot python code e.g. derivations of ShellCommand?.

A good example of the use of these properties is running a sub-set of a project's test suite. This could be useful when developing code in one module because it saves you the cost of running all the tests from other modules. The developer could launch a "try" job with an input property specifying the names of the tests to run. This property could then be interpreted a ShellCommand? derived class that exported it to the environment of the build system.

Attachments (11)

custombuildproperties.diff (9.1 KB) - added by rochg 13 years ago.
Original custom build properties patch from Paul Gain
html.py.diff (2.6 KB) - added by rochg 13 years ago.
Resilience fixes to guard against no custom properties being defined
buildset.py.diff (1.4 KB) - added by rochg 13 years ago.
Fix to make BuildSet? work with BuildRequest? now that BuildRequest? supports custom properties
runner.py.diff (1.3 KB) - added by rochg 13 years ago.
Interpret command line custom properties in TryOptions?
tryclient.py.diff (803 bytes) - added by rochg 13 years ago.
Included custom properties when making remote "try" call
scheduler.py.diff (1.2 KB) - added by rochg 13 years ago.
Accept custom properties in Try_Userpass_Perspective
shell.py.diff (2.4 KB) - added by rochg 13 years ago.
Example of the use of custom properties. Not intended for the BuildBot code base.
customprops076.tgz (11.9 KB) - added by rochg 12 years ago.
Partial updates for patches to run against 0.7.6 (no web interface yet)
customprops-fix-debugclient.patch (2.4 KB) - added by gward 12 years ago.
Teach DebugPerspective? and debugclient about custom build properties
customprops_web.diff (2.2 KB) - added by redsymbol 12 years ago.
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
124.patch (127.4 KB) - added by dustin 12 years ago.
all #124 bugs in properties branch

Download all attachments as: .zip

Change History (36)

comment:1 Changed 13 years ago by rochg

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@…> 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

Changed 13 years ago by rochg

Original custom build properties patch from Paul Gain

Changed 13 years ago by rochg

Resilience fixes to guard against no custom properties being defined

Changed 13 years ago by rochg

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

Changed 13 years ago by rochg

Interpret command line custom properties in TryOptions?

Changed 13 years ago by rochg

Included custom properties when making remote "try" call

Changed 13 years ago by rochg

Accept custom properties in Try_Userpass_Perspective

Changed 13 years ago by rochg

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

comment:2 Changed 13 years ago by Pike

  • Cc Pike added

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.)

Changed 12 years ago by rochg

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

comment:3 Changed 12 years ago by gward

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.

Changed 12 years ago by gward

Teach DebugPerspective? and debugclient about custom build properties

Changed 12 years ago by redsymbol

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

comment:4 Changed 12 years ago by dustin

  • Cc dustin added
  • Version changed from 0.7.5 to 0.7.6

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.

comment:5 Changed 12 years ago by dustin

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

comment:6 Changed 12 years ago by Pike

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.

comment:7 Changed 12 years ago by dossy

  • Cc dossy added

+1.

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

comment:8 Changed 12 years ago by dustin

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!).

comment:9 Changed 12 years ago by dustin

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)

comment:10 Changed 12 years ago by dustin

  • Cc hollowman gward added

comment:11 Changed 12 years ago by dustin

  • Cc redsymbol added

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)

comment:12 Changed 12 years ago by dustin

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.

comment:13 Changed 12 years ago by dustin

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

comment:14 Changed 12 years ago by dustin

  • Type changed from defect to enhancement

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?

comment:15 follow-up: Changed 12 years ago by Pike

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.

Changed 12 years ago by dustin

all #124 bugs in properties branch

comment:16 in reply to: ↑ 15 Changed 12 years ago by dustin

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.

comment:17 Changed 12 years ago by dustin

in the devel tree as #124:.*

comment:18 Changed 12 years ago by Pike

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.

comment:19 Changed 12 years ago by dustin

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.

comment:20 Changed 12 years ago by sergey

  • Cc sergey added

comment:21 Changed 12 years ago by dustin

  • Milestone changed from undecided to 0.7.8
  • Resolution set to fixed
  • Status changed from new to closed

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.

comment:22 Changed 11 years ago by cunger

what's up with this bug why is it closed? do we actually have custom build properties now?

comment:23 Changed 11 years ago by cunger

  • Cc christian.unger@… added

comment:24 Changed 11 years ago by dustin

Yes, we do. You asked the same question on the mailing list. Is there something in particular you'd like to know, or is this just a general "please explain life" kind of question? Have you looked in the manual?

comment:25 Changed 11 years ago by cunger

hey dustin, sorry I just wasn't aware. will look up in the manual. and please don't try to explain my life :-)

Note: See TracTickets for help on using tickets.