[Findit] Add failure type to dashboard.

[Findit] Add failure type to dashboard.

BUG=605649
Committed: https://chromium.googlesource.com/infra/infra/+/2cf3601bfe1e1f3ad817b25dd8798bfa4cf625f8

[stgao, 2016-04-21 19:02:29]

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

[stgao, 2016-04-21 19:02:29]

ptal

[lijeffrey1, 2016-04-21 19:40:05]

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

[lijeffrey1, 2016-04-21 19:40:06]

https://codereview.chromium.org/1912483004/diff/1/appengine/findit/common/wat...
File appengine/findit/common/waterfall/failure_type.py (right):

https://codereview.chromium.org/1912483004/diff/1/appengine/findit/common/wat...
appengine/findit/common/waterfall/failure_type.py:18: return
description.get(failure_type, 'UNKNOWN')
Should this default to 'unknown' or description[UNKNOWN]?

i.e. return description.get(failure_type, description[UNKNOWN])?

[chanli, 2016-04-21 20:10:45]

https://codereview.chromium.org/1912483004/diff/1/appengine/findit/common/wat...
File appengine/findit/common/waterfall/failure_type.py (right):

https://codereview.chromium.org/1912483004/diff/1/appengine/findit/common/wat...
appengine/findit/common/waterfall/failure_type.py:18: return
description.get(failure_type, 'UNKNOWN')
On 2016/04/21 19:40:05, lijeffrey1 wrote:
> Should this default to 'unknown' or description[UNKNOWN]?
> 
> i.e. return description.get(failure_type, description[UNKNOWN])?

+1

[chanli, 2016-04-21 20:12:13]

On 2016/04/21 20:10:45, chanli wrote:
>
https://codereview.chromium.org/1912483004/diff/1/appengine/findit/common/wat...
> File appengine/findit/common/waterfall/failure_type.py (right):
> 
>
https://codereview.chromium.org/1912483004/diff/1/appengine/findit/common/wat...
> appengine/findit/common/waterfall/failure_type.py:18: return
> description.get(failure_type, 'UNKNOWN')
> On 2016/04/21 19:40:05, lijeffrey1 wrote:
> > Should this default to 'unknown' or description[UNKNOWN]?
> > 
> > i.e. return description.get(failure_type, description[UNKNOWN])?
> 
> +1

I filed a bug for this: 605649. Please add the bug to this cl.

[stgao, 2016-04-21 20:38:47]

Description was changed from

==========
[Findit] Add failure type to dashboard.

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

to

==========
[Findit] Add failure type to dashboard.

BUG=605649
==========

[stgao, 2016-04-21 20:40:47]

https://codereview.chromium.org/1912483004/diff/1/appengine/findit/common/wat...
File appengine/findit/common/waterfall/failure_type.py (right):

https://codereview.chromium.org/1912483004/diff/1/appengine/findit/common/wat...
appengine/findit/common/waterfall/failure_type.py:18: return
description.get(failure_type, 'UNKNOWN')
On 2016/04/21 20:10:45, chanli wrote:
> On 2016/04/21 19:40:05, lijeffrey1 wrote:
> > Should this default to 'unknown' or description[UNKNOWN]?
> > 
> > i.e. return description.get(failure_type, description[UNKNOWN])?
> 
> +1

We could, but my thought is if the failure type is not in the description list,
it's a bug. And the upper case 'UNKNOWN' could distinguish that from the normal
case 'unknown'.

[lijeffrey, 2016-04-21 20:50:43]

https://codereview.chromium.org/1912483004/diff/1/appengine/findit/common/wat...
File appengine/findit/common/waterfall/failure_type.py (right):

https://codereview.chromium.org/1912483004/diff/1/appengine/findit/common/wat...
appengine/findit/common/waterfall/failure_type.py:18: return
description.get(failure_type, 'UNKNOWN')
On 2016/04/21 20:40:47, stgao wrote:
> On 2016/04/21 20:10:45, chanli wrote:
> > On 2016/04/21 19:40:05, lijeffrey1 wrote:
> > > Should this default to 'unknown' or description[UNKNOWN]?
> > > 
> > > i.e. return description.get(failure_type, description[UNKNOWN])?
> > 
> > +1
> 
> We could, but my thought is if the failure type is not in the description
list,
> it's a bug. And the upper case 'UNKNOWN' could distinguish that from the
normal
> case 'unknown'.

this could get confusing, how about 'no_failure_description' or
'unaccounted_failure_type' or something similar? Because we usually use .lower()
when doing string comparisons if we were to query for such failure
types/descriptions it's easy to overlook the fact that some of them are
upper/lower case and which one means which

[stgao, 2016-04-21 22:42:35]

https://codereview.chromium.org/1912483004/diff/1/appengine/findit/common/wat...
File appengine/findit/common/waterfall/failure_type.py (right):

https://codereview.chromium.org/1912483004/diff/1/appengine/findit/common/wat...
appengine/findit/common/waterfall/failure_type.py:18: return
description.get(failure_type, 'UNKNOWN')
On 2016/04/21 20:50:43, lijeffrey wrote:
> On 2016/04/21 20:40:47, stgao wrote:
> > On 2016/04/21 20:10:45, chanli wrote:
> > > On 2016/04/21 19:40:05, lijeffrey1 wrote:
> > > > Should this default to 'unknown' or description[UNKNOWN]?
> > > > 
> > > > i.e. return description.get(failure_type, description[UNKNOWN])?
> > > 
> > > +1
> > 
> > We could, but my thought is if the failure type is not in the description
> list,
> > it's a bug. And the upper case 'UNKNOWN' could distinguish that from the
> normal
> > case 'unknown'.
> 
> this could get confusing, how about 'no_failure_description' or
> 'unaccounted_failure_type' or something similar? Because we usually use
.lower()
> when doing string comparisons if we were to query for such failure
> types/descriptions it's easy to overlook the fact that some of them are
> upper/lower case and which one means which

OK. Changed to "No Description for %s" % failure_type.

[lijeffrey, 2016-04-21 23:54:10]

lgtm with a tiny nit

https://codereview.chromium.org/1912483004/diff/20001/appengine/findit/common...
File appengine/findit/common/waterfall/failure_type.py (right):

https://codereview.chromium.org/1912483004/diff/20001/appengine/findit/common...
appengine/findit/common/waterfall/failure_type.py:18: return
description.get(failure_type, 'No Description for %s' % failure_type)
nit: lower case description

[stgao, 2016-04-22 00:02:56]

https://codereview.chromium.org/1912483004/diff/20001/appengine/findit/common...
File appengine/findit/common/waterfall/failure_type.py (right):

https://codereview.chromium.org/1912483004/diff/20001/appengine/findit/common...
appengine/findit/common/waterfall/failure_type.py:18: return
description.get(failure_type, 'No Description for %s' % failure_type)
On 2016/04/21 23:54:10, lijeffrey wrote:
> nit: lower case description

oops

[stgao, 2016-04-22 00:03:03]

The CQ bit was checked by stgao@chromium.org

[stgao, 2016-04-22 00:03:04]

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

[commit-bot: I haz the power, 2016-04-22 00:03:13]

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

[commit-bot: I haz the power, 2016-04-22 00:07:11]

Description was changed from

==========
[Findit] Add failure type to dashboard.

BUG=605649
==========

to

==========
[Findit] Add failure type to dashboard.

BUG=605649

Committed:
https://chromium.googlesource.com/infra/infra/+/2cf3601bfe1e1f3ad817b25dd8798...
==========

[commit-bot: I haz the power, 2016-04-22 00:07:12]

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