Refactor post requests, implement bot cancel/terminate

Refactor post requests, implement bot cancel/terminate

There is a prompt before the delete/cancel goes through.  

Tasks that fail are highlighted red, tasks with BOT_DIED are grey, and the refresh button also works.

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

[kjlubick, 2016-09-01 18:50:12]

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

[kjlubick, 2016-09-01 18:50:12]

The prompt:
https://screenshot.googleplex.com/qx9MGK1EnCt

The toast after completion:
https://screenshot.googleplex.com/47pwic6DVv1

It's live on
https://2316-bae452c-tainted-kjlubick-dot-chromium-swarm-dev.appspot.com, but
only admins can delete bots.

[kjlubick, 2016-09-01 18:50:36]

Description was changed from

==========
Refactor post requests, implement bot cancel/terminate


Tasks that fail are highlighted red, and the refresh button also works.

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

to

==========
Refactor post requests, implement bot cancel/terminate

There is a prompt before the delete/cancel goes through.  

Tasks that fail are highlighted red, and the refresh button also works.

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

[kjlubick, 2016-09-02 17:09:55]

PTAL

[jcgregorio, 2016-09-02 18:31:22]

https://codereview.chromium.org/2302973002/diff/40001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/common/common-behavior.html (right):

https://codereview.chromium.org/2302973002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/common-behavior.html:71: 
So there's already a similar solution that we have in sk.

sk common.js already is included here and includes the sk.errorMessage function:

  https://github.com/google/skia-buildbot/blob/master/res/js/common.js#L68

The only thing you're missing is an element to catch the errors, something like
error-toast-sk:

 
https://github.com/google/skia-buildbot/blob/master/res/imp/error-toast-sk.html

[kjlubick, 2016-09-06 13:06:31]

https://codereview.chromium.org/2302973002/diff/40001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/common/common-behavior.html (right):

https://codereview.chromium.org/2302973002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/common-behavior.html:71: 
On 2016/09/02 at 18:31:22, jcgregorio wrote:
> So there's already a similar solution that we have in sk.
> 
> sk common.js already is included here and includes the sk.errorMessage
function:
> 
>   https://github.com/google/skia-buildbot/blob/master/res/js/common.js#L68
> 
> The only thing you're missing is an element to catch the errors, something
like error-toast-sk:
> 
>  
https://github.com/google/skia-buildbot/blob/master/res/imp/error-toast-sk.html

I considered using sk.errorMessage, but it doesn't allow me to keep the toast
open while the request is pending, which is something I want.

[jcgregorio, 2016-09-06 15:16:53]

https://codereview.chromium.org/2302973002/diff/40001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/common/common-behavior.html (right):

https://codereview.chromium.org/2302973002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/common-behavior.html:71: 
On 2016/09/06 at 13:06:31, kjlubick wrote:
> On 2016/09/02 at 18:31:22, jcgregorio wrote:
> > So there's already a similar solution that we have in sk.
> > 
> > sk common.js already is included here and includes the sk.errorMessage
function:
> > 
> >   https://github.com/google/skia-buildbot/blob/master/res/js/common.js#L68
> > 
> > The only thing you're missing is an element to catch the errors, something
like error-toast-sk:
> > 
> >  
https://github.com/google/skia-buildbot/blob/master/res/imp/error-toast-sk.html
> 
> I considered using sk.errorMessage, but it doesn't allow me to keep the toast
open while the request is pending, which is something I want.

I don't understand, why are you keeping it open?

[kjlubick, 2016-09-06 15:26:50]

https://codereview.chromium.org/2302973002/diff/40001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/common/common-behavior.html (right):

https://codereview.chromium.org/2302973002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/common-behavior.html:71: 
On 2016/09/06 at 15:16:53, jcgregorio wrote:
> On 2016/09/06 at 13:06:31, kjlubick wrote:
> > On 2016/09/02 at 18:31:22, jcgregorio wrote:
> > > So there's already a similar solution that we have in sk.
> > > 
> > > sk common.js already is included here and includes the sk.errorMessage
function:
> > > 
> > >   https://github.com/google/skia-buildbot/blob/master/res/js/common.js#L68
> > > 
> > > The only thing you're missing is an element to catch the errors, something
like error-toast-sk:
> > > 
> > >  
https://github.com/google/skia-buildbot/blob/master/res/imp/error-toast-sk.html
> > 
> > I considered using sk.errorMessage, but it doesn't allow me to keep the
toast open while the request is pending, which is something I want.
> 
> I don't understand, why are you keeping it open?

It just feels like a better way to communicate with the user that something is
happening, that the request is pending.  If the toast closes after 10 seconds
(like error-toast-sk), it might be confusing to the user that the request is
still trying to go through.  Maybe not.

Maybe I can make my own version of error-toast-sk that takes an argument to
"stay open" or not.  What do you think?

[jcgregorio, 2016-09-06 15:33:33]

https://codereview.chromium.org/2302973002/diff/40001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/common/common-behavior.html (right):

