Milo: Add machine pool info for buildbot builder view.

Milo: Add machine pool info for buildbot builder view.

* Add bot list to machine pool struct
* Add machine pool rendering to builder.html
* Bar chart summary for machine pool info
* Buildbot builder view now populates machine info
* Flow builder view such that if there is room, then machine pool, pending builds, and current build takes up the top part of the page

BUG=718487
Review-Url: https://codereview.chromium.org/2931773004
Committed: https://github.com/luci/luci-go/commit/a6c6b4313a95e83ac50ec1bcbb4c5b996ec822de

[Ryan Tseng, 2017-06-08 22:44:29]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-08 22:44:36]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-08 22:51:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-08 22:51:27]

Dry run: This issue passed the CQ dry run.

[Ryan Tseng, 2017-06-09 20:38:43]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-09 20:38:48]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ryan Tseng, 2017-06-09 20:42:15]

Description was changed from

==========
Milo: Add machine pool info for buildbot builder view.

BUG=718487
==========

to

==========
Milo: Add machine pool info for buildbot builder view.

* Add bot list to machine pool struct
* Add machine pool rendering to builder.html
* Buildbot builder view now populates machine info

BUG=718487
==========

[Ryan Tseng, 2017-06-09 20:42:15]

hinoka@google.com changed reviewers:
+ nodir@chromium.org

[commit-bot: I haz the power, 2017-06-09 20:45:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-09 20:45:58]

Dry run: This issue passed the CQ dry run.

[Ryan Tseng, 2017-06-09 20:47:58]

Description was changed from

==========
Milo: Add machine pool info for buildbot builder view.

* Add bot list to machine pool struct
* Add machine pool rendering to builder.html
* Buildbot builder view now populates machine info

BUG=718487
==========

to

==========
Milo: Add machine pool info for buildbot builder view.

* Add bot list to machine pool struct
* Add machine pool rendering to builder.html
* Bar chart summary for machine pool info
* Buildbot builder view now populates machine info

BUG=718487
==========

[Ryan Tseng, 2017-06-09 20:48:06]

ptal, demo:
https://1768-84cdc84-tainted-hinoka-dot-luci-milo-dev.appspot.com/buildbot/tr...

[Ryan Tseng, 2017-06-09 20:53:50]

Description was changed from

==========
Milo: Add machine pool info for buildbot builder view.

* Add bot list to machine pool struct
* Add machine pool rendering to builder.html
* Bar chart summary for machine pool info
* Buildbot builder view now populates machine info

BUG=718487
==========

to

==========
Milo: Add machine pool info for buildbot builder view.

* Add bot list to machine pool struct
* Add machine pool rendering to builder.html
* Bar chart summary for machine pool info
* Buildbot builder view now populates machine info
* Flow builder view such that if there is room, then machine pool, pending
builds, and current build takes up the top part of the page

BUG=718487
==========

[Ryan Tseng, 2017-06-13 00:03:15]

bump

[nodir, 2017-06-13 02:38:40]

https://codereview.chromium.org/2931773004/diff/20001/milo/api/resp/builder.go
File milo/api/resp/builder.go (right):

https://codereview.chromium.org/2931773004/diff/20001/milo/api/resp/builder.g...
milo/api/resp/builder.go:5: //go:generate stringer -type=BotStatus
why is it so far from the BotStatus definition? consider moving this line closer

https://codereview.chromium.org/2931773004/diff/20001/milo/api/resp/builder.g...
milo/api/resp/builder.go:78: Unknown BotStatus = iota
This is a package-global constant. Consider renaming thisto UnknownStatus or
something.

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/buildbot...
File milo/appengine/buildbot/builder.go (right):

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/buildbot...
milo/appengine/buildbot/builder.go:174: bots := make([]resp.Bot, 0, len(slaves))
MachinePool struct has all these variables. Consider using one MachinePool
variable and then returning. It would be simpler

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/buildbot...
milo/appengine/buildbot/builder.go:269: host := "build.chromium.org/p/"
this is not a host. Host cannot have a path.

consider renaming to baseURL and prepending https://

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/common/f...
File milo/appengine/common/funcs.go (right):

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/common/f...
milo/appengine/common/funcs.go:249: // percent divides two numbers and returns
the percentage in string form.
i think it divides one number

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/frontend...
File milo/appengine/frontend/templates/pages/builder.html (right):

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/frontend...
milo/appengine/frontend/templates/pages/builder.html:39: </tr>
copy pasta :(
i don't have suggestions

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/frontend...
milo/appengine/frontend/templates/pages/builder.html:66: {{ if gt $i 0 }},{{ end
}}
i believe this creates a space between an email and comma, e.g. "a@a.a , b@b.b"
while it should be "a@a.a, b@b.b"

you can solve the problem by using `{{- if` here and -}} below

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/frontend...
milo/appengine/frontend/templates/pages/builder.html:73: (<a href="{{ .CommitURL
}}">{{ .AuthorEmail }}</a>)
L69 suggests that AuthorEmail can be empty. Can it? If so, this code path should
handle it too

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/frontend...
milo/appengine/frontend/templates/pages/builder.html:161: {{ end }}
please reduce copy paste by using {{define "blamelist"}}

