Ticket #9 (new enhancement)

Opened 2 years ago

Last modified 1 year ago

master.cfg should define names rather than keys in a single named dictionary

Reported by: warner Assigned to: warner
Priority: minor Milestone: 0.8.0
Component: configuration Version:
Keywords: Cc:

Description

I think the syntax of the master.cfg file might look cleaner if we were defining well-known names rather than putting everything into a dictionary. In other words, rather than: 

BuildmasterConfig = c = {}
c['sources'] = [stuff]
c['builders'] = [stuff]

we should have: 

Sources = [stuff]
Builders = [stuff]

This makes it easier to have collisions between temporary names (that you're using as intermediate values) and names that the Buildmaster pays attention to, but we can establish a convention that all of the Buildmaster's names will be capitalized, which will avoid much of the problem.

This would also make a simple master.cfg look less like python, which might help Buildbot feel less python-specific, which I think would be a good thing. Complex master.cfgs can continue to use all the power of a full programming language, of course.

Change History

01/29/07 02:58:04 changed by warner

  • component changed from buildprocess to configuration.

07/29/07 16:01:36 changed by warner

on the other hand, I'm concerned about the namespace-collisions between the helper classes and intermediate variables and the names that actually affect the configuration. For example, in the current code we have: 

from buildbot.slave import BuildSlave
c['slaves'] = [BuildSlave(one), BuildSlave(two)]

but with the proposal we have here, that would be: 

from buildbot.slave import BuildSlave
Slaves = [BuildSlave(one), BuildSlave(two)

and that makes it hard to tell which of the names are magic-control-the-config and which are not. The usual python convention is that classnames are initial-caps or camel-case, whereas regular variables are lowercase. A buildbot-specific convention that config-variables are camel-case would collide with classnames. We could go with a common prefix convention instead: 

from buildbot.slave import BuildSlave
c_slaves = [BuildSlave(one), BuildSlave(two)]

but I'm not sure that's a huge win over c['slaves']=

07/31/07 11:32:23 changed by warner

  • milestone set to 0.8.0.

Here's another zany idea: pre-populate the namespace with all the classes that might be useful. This means everything in http://buildbot.net/repos/release/docs/buildbot.html#Index-of-Useful-Classes except the ones that incur extra dependencies (like twisted.words or CVSToys). And maybe even include those condtionally (by catching ImportError).

And then we could have a plugin-registration mechanism like TracPlugins, which could then automatically add the marked classes into that namespace.

I'm thinking that we'd make 'buildbot help-config' emit a list of all the names (and a brief description) that would get added to the namespace this way, and 'buildbot help-config BuildSlave' be a way to get more info (and it should really provide a URL to the section of the user's manual for even more explanation).

We'd still probably need a convention to distinguish classnames from confignames, but it would at least get rid of all the import statements in the config file.

Or would it be confusing to see these classnames used in a sample config file and not have a clear import statement beforehand, explaining where they come from?

That would

08/09/07 03:38:48 changed by warner

ooh, and another crazy idea: for the keys that are defined as lists, pre-populate the namespace with append functions, like add_scheduler and add_builder. That would make the config file look something like: 

add_slave(BuildSlave('linux', 'password'))
f1 = BuildFactory()
f1.addStep(SVN("svn://repourlstuff"))
f1.addStep(Compile(["make", "all"]))
add_builder(Builder("full", "linux", f1))
add_changesource(SVNPoller("svn://repourlstuff"))
add_scheduler(Scheduler("trunk", 60, ["full"]))
add_status(WebStatus(8010))
add_status(IRC("irc.freenode.net", "#twisted", "buildbot"))
project_name = "Twisted"
project_url = "http://twistedmatrix.com"
slaveport = 9986

I worry a bit that this hides too much of the magic, but on the other hand it could make the config file less scary to non-python programmers while not impeding the python-literate in the slightest (pre-populated names would be completely overriden by any names defined in the config file itself, so there are no reserved names that config file authors must avoid).