https://codereview.chromium.org/2302973002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/common-behavior.html:71: 
On 2016/09/06 at 15:26:50, kjlubick wrote:
> On 2016/09/06 at 15:16:53, jcgregorio wrote:
> > On 2016/09/06 at 13:06:31, kjlubick wrote:
> > > On 2016/09/02 at 18:31:22, jcgregorio wrote:
> > > > So there's already a similar solution that we have in sk.
> > > > 
> > > > sk common.js already is included here and includes the sk.errorMessage
function:
> > > > 
> > > >  
https://github.com/google/skia-buildbot/blob/master/res/js/common.js#L68
> > > > 
> > > > The only thing you're missing is an element to catch the errors,
something like error-toast-sk:
> > > > 
> > > >  
https://github.com/google/skia-buildbot/blob/master/res/imp/error-toast-sk.html
> > > 
> > > I considered using sk.errorMessage, but it doesn't allow me to keep the
toast open while the request is pending, which is something I want.
> > 
> > I don't understand, why are you keeping it open?
> 
> It just feels like a better way to communicate with the user that something is
happening, that the request is pending.  If the toast closes after 10 seconds
(like error-toast-sk), it might be confusing to the user that the request is
still trying to go through.  Maybe not.
> 
> Maybe I can make my own version of error-toast-sk that takes an argument to
"stay open" or not.  What do you think?

Yes, or add a parameter to error-toast-sk, either way would be fine.

[kjlubick, 2016-09-06 20:19:30]

On 2016/09/06 at 15:33:33, jcgregorio wrote:
>
https://codereview.chromium.org/2302973002/diff/40001/appengine/swarming/elem...
> File appengine/swarming/elements/res/imp/common/common-behavior.html (right):
> 
>
https://codereview.chromium.org/2302973002/diff/40001/appengine/swarming/elem...
> appengine/swarming/elements/res/imp/common/common-behavior.html:71: 
> On 2016/09/06 at 15:26:50, kjlubick wrote:
> > On 2016/09/06 at 15:16:53, jcgregorio wrote:
> > > On 2016/09/06 at 13:06:31, kjlubick wrote:
> > > > On 2016/09/02 at 18:31:22, jcgregorio wrote:
> > > > > So there's already a similar solution that we have in sk.
> > > > > 
> > > > > sk common.js already is included here and includes the sk.errorMessage
function:
> > > > > 
> > > > >  
https://github.com/google/skia-buildbot/blob/master/res/js/common.js#L68
> > > > > 
> > > > > The only thing you're missing is an element to catch the errors,
something like error-toast-sk:
> > > > > 
> > > > >  
https://github.com/google/skia-buildbot/blob/master/res/imp/error-toast-sk.html
> > > > 
> > > > I considered using sk.errorMessage, but it doesn't allow me to keep the
toast open while the request is pending, which is something I want.
> > > 
> > > I don't understand, why are you keeping it open?
> > 
> > It just feels like a better way to communicate with the user that something
is happening, that the request is pending.  If the toast closes after 10 seconds
(like error-toast-sk), it might be confusing to the user that the request is
still trying to go through.  Maybe not.
> > 
> > Maybe I can make my own version of error-toast-sk that takes an argument to
"stay open" or not.  What do you think?
> 
> Yes, or add a parameter to error-toast-sk, either way would be fine.

Done.  Requires https://codereview.chromium.org/2309423004 to land

[jcgregorio, 2016-09-06 20:40:56]

https://codereview.chromium.org/2302973002/diff/60001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/common/common-behavior.html (right):