[Ryan Tseng, 2017-06-13 23:07:57]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-13 23:08:01]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ryan Tseng, 2017-06-13 23:10:33]

https://codereview.chromium.org/2931773004/diff/20001/milo/api/resp/builder.go
File milo/api/resp/builder.go (right):

https://codereview.chromium.org/2931773004/diff/20001/milo/api/resp/builder.g...
milo/api/resp/builder.go:5: //go:generate stringer -type=BotStatus
On 2017/06/13 02:38:40, nodir wrote:
> why is it so far from the BotStatus definition? consider moving this line
closer

This is to get go generate to create botstatus_string.go.  I think the
convention is to put it on top of the file before package and imports.

https://codereview.chromium.org/2931773004/diff/20001/milo/api/resp/builder.g...
milo/api/resp/builder.go:78: Unknown BotStatus = iota
On 2017/06/13 02:38:40, nodir wrote:
> This is a package-global constant. Consider renaming thisto UnknownStatus or
> something. 

Done.  Going to keep the below 3 as is since it makes for less code on the
rendering side.

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/buildbot...
File milo/appengine/buildbot/builder.go (right):

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/buildbot...
milo/appengine/buildbot/builder.go:174: bots := make([]resp.Bot, 0, len(slaves))
On 2017/06/13 02:38:40, nodir wrote:
> MachinePool struct has all these variables. Consider using one MachinePool
> variable and then returning. It would be simpler

Done.

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/buildbot...
milo/appengine/buildbot/builder.go:269: host := "build.chromium.org/p/"
On 2017/06/13 02:38:40, nodir wrote:
> this is not a host. Host cannot have a path.
> 
> consider renaming to baseURL and prepending https://

Done.

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/common/f...
File milo/appengine/common/funcs.go (right):

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/common/f...
milo/appengine/common/funcs.go:249: // percent divides two numbers and returns
the percentage in string form.
On 2017/06/13 02:38:40, nodir wrote:
> i think it divides one number

Done.

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/frontend...
File milo/appengine/frontend/templates/pages/builder.html (right):

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/frontend...
milo/appengine/frontend/templates/pages/builder.html:39: </tr>
On 2017/06/13 02:38:40, nodir wrote:
> copy pasta :(
> i don't have suggestions

Could define a template function, but that seems a little heavyweight

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/frontend...
milo/appengine/frontend/templates/pages/builder.html:66: {{ if gt $i 0 }},{{ end
}}
On 2017/06/13 02:38:40, nodir wrote:
> i believe this creates a space between an email and comma, e.g. mailto:"a@a.a
, mailto:b@b.b"
> while it should be mailto:"a@a.a, mailto:b@b.b"
> 
> you can solve the problem by using `{{- if` here and -}} below

Done.

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/frontend...
milo/appengine/frontend/templates/pages/builder.html:73: (<a href="{{ .CommitURL
}}">{{ .AuthorEmail }}</a>)
On 2017/06/13 02:38:40, nodir wrote:
> L69 suggests that AuthorEmail can be empty. Can it? If so, this code path
should
> handle it too

Done.

https://codereview.chromium.org/2931773004/diff/20001/milo/appengine/frontend...
milo/appengine/frontend/templates/pages/builder.html:161: {{ end }}
On 2017/06/13 02:38:40, nodir wrote:
> please reduce copy paste by using {{define "blamelist"}}

Done.

[commit-bot: I haz the power, 2017-06-13 23:11:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-13 23:11:52]

Dry run: Try jobs failed on following builders:
  Luci-go Linux Trusty 64 Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/36bc8871ed59cf10)

[Ryan Tseng, 2017-06-13 23:17:02]

(Looks like tryservers are broken right now, but tests pass locally)

[Ryan Tseng, 2017-06-16 00:59:40]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-16 00:59:43]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ryan Tseng, 2017-06-16 01:02:26]

hinoka@google.com changed reviewers:
+ iannucci@chromium.org

[Ryan Tseng, 2017-06-16 01:02:27]

Demo:
https://1770-f63e083-tainted-hinoka-dot-luci-milo-dev.appspot.com/buildbot/tr...

[commit-bot: I haz the power, 2017-06-16 01:07:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-16 01:07:14]

Dry run: This issue passed the CQ dry run.

[Ryan Tseng, 2017-06-16 01:13:36]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-16 01:13:43]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-16 01:22:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-16 01:22:34]

