Actually handle booleans properly in gatekeeper.

Actually handle booleans properly in gatekeeper.

Note: there is a legacy artifact that 'forgive_all' is still a string. We may want to add backward compatibility here, but this CL doesn't touch that for now.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299336

[ghost stip (do not use), 2016-03-11 23:07:39]

Description was changed from

==========
Actually handle booleans properly in gatekeeper.

Note: there is a legacy artifact that 'forgive_all' is still a string. We may
want to add backward compatibility here, but this CL doesn't touch that for now.
==========

to

==========
Actually handle booleans properly in gatekeeper.

Note: there is a legacy artifact that 'forgive_all' is still a string. We may
want to add backward compatibility here, but this CL doesn't touch that for now.
==========

[ghost stip (do not use), 2016-03-11 23:07:39]

stip@chromium.org changed reviewers:
+ dpranke@chromium.org

[ghost stip (do not use), 2016-03-11 23:08:19]

ptal

[Dirk Pranke, 2016-03-11 23:59:07]

I think this is mostly okay, but you need to document 'close_tree' and
'respect_build_status' in the comments at the top; otherwise, I'm not actually
sure what the defaults are or what these things are supposed to do.

[Dirk Pranke, 2016-03-16 01:54:06]

ping ... you still need to land this so I can land the config changes ...

[ghost stip (do not use), 2016-03-16 07:30:34]

sorry about the delay! ptal

[Dirk Pranke, 2016-03-16 22:29:52]

lgtm w/ an adequately satisfying answer to the question :).

https://codereview.chromium.org/1789693003/diff/20001/scripts/slave/gatekeepe...
File scripts/slave/gatekeeper_ng_config.py (right):

https://codereview.chromium.org/1789693003/diff/20001/scripts/slave/gatekeepe...
scripts/slave/gatekeeper_ng_config.py:67: False.
This seems kinda confusing (the concept, not the docs): why would we have
situations where all steps passed but the build failed, or a step failed but the
build passed, i.e., why is build_status != all(step['status'] == SUCCESS for
step in steps)?

[ghost stip (do not use), 2016-03-16 22:56:16]

https://codereview.chromium.org/1789693003/diff/20001/scripts/slave/gatekeepe...
File scripts/slave/gatekeeper_ng_config.py (right):

https://codereview.chromium.org/1789693003/diff/20001/scripts/slave/gatekeepe...
scripts/slave/gatekeeper_ng_config.py:67: False.
On 2016/03/16 22:29:52, Dirk Pranke wrote:
> This seems kinda confusing (the concept, not the docs): why would we have
> situations where all steps passed but the build failed, or a step failed but
the
> build passed, i.e., why is build_status != all(step['status'] == SUCCESS for
> step in steps)?

it's the return code of the build. think of what you see on the buildbot
waterfall -- you see a single 'failed' or 'passed,' which is actually determined
by arbitrary internal buildbot logic. with recipes this becomes less relevant
over time. I don't think anyone has used it, and it would be interesting to
remove it in the future.

[Dirk Pranke, 2016-03-16 23:06:25]

On 2016/03/16 22:56:16, stip wrote:
>
https://codereview.chromium.org/1789693003/diff/20001/scripts/slave/gatekeepe...
> File scripts/slave/gatekeeper_ng_config.py (right):
> 
>
https://codereview.chromium.org/1789693003/diff/20001/scripts/slave/gatekeepe...
> scripts/slave/gatekeeper_ng_config.py:67: False.
> On 2016/03/16 22:29:52, Dirk Pranke wrote:
> > This seems kinda confusing (the concept, not the docs): why would we have
> > situations where all steps passed but the build failed, or a step failed but
> the
> > build passed, i.e., why is build_status != all(step['status'] == SUCCESS for
> > step in steps)?
> 
> it's the return code of the build. think of what you see on the buildbot
> waterfall -- you see a single 'failed' or 'passed,' which is actually
determined
> by arbitrary internal buildbot logic. with recipes this becomes less relevant
> over time. I don't think anyone has used it, and it would be interesting to
> remove it in the future.

Yeah, I understand that buildbot has the logic, I would just hope we wouldn't
use it
or encourage it ...

[ghost stip (do not use), 2016-03-16 23:53:00]

The CQ bit was checked by stip@chromium.org

[commit-bot: I haz the power, 2016-03-16 23:53:04]

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

[commit-bot: I haz the power, 2016-03-16 23:57:39]

Description was changed from

==========
Actually handle booleans properly in gatekeeper.

Note: there is a legacy artifact that 'forgive_all' is still a string. We may
want to add backward compatibility here, but this CL doesn't touch that for now.
==========

to

==========
Actually handle booleans properly in gatekeeper.

Note: there is a legacy artifact that 'forgive_all' is still a string. We may
want to add backward compatibility here, but this CL doesn't touch that for now.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299336
==========

[commit-bot: I haz the power, 2016-03-16 23:57:40]

Committed patchset #2 (id:20001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=299336