https://codereview.chromium.org/2302973002/diff/60001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/common-behavior.html:74:
_postWithToast: function(url, msg, auth_headers) {
Now that you are using errorMessage and the error-toast is a separate element,
this doesn't need to be part of a behavior, it can go into the swarming JS.

https://codereview.chromium.org/2302973002/diff/60001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/common/error-toast.html (right):

https://codereview.chromium.org/2302973002/diff/60001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/error-toast.html:12: message: "The
error message to display goes here.",
Document duration in the example.

[kjlubick, 2016-09-07 12:00:14]

Description was changed from

==========
Refactor post requests, implement bot cancel/terminate

There is a prompt before the delete/cancel goes through.  

Tasks that fail are highlighted red, and the refresh button also works.

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

to

==========
Refactor post requests, implement bot cancel/terminate

There is a prompt before the delete/cancel goes through.  

Tasks that fail are highlighted red, tasks with BOT_DIED are grey, and the
refresh button also works.

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

[kjlubick, 2016-09-07 12:00:21]

https://codereview.chromium.org/2302973002/diff/60001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/common/common-behavior.html (right):

https://codereview.chromium.org/2302973002/diff/60001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/common-behavior.html:74:
_postWithToast: function(url, msg, auth_headers) {
On 2016/09/06 at 20:40:56, jcgregorio wrote:
> Now that you are using errorMessage and the error-toast is a separate element,
this doesn't need to be part of a behavior, it can go into the swarming JS.

Done.

https://codereview.chromium.org/2302973002/diff/60001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/common/error-toast.html (right):

https://codereview.chromium.org/2302973002/diff/60001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/error-toast.html:12: message: "The
error message to display goes here.",
On 2016/09/06 at 20:40:56, jcgregorio wrote:
> Document duration in the example.

Done.

[jcgregorio, 2016-09-07 12:12:48]

LGTM, with a few blank line nits.

https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/botpage/bot-page.html (right):

https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/botpage/bot-page.html:83: .dead, .bot_died {
Each class should be on a new line:

.dead,
.bot_died {

Here and above for .quarantined.

https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/common/common-behavior.html (right):

https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/common-behavior.html:71: 
Remove blank line.

https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/common/error-toast.html (right):

https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/error-toast.html:19: Events:
Blank lines before Events: and Methods:

https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/error-toast.html:33: ready:
function() {
Blank line before ready:

[kjlubick, 2016-09-07 12:36:38]

https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/botpage/bot-page.html (right):

https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/botpage/bot-page.html:83: .dead, .bot_died {
On 2016/09/07 at 12:12:48, jcgregorio wrote:
> Each class should be on a new line:
> 
> .dead,
> .bot_died {
> 
> Here and above for .quarantined.

Done.  Should we add this to the style guide?

https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/common/common-behavior.html (right):

https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/common-behavior.html:71: 
On 2016/09/07 at 12:12:48, jcgregorio wrote:
> Remove blank line.

Done.

https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/common/error-toast.html (right):

https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/error-toast.html:19: Events:
On 2016/09/07 at 12:12:48, jcgregorio wrote:
> Blank lines before Events: and Methods:

Done.

https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/error-toast.html:33: ready:
function() {
On 2016/09/07 at 12:12:48, jcgregorio wrote:
> Blank line before ready:

Done.

[kjlubick, 2016-09-07 12:36:41]

The CQ bit was checked by kjlubick@google.com

[kjlubick, 2016-09-07 12:36:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
jcgregorio@google.com
Link to the patchset: https://codereview.chromium.org/2302973002/#ps100001
(title: "Address nits")

[commit-bot: I haz the power, 2016-09-07 12:36:46]

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-09-07 12:36:47]

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

[commit-bot: I haz the power, 2016-09-07 12:36:50]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
Committers are members of the group "project-infra-committers".
Note that this has nothing to do with OWNERS files.

[KevinL, 2016-09-07 12:36:55]

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

[KevinL, 2016-09-07 12:36:55]

lgtm

[KevinL, 2016-09-07 12:36:58]

The CQ bit was checked by kjlubick@chromium.org

[commit-bot: I haz the power, 2016-09-07 12:37:03]

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

[stephana, 2016-09-07 12:38:19]

On 2016/09/07 12:12:48, jcgregorio wrote:
> LGTM, with a few blank line nits.
> 
>
https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
> File appengine/swarming/elements/res/imp/botpage/bot-page.html (right):
> 
>
https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
> appengine/swarming/elements/res/imp/botpage/bot-page.html:83: .dead, .bot_died
{
> Each class should be on a new line:
> 
> .dead,
> .bot_died {
> 
> Here and above for .quarantined.
> 
>
https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
> File appengine/swarming/elements/res/imp/common/common-behavior.html (right):
> 
>
https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
> appengine/swarming/elements/res/imp/common/common-behavior.html:71: 
> Remove blank line.
> 
>
https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
> File appengine/swarming/elements/res/imp/common/error-toast.html (right):
> 
>
https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
> appengine/swarming/elements/res/imp/common/error-toast.html:19: Events:
> Blank lines before Events: and Methods:
> 
>
https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
> appengine/swarming/elements/res/imp/common/error-toast.html:33: ready:
> function() {
> Blank line before ready:

lgtm

[commit-bot: I haz the power, 2016-09-07 12:40:24]

Description was changed from

==========
Refactor post requests, implement bot cancel/terminate

There is a prompt before the delete/cancel goes through.  

Tasks that fail are highlighted red, tasks with BOT_DIED are grey, and the
refresh button also works.

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

to

==========
Refactor post requests, implement bot cancel/terminate

There is a prompt before the delete/cancel goes through.  

Tasks that fail are highlighted red, tasks with BOT_DIED are grey, and the
refresh button also works.

BUG=631047

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

[commit-bot: I haz the power, 2016-09-07 12:40:25]

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

[jcgregorio, 2016-09-07 12:41:21]

https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/botpage/bot-page.html (right):

https://codereview.chromium.org/2302973002/diff/80001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/botpage/bot-page.html:83: .dead, .bot_died {
On 2016/09/07 at 12:36:38, kjlubick wrote:
> On 2016/09/07 at 12:12:48, jcgregorio wrote:
> > Each class should be on a new line:
> > 
> > .dead,
> > .bot_died {
> > 
> > Here and above for .quarantined.
> 
> Done.  Should we add this to the style guide?

Sure.