[Buildbot-devel] Making BuildRequests merging configurable, rfc

[Jean-Paul Calderone]

On Thu, 22 Jan 2009 19:17:28 +0100, Axel Hecht <l10n.moz at googlemail.com> wrote:
>Hi,
>
>I'd like to get some comments on http://buildbot.net/trac/ticket/415 and
>http://github.com/Pike/buildbot/tree/bug415.
>
>Basically, I'm adding options to master.cfg on when to merge build requests,
>plus the ability to give a completely independent implementation. From the
>html-ified docs...
>
>4.6 Merging BuildRequests
>
>By default, buildbot merges BuildRequests that have the compatible
>SourceStamps. Starting with 0.7.10, this behaviour can be customized to, for
>example, only merge when the reasons match as well, or if the build
>properties don't differ.
>
>    Note: This feature is experimental, please provide feedback in the
>mailing list on this. Future versions of buildbot might change the way this
>functionality is exposed, too.
>
>The default behaviour to merge BuildRequests with matching SourceStamps can
>be overwritten by setting
>
>     c['mergeMatchingRequests'] = False
>
>Be cautious when doing that, as most VC steps will exhibit unexpected
>behaviour when given Changes with mismatching branches.
>
>To customize the behaviour such that only BuildRequests with the same reason
>get merged, set
>
>     c['mergeMatchingReasons'] = True
>
>Another criteria on which merging can be controlled are build properties, to
>switch those on set
>
>     c['mergeMatchingProperties'] = True
>
>These settings can be combined to only merge BuildRequests with matching
>SourceStamps and matching build properties. All settings are taking effect
>on all builders. If you need a higher level of customization, you can
>specify your own class instance,
>
>     c['loadMaster'] = MyLoadMaster()
>
>....
>
>I'd welcome feedback on design, configuration, and docs here or in the
>ticket.

What about just adding a single new configuration key, rather than four?
This could be done by specifying the four behaviors you documented above
like this instead:

   c['loadMaster'] = MergeMatching(requests=False)
   c['loadMaster'] = MergeMatching(reasons=True)
   c['loadMaster'] = MergeMatching(properties=True)
   c['loadMaster'] = MyLoadMaster()

where `MergeMatching` (feel free to think of a less sucky name) is a
class provided by buildbot.  The default behavior would be provided by
the configuration dict starting with a 'loadMaster' key mapped to
MergeMatching(requests=True) or something like that.  This should simplify
the configuration handling code as well, since there's no longer any
special cases - just grab the 'loadMaster' key and use it.

Also, I'm not sure where the key name "loadMaster" came from.  I can't
figure out how it relates to this feature.  I would have thought something
with "merge" in it would make sense.  Hopefully this doesn't mean I've
totally misunderstood what you're talking about. ;)

Jean-Paul