Dry run: This issue passed the CQ dry run.

[iannucci, 2017-06-16 01:44:16]

lgtm

https://codereview.chromium.org/2931773004/diff/60001/milo/Makefile
File milo/Makefile (right):

https://codereview.chromium.org/2931773004/diff/60001/milo/Makefile#newcode9
milo/Makefile:9: gae.py devserver -A luci-milo --app-dir $(projdir)/appengine --
--host 0.0.0.0 --port 8082 --admin_port 7999 --log_level debug
intended?

https://codereview.chromium.org/2931773004/diff/60001/milo/api/resp/builder.go
File milo/api/resp/builder.go (right):

https://codereview.chromium.org/2931773004/diff/60001/milo/api/resp/builder.g...
milo/api/resp/builder.go:86: Name   *Link
embed the Link struct. we should use pointers iff:
  * it's a huge object OR
  * it's an optional object

https://codereview.chromium.org/2931773004/diff/60001/milo/appengine/buildbot...
File milo/appengine/buildbot/builder.go (right):

https://codereview.chromium.org/2931773004/diff/60001/milo/appengine/buildbot...
milo/appengine/buildbot/builder.go:184: // This shouldn't happen
what will this render like? unknown?

[Ryan Tseng, 2017-06-16 02:02:22]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-16 02:02:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ryan Tseng, 2017-06-16 02:05:43]

https://codereview.chromium.org/2931773004/diff/60001/milo/Makefile
File milo/Makefile (right):

https://codereview.chromium.org/2931773004/diff/60001/milo/Makefile#newcode9
milo/Makefile:9: gae.py devserver -A luci-milo --app-dir $(projdir)/appengine --
--host 0.0.0.0 --port 8082 --admin_port 7999 --log_level debug
On 2017/06/16 01:44:15, iannucci wrote:
> intended?

Sorta, cept it didn't work, so removed.

https://codereview.chromium.org/2931773004/diff/60001/milo/api/resp/builder.go
File milo/api/resp/builder.go (right):

https://codereview.chromium.org/2931773004/diff/60001/milo/api/resp/builder.g...
milo/api/resp/builder.go:86: Name   *Link
On 2017/06/16 01:44:16, iannucci wrote:
> embed the Link struct. we should use pointers iff:
>   * it's a huge object OR
>   * it's an optional object

Done.

https://codereview.chromium.org/2931773004/diff/60001/milo/appengine/buildbot...
File milo/appengine/buildbot/builder.go (right):

https://codereview.chromium.org/2931773004/diff/60001/milo/appengine/buildbot...
milo/appengine/buildbot/builder.go:184: // This shouldn't happen
On 2017/06/16 01:44:16, iannucci wrote:
> what will this render like? unknown?

It just wouldn't show up in the bar graph, and idle + busy + disconnect < total.
 But it shouldn't happen :D.

In the entry, it should render "UnknownStatus" Since that's the status string
text, with whatever the default color is.

[Ryan Tseng, 2017-06-16 02:05:49]

The CQ bit was unchecked by hinoka@google.com

[Ryan Tseng, 2017-06-16 02:05:54]

The CQ bit was checked by hinoka@google.com

[Ryan Tseng, 2017-06-16 02:05:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
iannucci@chromium.org
Link to the patchset: https://codereview.chromium.org/2931773004/#ps100001
(title: "Review")

[commit-bot: I haz the power, 2017-06-16 02:06:03]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-16 02:14:25]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1497578754397580,
"parent_rev": "30e084f872754dca234b8e4104456f44ec5ee29c", "commit_rev":
"a6c6b4313a95e83ac50ec1bcbb4c5b996ec822de"}

[commit-bot: I haz the power, 2017-06-16 02:14:28]

Description was changed from

==========
Milo: Add machine pool info for buildbot builder view.

* Add bot list to machine pool struct
* Add machine pool rendering to builder.html
* Bar chart summary for machine pool info
* Buildbot builder view now populates machine info
* Flow builder view such that if there is room, then machine pool, pending
builds, and current build takes up the top part of the page

BUG=718487
==========

to

==========
Milo: Add machine pool info for buildbot builder view.

* Add bot list to machine pool struct
* Add machine pool rendering to builder.html
* Bar chart summary for machine pool info
* Buildbot builder view now populates machine info
* Flow builder view such that if there is room, then machine pool, pending
builds, and current build takes up the top part of the page

BUG=718487

Review-Url: https://codereview.chromium.org/2931773004
Committed:
https://github.com/luci/luci-go/commit/a6c6b4313a95e83ac50ec1bcbb4c5b996ec822de
==========

[commit-bot: I haz the power, 2017-06-16 02:14:30]

Committed patchset #6 (id:100001) as
https://github.com/luci/luci-go/commit/a6c6b4313a95e83ac50ec1bcbb4c5b996ec822de