Milo: Console view prototype

appengine/cmd/milo/buildbot/console.go

Index: appengine/cmd/milo/buildbot/console.go
diff --git a/appengine/cmd/milo/buildbot/console.go b/appengine/cmd/milo/buildbot/console.go
new file mode 100644
index 0000000000000000000000000000000000000000..05b6d7befb20a2ec4814572e121df295abf89477
--- /dev/null
+++ b/appengine/cmd/milo/buildbot/console.go
@@ -0,0 +1,92 @@
+// Copyright 2016 The LUCI Authors. All rights reserved.
+// Use of this source code is governed under the Apache License, Version 2.0
+// that can be found in the LICENSE file.
+
+package buildbot
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/luci/gae/service/datastore"
+	"github.com/luci/luci-go/appengine/cmd/milo/resp"
+	"github.com/luci/luci-go/common/clock"
+	log "github.com/luci/luci-go/common/logging"
+	"github.com/luci/luci-go/common/sync/parallel"
+	"golang.org/x/net/context"
+)
+
+// getFullBuilds fetches all of the recent builds from the datastore.
+func getFullBuilds(c context.Context, masterName, builderName string, finished bool) ([]*buildbotBuild, error) {
+	// TODO(hinoka): Builder specific structs.
+	ds := datastore.Get(c)
+	q := datastore.NewQuery("buildbotBuild")
+	q = q.Eq("finished", finished)
+	q = q.Eq("master", masterName)
+	q = q.Eq("builder", builderName)
+	q = q.Limit(25) // TODO(hinoka): This should be adjustable
+	q = q.Order("-number")
+	q.Finalize()
+	buildbots := make([]*buildbotBuild, 0, 25)
+	err := ds.GetAll(q, &buildbots)
+	return buildbots, err
+}
+
+func GetConsoleBuilds(
+	c context.Context, builders []resp.BuilderRef, commits []string) (
+	[][]*resp.ConsoleBuild, error) {

[nodir, 2016/08/04 23:13:12]

document what indexes in the return value mean

[hinoka, 2016/08/05 00:10:41]

Done.

+
+	results := make([][]*resp.ConsoleBuild, len(commits))
+	for i := range results {
+		results[i] = make([]*resp.ConsoleBuild, len(builders))
+	}
+	// HACK(hinoka): This fetches 25 full builds and then filters them. Replace this
+	// with something more reasonable.
+	// This is kind of a hack but it's okay for now.
+	err := parallel.FanOutIn(func(taskC chan<- func() error) {
+		for i, builder := range builders {
+			i := i // How do scopes werk
+			f := strings.SplitN(builder.Name, "/", 2)

[nodir, 2016/08/04 23:13:13]

i am not sure what f stands for

[hinoka, 2016/08/05 00:10:42]

Done.

+			if len(f) != 2 {
+				taskC <- func() error {
+					return fmt.Errorf("%s is an invalid builder name", builder.Name)

[nodir, 2016/08/04 23:13:12]

builder var is captured, so there is a race
you need to do `builder := builder` in the beginning of the loop

[hinoka, 2016/08/05 00:10:41]

Oh good catch

+				}
+				return
+			}
+			master := f[0]
+			builderName := f[1]
+			taskC <- func() error {
+				t1 := clock.Now(c)
+				builds, err := getFullBuilds(c, master, builderName, true)
+				if err != nil {
+					return err
+				}
+				t2 := clock.Now(c)
+				var currentStatus *resp.Status
+				for j, commit := range commits {
+					for _, build := range builds {
+						if build.Sourcestamp.Revision == commit {

[nodir, 2016/08/04 23:13:12]

why does this have only one source stamp? a build may span multiple commits if
buildbot build requests for different commits got merged into one build

[hinoka, 2016/08/05 00:10:41]

This is buildbot's own concept of sourcestamp, that is, the revision under
testing.  This is different than the concept of build request merging.

+							results[j][i] = &resp.ConsoleBuild{

[nodir, 2016/08/04 23:13:12]

since there is no "break" in the end of this `if`, results[i][j] can be
overwritten by another build in |builds| that has same revision

this also demonstrates a design problem: what if there are multiple builds for
the same commit? do we show an arbitrary one? (that's not good)

maybe the return value should be [][][]resp.ConsoleBuild

[hinoka, 2016/08/05 00:10:41]

I think we should pick the latest one (This code won't pick the latest one, but
that's okay for now).  But I agree we should discuss what should happen with
multiple builds, particularly since DM has native support for multiple attempts
on the same quest.

+								Link: &resp.Link{
+									Label: strings.Join(build.Text, " "),
+									URL: fmt.Sprintf(
+										"/buildbot/%s/%s/%d", master, builderName, build.Number),
+								},
+								Status: build.toStatus(),
+							}
+							currentStatus = &results[j][i].Status
+						}
+					}
+					if currentStatus != nil && results[j][i] == nil {
+						results[j][i] = &resp.ConsoleBuild{Status: *currentStatus}
+					}
+				}
+				log.Debugf(c,
+					"Builder %s took %s to query, %s to compute.", builderName,
+					t2.Sub(t1), clock.Since(c, t2))
+				return nil
+			}
+		}
+	})
+	return results, err
+}