Add links to MP and resolve minor fixes

Add links to MP and resolve minor fixes

A big change here is how external linking happens.
A prefix gets passed in through the server details api
and is applied. milo_prefix and sk_revision_prefix
have been added along with the one for Machine Provider.

Machine provider stuff shows up on bot-list as well as bot-page.

Fixed layout on bot-page with long ids

Fixed disk_space being off by 6 orders of magnitude

Fixed case where bot state could be "null"

BUG=631047
Committed: https://github.com/luci/luci-py/commit/045063d086d171bf39d24fe258a7c04a570e0e6f

[kjlubick, 2016-10-10 14:38:48]

Description was changed from

==========
Add links to MP and resolve minor fixes

Fixed layout on bot-page with long ids

Fixed disk_space being off by 6 orders of magnitude

Fixed case where bot state could be "null"

BUG=
==========

to

==========
Add links to MP and resolve minor fixes

A big change here is how external linking happens.
A prefix gets passed in through the server details api
and is applied. milo_prefix and sk_revision_prefix
have been added along with the one for Machine Provider.

Machine provider stuff shows up on bot-list as well as bot-page.

Fixed layout on bot-page with long ids

Fixed disk_space being off by 6 orders of magnitude

Fixed case where bot state could be "null"

BUG=
==========

[kjlubick, 2016-10-10 14:38:48]

kjlubick@google.com changed reviewers:
+ jcgregorio@chromium.org, maruel@chromium.org, stephana@chromium.org

[kjlubick, 2016-10-10 14:40:33]

Links:
https://screenshot.googleplex.com/Q7nmWJxvW2y

On bot list:
https://screenshot.googleplex.com/X7VZJovAOjv

On bot list (verbose):
https://screenshot.googleplex.com/BwY4dfkuxB1

Unfortunately, the linking doesn't work on staging (and won't work right away on
prod) because the server isn't set in the configs: see
infradata/config/+/master/configs/chromium-swarm-dev/settings.cfg#27

[kjlubick, 2016-10-10 15:13:56]

Description was changed from

==========
Add links to MP and resolve minor fixes

A big change here is how external linking happens.
A prefix gets passed in through the server details api
and is applied. milo_prefix and sk_revision_prefix
have been added along with the one for Machine Provider.

Machine provider stuff shows up on bot-list as well as bot-page.

Fixed layout on bot-page with long ids

Fixed disk_space being off by 6 orders of magnitude

Fixed case where bot state could be "null"

BUG=
==========

to

==========
Add links to MP and resolve minor fixes

A big change here is how external linking happens.
A prefix gets passed in through the server details api
and is applied. milo_prefix and sk_revision_prefix
have been added along with the one for Machine Provider.

Machine provider stuff shows up on bot-list as well as bot-page.

Fixed layout on bot-page with long ids

Fixed disk_space being off by 6 orders of magnitude

Fixed case where bot state could be "null"

BUG=631047
==========

[M-A Ruel, 2016-10-11 16:26:55]

+Ryan and Erik fyi about milo back-links.

https://codereview.chromium.org/2404883002/diff/40001/appengine/swarming/hand...
File appengine/swarming/handlers_endpoints.py (right):

https://codereview.chromium.org/2404883002/diff/40001/appengine/swarming/hand...
appengine/swarming/handlers_endpoints.py:139: mpp = ""
Use single quotes.

