[Findit] Adding more try job error detection

[Findit] Adding more try job error detection

A few false negatives under completed try jobs actually had errors occur during execution, but not captured. This change checks the buildbucket response itself too for more hints as to what happened.

BUG=609420
Committed: https://chromium.googlesource.com/infra/infra/+/1eb52debda33d5c79401e4f9a3b0e5924ef5675f

[lijeffrey, 2016-05-05 10:05:12]

Description was changed from

==========
[Findit] Adding more try job error detection

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

to

==========
[Findit] Adding more try job error detection

A few false negatives under completed try jobs actually had errors occur during
execution, but not captured. This change takes care of the case where a report
is returned but the results are empty.

BUG=609420
==========

[lijeffrey, 2016-05-05 10:05:12]

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

[lijeffrey, 2016-05-05 10:05:43]

ptal

[lijeffrey, 2016-05-06 01:40:34]

Description was changed from

==========
[Findit] Adding more try job error detection

A few false negatives under completed try jobs actually had errors occur during
execution, but not captured. This change takes care of the case where a report
is returned but the results are empty.

BUG=609420
==========

to

==========
[Findit] Adding more try job error detection

A few false negatives under completed try jobs actually had errors occur during
execution, but not captured. This change checks the buildbucket response itself
too for more hints as to what happened.

BUG=609420
==========

[lijeffrey, 2016-05-06 01:43:30]

Added some handling for more cases by checking the buildbucket response itself
too, ptal

[chanli, 2016-05-06 23:17:47]

https://codereview.chromium.org/1948093003/diff/40001/appengine/findit/waterf...
File appengine/findit/waterfall/monitor_try_job_pipeline.py (right):

https://codereview.chromium.org/1948093003/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/monitor_try_job_pipeline.py:78: 'message': 'Compile
failed unexpectedly.',
I think 'BUILD FAILURE' or 'INFRA_FAILURE' are already very good messages. Do
you really need to separate them as a special cases?

https://codereview.chromium.org/1948093003/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/monitor_try_job_pipeline.py:81:
try_job_error.INFRA_FAILURE
If it's because a bad revision which was fixed within the same regression range,
I don't think it should be grouped as INFRA_FAILURE

https://codereview.chromium.org/1948093003/diff/40001/appengine/findit/waterf...
File appengine/findit/waterfall/test/monitor_try_job_pipeline_test.py (right):

https://codereview.chromium.org/1948093003/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/test/monitor_try_job_pipeline_test.py:472: 
nit: empty lines

[lijeffrey, 2016-05-07 00:12:45]

https://codereview.chromium.org/1948093003/diff/40001/appengine/findit/waterf...
File appengine/findit/waterfall/monitor_try_job_pipeline.py (right):

https://codereview.chromium.org/1948093003/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/monitor_try_job_pipeline.py:78: 'message': 'Compile
failed unexpectedly.',
On 2016/05/06 23:17:46, chanli wrote:
> I think 'BUILD FAILURE' or 'INFRA_FAILURE' are already very good messages. Do
> you really need to separate them as a special cases?

This is for consistency, the other errors are more decriptive such as
"Buildbucket reported an error." (which can be a variety of errors) and "No
result report was found" which is technically a placeholder for some bigger
error(s) being responsible that we hasn't been determined yet.

https://codereview.chromium.org/1948093003/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/monitor_try_job_pipeline.py:81:
try_job_error.INFRA_FAILURE
On 2016/05/06 23:17:46, chanli wrote:
> If it's because a bad revision which was fixed within the same regression
range,
> I don't think it should be grouped as INFRA_FAILURE

We don't have a way of knowing that it's due to a bad revision though. It can
also be disk full or some other uncaught exception.

https://codereview.chromium.org/1948093003/diff/40001/appengine/findit/waterf...
File appengine/findit/waterfall/test/monitor_try_job_pipeline_test.py (right):

https://codereview.chromium.org/1948093003/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/test/monitor_try_job_pipeline_test.py:472: 
On 2016/05/06 23:17:46, chanli wrote:
> nit: empty lines

Done.

[chanli, 2016-05-07 00:14:20]

On 2016/05/07 00:12:45, lijeffrey wrote:
>
https://codereview.chromium.org/1948093003/diff/40001/appengine/findit/waterf...
> File appengine/findit/waterfall/monitor_try_job_pipeline.py (right):
> 
>
https://codereview.chromium.org/1948093003/diff/40001/appengine/findit/waterf...
> appengine/findit/waterfall/monitor_try_job_pipeline.py:78: 'message': 'Compile
> failed unexpectedly.',
> On 2016/05/06 23:17:46, chanli wrote:
> > I think 'BUILD FAILURE' or 'INFRA_FAILURE' are already very good messages.
Do
> > you really need to separate them as a special cases?
> 
> This is for consistency, the other errors are more decriptive such as
> "Buildbucket reported an error." (which can be a variety of errors) and "No
> result report was found" which is technically a placeholder for some bigger
> error(s) being responsible that we hasn't been determined yet.
> 
>
https://codereview.chromium.org/1948093003/diff/40001/appengine/findit/waterf...
> appengine/findit/waterfall/monitor_try_job_pipeline.py:81:
> try_job_error.INFRA_FAILURE
> On 2016/05/06 23:17:46, chanli wrote:
> > If it's because a bad revision which was fixed within the same regression
> range,
> > I don't think it should be grouped as INFRA_FAILURE
> 
> We don't have a way of knowing that it's due to a bad revision though. It can
> also be disk full or some other uncaught exception.
> 
>
https://codereview.chromium.org/1948093003/diff/40001/appengine/findit/waterf...
> File appengine/findit/waterfall/test/monitor_try_job_pipeline_test.py (right):
> 
>
https://codereview.chromium.org/1948093003/diff/40001/appengine/findit/waterf...
> appengine/findit/waterfall/test/monitor_try_job_pipeline_test.py:472: 
> On 2016/05/06 23:17:46, chanli wrote:
> > nit: empty lines
> 
> Done.

lgtm

[lijeffrey, 2016-05-07 00:15:05]

The CQ bit was checked by lijeffrey@chromium.org

[commit-bot: I haz the power, 2016-05-07 00:15:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1948093003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1948093003/60001

[commit-bot: I haz the power, 2016-05-07 00:20:08]

Description was changed from

==========
[Findit] Adding more try job error detection

A few false negatives under completed try jobs actually had errors occur during
execution, but not captured. This change checks the buildbucket response itself
too for more hints as to what happened.

BUG=609420
==========

to

==========
[Findit] Adding more try job error detection

A few false negatives under completed try jobs actually had errors occur during
execution, but not captured. This change checks the buildbucket response itself
too for more hints as to what happened.

BUG=609420

Committed:
https://chromium.googlesource.com/infra/infra/+/1eb52debda33d5c79401e4f9a3b0e...
==========

[commit-bot: I haz the power, 2016-05-07 00:20:09]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/infra/infra/+/1eb52debda33d5c79401e4f9a3b0e...