code review 4519047: dashboard: fix for branches

dashboard: fix for branches

In the new world, one builder runs
        gobuilder -commit
which uploads information about commits to the dashboard,
which then hands the work out to the builders by hash.
There is no assumption anymore that the commit numbers
are consistent across builders.

New builders will need to be deployed.  For now darwin-amd64
is running the new builder to test the code.

The new JSON-based protocol for handing out work via /todo
should be easy to extend if we want to add support for sending
trial CLs to the builders.

This code is already running on godashboard.appspot.com.

[rsc, 2011-05-09 22:38:40]

Hello adg (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg

[rsc, 2011-05-10 03:34:18]

Hello adg (cc: golang-dev@googlegroups.com),

Please take another look.

[adg, 2011-05-11 16:07:57]

Thanks again for doing this.

http://codereview.appspot.com/4519047/diff/9001/misc/dashboard/builder/http.go
File misc/dashboard/builder/http.go (right):

http://codereview.appspot.com/4519047/diff/9001/misc/dashboard/builder/http.g...
misc/dashboard/builder/http.go:19: // dash runs the given method and command.
s/.$/ on the dashboard./

http://codereview.appspot.com/4519047/diff/9001/misc/dashboard/builder/http.g...
misc/dashboard/builder/http.go:22: func dash(meth, cmd string, resp interface{},
args param) os.Error {
Nice cleanups in this file. Thanks!

http://codereview.appspot.com/4519047/diff/9001/misc/dashboard/builder/main.go
File misc/dashboard/builder/main.go (right):

http://codereview.appspot.com/4519047/diff/9001/misc/dashboard/builder/main.g...
misc/dashboard/builder/main.go:372: log.Printf("poll...")
These should only be printed in verbose mode.

http://codereview.appspot.com/4519047/diff/9001/misc/dashboard/builder/main.g...
misc/dashboard/builder/main.go:402: func commitPoll(key string) {
This function could really use a comment.

http://codereview.appspot.com/4519047/diff/9001/misc/dashboard/builder/main.g...
misc/dashboard/builder/main.go:431: io.MultiReader(strings.NewReader("<top>"),
strings.NewReader(data), strings.NewReader("</top>")),
Is this a bug? Shouldn't these strings.Readers be on either side of the
&logStruct?

http://codereview.appspot.com/4519047/diff/9001/misc/dashboard/builder/main.g...
misc/dashboard/builder/main.go:450: l.Parent, _, _ = runLog(nil, "", goroot,
"hg", "log",
Couldn't we just use fullHash(l.Parent) here?

http://codereview.appspot.com/4519047/diff/9001/misc/dashboard/builder/main.g...
misc/dashboard/builder/main.go:456: if l.Parent == "" {
Considering we're not checking the error from runLog above, and mercur ial does
strange things sometimes, should we also check the length of l.Parent here?

http://codereview.appspot.com/4519047/diff/9001/misc/dashboard/builder/main.g...
misc/dashboard/builder/main.go:477: func addCommit(hash, key string) bool {
comment

http://codereview.appspot.com/4519047/diff/9001/misc/dashboard/godashboard/go...
File misc/dashboard/godashboard/gobuild.py (right):

http://codereview.appspot.com/4519047/diff/9001/misc/dashboard/godashboard/go...
misc/dashboard/godashboard/gobuild.py:133: class
DashboardHandler(webapp.RequestHandler):
Cool!

http://codereview.appspot.com/4519047/diff/9001/misc/dashboard/godashboard/go...
misc/dashboard/godashboard/gobuild.py:237: if not auth(self.request) or
self.request.get('builder'):
s/not auth .+ or//

http://codereview.appspot.com/4519047/diff/9001/misc/dashboard/godashboard/go...
misc/dashboard/godashboard/gobuild.py:348: # TODO: Send mail for build breakage.
aww

[rsc, 2011-05-12 04:12:17]

Hello adg@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[dave_cheney.net, 2011-05-12 05:51:25]

I have no comments on the code, but with this patch applied the arm5 is no
longer looping on rev 8001.

[adg, 2011-05-12 15:15:10]

LGTM

[rsc, 2011-05-12 15:21:37]

*** Submitted as http://code.google.com/p/go/source/detail?r=740b18a25fcf ***

dashboard: fix for branches

In the new world, one builder runs
        gobuilder -commit
which uploads information about commits to the dashboard,
which then hands the work out to the builders by hash.
There is no assumption anymore that the commit numbers
are consistent across builders.

New builders will need to be deployed.  For now darwin-amd64
is running the new builder to test the code.

The new JSON-based protocol for handing out work via /todo
should be easy to extend if we want to add support for sending
trial CLs to the builders.

This code is already running on godashboard.appspot.com.

R=adg, dave
CC=golang-dev
http://codereview.appspot.com/4519047

http://codereview.appspot.com/4519047/diff/9001/misc/dashboard/builder/main.go
File misc/dashboard/builder/main.go (right):

http://codereview.appspot.com/4519047/diff/9001/misc/dashboard/builder/main.g...
misc/dashboard/builder/main.go:431: io.MultiReader(strings.NewReader("<top>"),
strings.NewReader(data), strings.NewReader("</top>")),
On 2011/05/11 16:07:57, adg wrote:
> Is this a bug? Shouldn't these strings.Readers be on either side of the
> &logStruct?

No, they are on either side of the input.
logStruct is the output.
Although now that you mention it,

strings.NewReader("<top>"+data+"</top>") 

would work better