https://codereview.chromium.org/2404883002/diff/40001/appengine/swarming/hand...
appengine/swarming/handlers_endpoints.py:140: logging.info("Settings:
%s",config.settings())
you should log the returned object if you want to return anything.

https://codereview.chromium.org/2404883002/diff/40001/appengine/swarming/prot...
File appengine/swarming/proto/config.proto (right):

https://codereview.chromium.org/2404883002/diff/40001/appengine/swarming/prot...
appengine/swarming/proto/config.proto:46: // A partial url which a Skia git
revision will be appended. The result
I think we need a more generic way to define this since this doesn't scale.

One thing I could propose is to parse a tag that would include the link to the
source code itself. It's cool because then one can search for tasks with this
commit hash.

[kjlubick, 2016-10-11 17:00:03]

https://codereview.chromium.org/2404883002/diff/40001/appengine/swarming/hand...
File appengine/swarming/handlers_endpoints.py (right):

https://codereview.chromium.org/2404883002/diff/40001/appengine/swarming/hand...
appengine/swarming/handlers_endpoints.py:139: mpp = ""
On 2016/10/11 at 16:26:55, M-A Ruel wrote:
> Use single quotes.

Done.

https://codereview.chromium.org/2404883002/diff/40001/appengine/swarming/hand...
appengine/swarming/handlers_endpoints.py:140: logging.info("Settings:
%s",config.settings())
On 2016/10/11 at 16:26:55, M-A Ruel wrote:
> you should log the returned object if you want to return anything.

Oops, left over logging. Removed.

https://codereview.chromium.org/2404883002/diff/40001/appengine/swarming/prot...
File appengine/swarming/proto/config.proto (right):

https://codereview.chromium.org/2404883002/diff/40001/appengine/swarming/prot...
appengine/swarming/proto/config.proto:46: // A partial url which a Skia git
revision will be appended. The result
On 2016/10/11 at 16:26:55, M-A Ruel wrote:
> I think we need a more generic way to define this since this doesn't scale.
> 
> One thing I could propose is to parse a tag that would include the link to the
source code itself. It's cool because then one can search for tasks with this
commit hash.

I agree.  I'll rework this to deal with a tag instead.

[estaab, 2016-10-11 18:28:36]

estaab@chromium.org changed reviewers:
+ estaab@chromium.org

[estaab, 2016-10-11 18:28:37]

https://codereview.chromium.org/2404883002/diff/60001/appengine/swarming/prot...
File appengine/swarming/proto/config.proto (right):

https://codereview.chromium.org/2404883002/diff/60001/appengine/swarming/prot...
appengine/swarming/proto/config.proto:44: optional string milo_server_prefix =
11;
We probably don't want swarming to know about services built on top of it, e.g.
milo. How about display_server or build_server?

Also, it should probably be template rather than prefix since the swarming task
ID could appear anywhere (a simple example: you want to add some URL parameters
to the milo URL to alter the presentation on the landing page, "?display=grid").

So I'd suggest display_server_url_template that you'd set to
"https://luci-milo.appspot.com/swarming/task/%s". How does that sound?

https://codereview.chromium.org/2404883002/diff/60001/appengine/swarming/prot...
appengine/swarming/proto/config.proto:48: optional string sk_revision_prefix =
12;
As above, something like git_revision_url_template.

[kjlubick, 2016-10-12 13:55:56]

https://codereview.chromium.org/2404883002/diff/60001/appengine/swarming/prot...
File appengine/swarming/proto/config.proto (right):

https://codereview.chromium.org/2404883002/diff/60001/appengine/swarming/prot...
appengine/swarming/proto/config.proto:44: optional string milo_server_prefix =
11;
On 2016/10/11 at 18:28:37, estaab wrote:
> We probably don't want swarming to know about services built on top of it,
e.g. milo. How about display_server or build_server?
> 
> Also, it should probably be template rather than prefix since the swarming
task ID could appear anywhere (a simple example: you want to add some URL
parameters to the milo URL to alter the presentation on the landing page,
"?display=grid").
> 
> So I'd suggest display_server_url_template that you'd set to
"https://luci-milo.appspot.com/swarming/task/%s". How does that sound?

I like it.  Done.

https://codereview.chromium.org/2404883002/diff/60001/appengine/swarming/prot...
appengine/swarming/proto/config.proto:48: optional string sk_revision_prefix =
12;
On 2016/10/11 at 18:28:37, estaab wrote:
> As above, something like git_revision_url_template.

I'm actually just removing this, as the info is passed in via the tag sk_repo
and that will be more scalable for other projects as well.

[kjlubick, 2016-10-12 13:59:51]

https://codereview.chromium.org/2404883002/diff/40001/appengine/swarming/prot...
File appengine/swarming/proto/config.proto (right):

https://codereview.chromium.org/2404883002/diff/40001/appengine/swarming/prot...
appengine/swarming/proto/config.proto:46: // A partial url which a Skia git
revision will be appended. The result
On 2016/10/11 at 17:00:03, kjlubick wrote:
> On 2016/10/11 at 16:26:55, M-A Ruel wrote:
> > I think we need a more generic way to define this since this doesn't scale.
> > 
> > One thing I could propose is to parse a tag that would include the link to
the source code itself. It's cool because then one can search for tasks with
this commit hash.
> 
> I agree.  I'll rework this to deal with a tag instead.

As it turns out, there was a sk_repo tag already being used, so I reworked the
code to use that instead.

[M-A Ruel, 2016-10-12 19:35:07]

lgtm with comments

https://codereview.chromium.org/2404883002/diff/90001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/tasklist/task-list.html (right):

https://codereview.chromium.org/2404883002/diff/90001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/tasklist/task-list.html:251: var
specialColumns = ["deduped_from", "name", "state", "bot", "sk_revision"];
This is still very one off. I would have preferred something even more generic.
I understand that this means going back and changing the tag associated to the
tasks. So my take is to:
- Decide for a generic tag name to designate the root commit URL and the commit
hash.
- In addition to this one, add the other one with a more generic name.
- Migrate task triggering to use the new tag.
- Remove sk_ specific code in there.

Same for sk_repo below.

While the above can be done in a follow up, please update
https://github.com/luci/luci-py/blob/master/appengine/swarming/doc/Magic-Valu...
in this CL with information about the special parsing of this tag.

https://codereview.chromium.org/2404883002/diff/90001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/tasklist/task-list.html:431: return repo +
"/+/"+ r;
This is completely GoB specific. I'd like something that would work with
arbitrary git hosting service, e.g. github.

[kjlubick, 2016-10-12 20:02:38]

https://codereview.chromium.org/2404883002/diff/90001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/tasklist/task-list.html (right):

https://codereview.chromium.org/2404883002/diff/90001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/tasklist/task-list.html:251: var
specialColumns = ["deduped_from", "name", "state", "bot", "sk_revision"];
On 2016/10/12 at 19:35:07, M-A Ruel wrote:
> This is still very one off. I would have preferred something even more
generic. I understand that this means going back and changing the tag associated
to the tasks. So my take is to:
> - Decide for a generic tag name to designate the root commit URL and the
commit hash.
> - In addition to this one, add the other one with a more generic name.
> - Migrate task triggering to use the new tag.
> - Remove sk_ specific code in there.
> 
> Same for sk_repo below.
> 
> While the above can be done in a follow up, please update
https://github.com/luci/luci-py/blob/master/appengine/swarming/doc/Magic-Valu...
in this CL with information about the special parsing of this tag.

I can take care of it in this CL and update our swarming task scheduler later. 
Magic Values have been added as source_revision and source_repo.

https://codereview.chromium.org/2404883002/diff/90001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/tasklist/task-list.html:431: return repo +
"/+/"+ r;
On 2016/10/12 at 19:35:07, M-A Ruel wrote:
> This is completely GoB specific. I'd like something that would work with
arbitrary git hosting service, e.g. github.

Done.  Like the other things, it's a "replace %s with the thing you want"

[jcgregorio, 2016-10-13 12:03:44]

jcgregorio@google.com changed reviewers:
+ jcgregorio@google.com

[jcgregorio, 2016-10-13 12:03:46]

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
File appengine/swarming/elements/res/imp/botlist/bot-list-data.html (right):

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
appengine/swarming/elements/res/imp/botlist/bot-list-data.html:216:
bot.lease_expiration_ts = new Date(bot.lease_expiration_ts);
lease_expiration_ts is in seconds but new Date() is in milliseconds.

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
File appengine/swarming/elements/res/imp/botlist/bot-list.html (right):

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
appengine/swarming/elements/res/imp/botlist/bot-list.html:194: <a
href$="[[_mpLink(bot,_server_details.machine_provider_template)]]">
Need space after comma, not consistent with rest of file.

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
appengine/swarming/elements/res/imp/botlist/bot-list.html:327: 
why blank line here?

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
appengine/swarming/elements/res/imp/botlist/bot-list.html:357: if
(bot.lease_expiration_ts < new Date()) {
lease_expiration_ts is in seconds but new Date() is in milliseconds.

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
File appengine/swarming/elements/res/imp/tasklist/task-list.html (right):

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
appengine/swarming/elements/res/imp/tasklist/task-list.html:425: _skLink:
function(task) {
Rename _skLink to _sourceLink.

[kjlubick, 2016-10-13 12:26:49]

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
File appengine/swarming/elements/res/imp/botlist/bot-list-data.html (right):

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
appengine/swarming/elements/res/imp/botlist/bot-list-data.html:216:
bot.lease_expiration_ts = new Date(bot.lease_expiration_ts);
On 2016/10/13 at 12:03:45, jcgregorio wrote:
> lease_expiration_ts is in seconds but new Date() is in milliseconds.

This is what we get from the server
"lease_expiration_ts": "2016-10-12T22:22:00",

Code as written seems to work fine, or did you mean something else?

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
File appengine/swarming/elements/res/imp/botlist/bot-list.html (right):

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
appengine/swarming/elements/res/imp/botlist/bot-list.html:194: <a
href$="[[_mpLink(bot,_server_details.machine_provider_template)]]">
On 2016/10/13 at 12:03:46, jcgregorio wrote:
> Need space after comma, not consistent with rest of file.

Done.

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
appengine/swarming/elements/res/imp/botlist/bot-list.html:327: 
On 2016/10/13 at 12:03:46, jcgregorio wrote:
> why blank line here?

Sometimes, the code just needs to breathe.  jk, removed.

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
appengine/swarming/elements/res/imp/botlist/bot-list.html:357: if
(bot.lease_expiration_ts < new Date()) {
On 2016/10/13 at 12:03:46, jcgregorio wrote:
> lease_expiration_ts is in seconds but new Date() is in milliseconds.

At this point, lease_expiration_ts is now a parsed Date and the comparison seems
to work fine in staging.  See below.

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
File appengine/swarming/elements/res/imp/tasklist/task-list.html (right):

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
appengine/swarming/elements/res/imp/tasklist/task-list.html:425: _skLink:
function(task) {
On 2016/10/13 at 12:03:45, jcgregorio wrote:
> Rename _skLink to _sourceLink.

Done.

[jcgregorio, 2016-10-13 12:43:12]

lgtm

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
File appengine/swarming/elements/res/imp/botlist/bot-list-data.html (right):

https://codereview.chromium.org/2404883002/diff/100001/appengine/swarming/ele...
appengine/swarming/elements/res/imp/botlist/bot-list-data.html:216:
bot.lease_expiration_ts = new Date(bot.lease_expiration_ts);
On 2016/10/13 at 12:26:49, kjlubick wrote:
> On 2016/10/13 at 12:03:45, jcgregorio wrote:
> > lease_expiration_ts is in seconds but new Date() is in milliseconds.
> 
> This is what we get from the server
> "lease_expiration_ts": "2016-10-12T22:22:00",
> 
> Code as written seems to work fine, or did you mean something else?

Ah, I didn't realize it was a string, thought it was an int in seconds from the
epoch.

[kjlubick, 2016-10-13 12:46:34]

The CQ bit was checked by kjlubick@google.com

[kjlubick, 2016-10-13 12:46:35]

The patchset sent to the CQ was uploaded after l-g-t-m from maruel@chromium.org
Link to the patchset: https://codereview.chromium.org/2404883002/#ps110001
(title: "Address Polymer comments")

[commit-bot: I haz the power, 2016-10-13 12:46:40]

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, 2016-10-13 12:50:22]

Description was changed from

==========
Add links to MP and resolve minor fixes

A big change here is how external linking happens.
A prefix gets passed in through the server details api
and is applied. milo_prefix and sk_revision_prefix
have been added along with the one for Machine Provider.

Machine provider stuff shows up on bot-list as well as bot-page.

Fixed layout on bot-page with long ids

Fixed disk_space being off by 6 orders of magnitude

Fixed case where bot state could be "null"

BUG=631047
==========

to

==========
Add links to MP and resolve minor fixes

A big change here is how external linking happens.
A prefix gets passed in through the server details api
and is applied. milo_prefix and sk_revision_prefix
have been added along with the one for Machine Provider.

Machine provider stuff shows up on bot-list as well as bot-page.

Fixed layout on bot-page with long ids

Fixed disk_space being off by 6 orders of magnitude

Fixed case where bot state could be "null"

BUG=631047

Committed:
https://github.com/luci/luci-py/commit/045063d086d171bf39d24fe258a7c04a570e0e6f
==========

[commit-bot: I haz the power, 2016-10-13 12:50:24]

Committed patchset #8 (id:110001) as
https://github.com/luci/luci-py/commit/045063d086d171bf39d24fe258a7c04a570e0e6f