[Buildbot-devel] New, more efficient web stuff

[Brian Warner]

On Fri, 4 Jan 2008 11:01:59 -0500
"Dustin J. Mitchell" <dustin at zmanda.com> wrote:

> I'm working on some major changes to both the web subsystem and to the
> storage backend

Yay!

> * status is just the current state -- what's in the configuration, and
> what's going on right now.  It also generates events for subscribers.
> * storage keeps the historical record, in a fairly straightforward
> object-persistence architecture with pluggable backends.

Yay! The buildbot design has IStatus and IBuilderStatus and friends be the
boundary between build status (past, present, and future) and anyone who is
interested in it. I'm interested in keeping this interface intact.. I
couldn't tell if you were suggesting that storage (past build status) be
reached through a different interface than current build status, but if so,
I'd prefer to see if we could improve the I*Status interfaces to let it be
used for both purposes.

> * the web is no longer a status subscriber, since it's not in the
> business of receiving events, but of displaying the historical record
> and the current status.

Yes, good.

> Instead, it's configured as an "extra_service" -- basically, a Twisted
> service that gets (re)started by the buildmaster, and does its own thing
> from there on out.

This feels funny to me. I think that present+past-oriented status plugins
(like the web display) can use IStatus in a non-subscription mode, as they
are right now. Status plugins that only care about future builds (like the
email sender) can subscribe instead of ever asking about historical builds.

Why make the non-subscribing status plugings behave at all differently than
the yes-subscribing ones?

> * HTML generation is strictly the domain of the web service -- objects
> themselves no longer generate HTML (>>ahem<<, Changes.asHTML), nor is
> the buildmaster in the business of generating URLs.

Excellent. The web status currently uses Interfaces and Adapters for many
things (including how to take an IBuildStatus and turn it into a single page,
or into the yellow box on the waterfall page that begins a build). It would
be great to finish this effort.

Note that the two examples you mentioned have a few extenuating
circumstances. Changes.asHTML should be cleaned up, but the reason I skipped
over it before was that there's some lingering support for change "links":
each Change should be able to point at a cvsweb- or trac-browser- style web
page that describes that change. This support is suffering from bitrot, but I
think it should be cleaned up and improved. Anyways that would be a reason
that the Change object might want to be involved in the HTMLization process.
Really it should just have a method to return one or more URLs, and then the
web-status-side code could format them into links as it sees fit.

The IStatus.getURLForThing() method you point to may be used by non-web
status plugins: it's how the email sender figures out what URLs to put in its
messages, for example. I agree it's kind of ugly, but I think we need the
functionality somewhere. The approach I'd consider is to have the email
plugin be able to find out if there's a web plugin (maybe
buildmaster.getStatusPlugin("web") ? ) and then ask the web plugin for the
URL for a specific object.

> * Twisted interfaces and adapters are used heaviliy to convert various
> business objects (status or storage) into web pages (IPage), snippets
> of HTML (ISnippet) for inclusion in a larger page, or URLs (IURL) for
> links.

Good, very good. IURL(build) is even better than
IStatus.getURLForThing(build), although it's going to get tricky to make sure
that, e.g., updating the WebStatus to listen on a different port causes
IURL(build) to start returning different URLs. Maybe the adapter should get
re-registered each time WebStatus.startService() gets called. Make sure to
have a couple of test cases that cover this.

> * write the "write" side of the storage interfaces -- how data gets
> *into* storage -- an implement it

Again, I think the existing interface between buildbot.process.base.Build and
buildbot.status.builder.BuildStatus should either be sufficient or improved
to make it sufficient. We don't have a named Interface for this, but we
should (IBuildStatusWriter?), to cover the methods like addStepWithName,
setProperty, setSourceStamp, etc.

I think that it should be possible to replace buildbot/status/builder.py with
a SQLite-based backend without changing *any* of the code in
buildbot/process/ or buildbot/status/web/ . However, splitting
buildbot/status/builder.py into separate "current status" and "historical
status" pieces (while retaining the externally-facing API) sounds like a
great plan.


> I'll admit that I'm embarrassed at the mess I made out of the waterfall. I
> tried to break the problem down into its functional components, but
> apparently missed some subtleties of when spans begin and end, etc.

Heh :). The waterfall page is easily the hardest part of the entire buildbot.
About every two years I completely rewrite the whole thing, and yet it never
seems to get cleaner or simpler. It's also the oldest part of the code, and
every time I look at it I get to see how much I've matured as a developer
since then :). Part of the reason I'm interested in making new web pages to
display status (one-line-per-build, etc) is because the waterfall is a
nightmare in many dimensions.

You'll note that there are some scaffolding methods in Waterfall that aren't
being used, "phase0/phase1", etc. These were used to create waterfall pages
that didn't try so hard to elide the empty boxes between builds.. there's a
lot of code to "bubble up" the idle box (immediately after a build) to the
last idle box just before a new build, so it can then set the ROWSPAN=
argument to just the right value to let that one box fill the space.

Sometimes I wonder if it might have been easier to render a gigantic PNG with
the whole display instead.. :).

> Dustin


Thanks so much for working on this!

cheers,
 -Brian