[Buildbot-devel] New, more efficient web stuff

[Dustin J. Mitchell]

On Jan 6, 2008 4:38 PM, Brian Warner <warner-buildbot at lothar.com> wrote:
> 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.

I am suggesting exactly that.  The intention is that  ISnippet, IBox,
and IPage adapters will ordinarily adapt a storage object and display
information about it, but if moment-by-moment data is available in the
form of status information, then they'll display that -- for example,
builds and steps will have ETAs, logs will do their "tail" thing, and
so on.

> 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?

Even if you consider it a generic status plugin, the web service does
even more -- it allows users to control the buildmaster, too.  I just
don't see the web service as a good fit for this paradigm.  Rather, I
think the model of "this is the buildmaster.  It has a web-server
built in." is fairly accurate.

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

My thinking on this issue has been that IURL will actually generate
URLs two ways -- when given an HTTP request, it can generate the URL
based on the incoming request.  When operating *without* a request
then, given some access to the buildmaster, it can generate a URL
based on c['buildbotURL'].  This isn't implemented yet (since nothing
outside of the web service uses IURL yet).

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

My plan was to actually add this to the existing interfaces, rather
than create a new one.

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

Incidentally, I renamed 'one_line_per_build' to just 'builders'.  For
the life of me, I couldn't remember whether builds or builders had
boxes or lines.

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

Yep.  I blew all of that away.  I don't know if you've looked at the
stuff I committed earlier today.  I'd be interested in your thoughts
on all of it, but in particular, on my reimplementation of the
waterfall, and on my storage interfaces.

Here's my justifications for not just adapting the existing status architecture:
  * storage needs to be sprinkled liberally with deferreds, since
fetching objects may be a blocking operation
  * storage will be implemented by multiple backend classes, all
providing the same interface; this makes adding common functionality
(like step.getETA) tricky, unless we use multiple inheritance, which
I'd like to avoid.  I'd like to keep those interfaces simple -- just
CRUD, basically.

How about this: status objects become transient "shells" around their
corresponding persistent object.  As an example, then, an active or
pending build will have a BuildStatus object, and that object will
carry a reference to an IPersistentBuild provider.  The existing
status interface can then keep most of its API -- the part that works
with data in RAM, not in persistent storage -- requiring
correspondingly few changes to other code in the codebase.  Once the
build completes, the BuildStatus object goes away, leaving only the
IPersistentBuild provider to represent the build.

Dustin

-- 
Storage Software Engineer
http://www.zmanda.com