[Findit] Display last build bucket response to try job dashboard

[Findit] Display last build bucket response to try job dashboard

The UI now looks like this:
https://lijeffrey1-dot-findit-for-me.appspot.com/waterfall/try-job-dashboard?start_date=2016-08-15&end_date=2016-08-18

This CL also fixes a misleading error message when buildbucket reports an error.

BUG=638690, 611866
Committed: https://chromium.googlesource.com/infra/infra/+/ca0df3b15c395d8b5ac01141cdbe4c18bec778c1

[lijeffrey, 2016-08-17 19:18:46]

Description was changed from

==========
[Findit] Display last build bucket response to try job dashboard


[Findit] Add ability to view last buildbucket response to try job dashboard

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

to

==========
[Findit] Display last build bucket response to try job dashboard

BUG=638690
==========

[lijeffrey, 2016-08-17 19:18:46]

lijeffrey@chromium.org changed reviewers:
+ chanli@chromium.org, stgao@chromium.org

[lijeffrey, 2016-08-17 19:19:52]

Description was changed from

==========
[Findit] Display last build bucket response to try job dashboard

BUG=638690
==========

to

==========
[Findit] Display last build bucket response to try job dashboard

The UI now looks like this:
https://lijeffrey1-dot-findit-for-me.appspot.com/waterfall/try-job-dashboard?...

BUG=638690
==========

[lijeffrey, 2016-08-17 19:20:02]

ptal

[chanli, 2016-08-17 21:17:38]

https://codereview.chromium.org/2259513003/diff/1/appengine/findit/handlers/t...
File appengine/findit/handlers/try_job_dashboard.py (right):

https://codereview.chromium.org/2259513003/diff/1/appengine/findit/handlers/t...
appengine/findit/handlers/try_job_dashboard.py:3: # found in the LICENSE file.
Nit: Add an empty line?

https://codereview.chromium.org/2259513003/diff/1/appengine/findit/handlers/t...
appengine/findit/handlers/try_job_dashboard.py:90:
try_job_data.last_buildbucket_response, sort_keys=True)
I'm not so sure about this, but what will happen if you add indent here?

https://codereview.chromium.org/2259513003/diff/1/appengine/findit/templates/...
File appengine/findit/templates/try_job_dashboard.html (right):

https://codereview.chromium.org/2259513003/diff/1/appengine/findit/templates/...
appengine/findit/templates/try_job_dashboard.html:203: <button
class="show-response" id="completed_response_{{ loop.index }}" data-text="{{
try_job_display_data.last_buildbucket_response }}">show</button>
It seems the tables share most of the columns, so maybe we should pull the
shareable part and reuse it?

[stgao, 2016-08-18 16:52:03]

https://codereview.chromium.org/2259513003/diff/1/appengine/findit/handlers/t...
File appengine/findit/handlers/try_job_dashboard.py (right):

https://codereview.chromium.org/2259513003/diff/1/appengine/findit/handlers/t...
appengine/findit/handlers/try_job_dashboard.py:90:
try_job_data.last_buildbucket_response, sort_keys=True)
Could the conversion from json to string be handled in the html template itself?

https://codereview.chromium.org/2259513003/diff/1/appengine/findit/templates/...
File appengine/findit/templates/try_job_dashboard.html (right):

https://codereview.chromium.org/2259513003/diff/1/appengine/findit/templates/...
appengine/findit/templates/try_job_dashboard.html:43:
$('#buildbucket-response-dialog').text(lastBuildbucketResponseText);
Now the text is a raw string, should we use JSON.stringify(json, null, '  ') to
make it more readable?

[lijeffrey, 2016-08-18 23:56:56]

Description was changed from

==========
[Findit] Display last build bucket response to try job dashboard

The UI now looks like this:
https://lijeffrey1-dot-findit-for-me.appspot.com/waterfall/try-job-dashboard?...

BUG=638690
==========

to

==========
[Findit] Display last build bucket response to try job dashboard

The UI now looks like this:
https://lijeffrey1-dot-findit-for-me.appspot.com/waterfall/try-job-dashboard?...

This CL also fixes a misleading error message when buildbucket reports an error.

BUG=638690, 611866
==========

[lijeffrey, 2016-08-18 23:59:15]

comments addressed, ptal

https://codereview.chromium.org/2259513003/diff/1/appengine/findit/handlers/t...
File appengine/findit/handlers/try_job_dashboard.py (right):

https://codereview.chromium.org/2259513003/diff/1/appengine/findit/handlers/t...
appengine/findit/handlers/try_job_dashboard.py:3: # found in the LICENSE file.
On 2016/08/17 21:17:38, chanli wrote:
> Nit: Add an empty line?

Done.

https://codereview.chromium.org/2259513003/diff/1/appengine/findit/handlers/t...
appengine/findit/handlers/try_job_dashboard.py:90:
try_job_data.last_buildbucket_response, sort_keys=True)
On 2016/08/18 16:52:03, stgao wrote:
> Could the conversion from json to string be handled in the html template
itself?

Unfortunately indent on the template side doesn't seem to do anything. It
appears the existence of a json inside the json here causes problems on the
other side, so instead I will write a function to preprocess the json here first
so it can be properly displayed.

https://codereview.chromium.org/2259513003/diff/1/appengine/findit/templates/...
File appengine/findit/templates/try_job_dashboard.html (right):

https://codereview.chromium.org/2259513003/diff/1/appengine/findit/templates/...
appengine/findit/templates/try_job_dashboard.html:43:
$('#buildbucket-response-dialog').text(lastBuildbucketResponseText);
On 2016/08/18 16:52:03, stgao wrote:
> Now the text is a raw string, should we use JSON.stringify(json, null, '  ')
to
> make it more readable?

Done.

https://codereview.chromium.org/2259513003/diff/1/appengine/findit/templates/...
appengine/findit/templates/try_job_dashboard.html:203: <button
class="show-response" id="completed_response_{{ loop.index }}" data-text="{{
try_job_display_data.last_buildbucket_response }}">show</button>
On 2016/08/17 21:17:38, chanli wrote:
> It seems the tables share most of the columns, so maybe we should pull the
> shareable part and reuse it? 

That would require this entire file be rewritten, I'd rather do that in a
separate CL

[chanli, 2016-08-19 00:01:36]

On 2016/08/18 23:59:15, lijeffrey wrote:
> comments addressed, ptal
> 
>
https://codereview.chromium.org/2259513003/diff/1/appengine/findit/handlers/t...
> File appengine/findit/handlers/try_job_dashboard.py (right):
> 
>
https://codereview.chromium.org/2259513003/diff/1/appengine/findit/handlers/t...
> appengine/findit/handlers/try_job_dashboard.py:3: # found in the LICENSE file.
> On 2016/08/17 21:17:38, chanli wrote:
> > Nit: Add an empty line?
> 
> Done.
> 
>
https://codereview.chromium.org/2259513003/diff/1/appengine/findit/handlers/t...
> appengine/findit/handlers/try_job_dashboard.py:90:
> try_job_data.last_buildbucket_response, sort_keys=True)
> On 2016/08/18 16:52:03, stgao wrote:
> > Could the conversion from json to string be handled in the html template
> itself?
> 
> Unfortunately indent on the template side doesn't seem to do anything. It
> appears the existence of a json inside the json here causes problems on the
> other side, so instead I will write a function to preprocess the json here
first
> so it can be properly displayed.
> 
>
https://codereview.chromium.org/2259513003/diff/1/appengine/findit/templates/...
> File appengine/findit/templates/try_job_dashboard.html (right):
> 
>
https://codereview.chromium.org/2259513003/diff/1/appengine/findit/templates/...
> appengine/findit/templates/try_job_dashboard.html:43:
> $('#buildbucket-response-dialog').text(lastBuildbucketResponseText);
> On 2016/08/18 16:52:03, stgao wrote:
> > Now the text is a raw string, should we use JSON.stringify(json, null, '  ')
> to
> > make it more readable?
> 
> Done.
> 
>
https://codereview.chromium.org/2259513003/diff/1/appengine/findit/templates/...
> appengine/findit/templates/try_job_dashboard.html:203: <button
> class="show-response" id="completed_response_{{ loop.index }}" data-text="{{
> try_job_display_data.last_buildbucket_response }}">show</button>
> On 2016/08/17 21:17:38, chanli wrote:
> > It seems the tables share most of the columns, so maybe we should pull the
> > shareable part and reuse it? 
> 
> That would require this entire file be rewritten, I'd rather do that in a
> separate CL

lgtm

[stgao, 2016-08-19 00:10:00]

lgtm

[lijeffrey, 2016-08-19 00:18:39]

The CQ bit was checked by lijeffrey@chromium.org

[lijeffrey, 2016-08-19 00:18:39]

The patchset sent to the CQ was uploaded after l-g-t-m from chanli@chromium.org,
stgao@chromium.org
Link to the patchset: https://codereview.chromium.org/2259513003/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2016-08-19 00:18:43]

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-08-19 04:32:12]

Description was changed from

==========
[Findit] Display last build bucket response to try job dashboard

The UI now looks like this:
https://lijeffrey1-dot-findit-for-me.appspot.com/waterfall/try-job-dashboard?...

This CL also fixes a misleading error message when buildbucket reports an error.

BUG=638690, 611866
==========

to

==========
[Findit] Display last build bucket response to try job dashboard

The UI now looks like this:
https://lijeffrey1-dot-findit-for-me.appspot.com/waterfall/try-job-dashboard?...

This CL also fixes a misleading error message when buildbucket reports an error.

BUG=638690, 611866

Committed:
https://chromium.googlesource.com/infra/infra/+/ca0df3b15c395d8b5ac01141cdbe4...
==========

[commit-bot: I haz the power, 2016-08-19 04:32:13]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/infra/infra/+/ca0df3b15c395d8b5ac01141cdbe4...