Refactoring the output of bisect script. Switching from plain printing to a combination of:

Enabling logging for bisect script.

Setting default configuration to print any INFO and above logging without formatting to standard out.

Committed: https://crrev.com/82992f77001ee3fecb35ddb2fc38d5ce716e0fcb
Cr-Commit-Position: refs/heads/master@{#301389}

[RobertoCN, 2014-10-19 07:12:40]

robertocn@chromium.org changed reviewers:
+ prasadv@chromium.org, qyearsley@chromium.org

[RobertoCN, 2014-10-19 07:15:07]

robertocn@chromium.org changed reviewers:
+ chrisphan@chromium.org

[RobertoCN, 2014-10-19 07:15:08]

Thoughts?

[qyearsley, 2014-10-20 18:26:01]

Looks good, I like the fact that "warn" and "info" can be differentiated now.

https://codereview.chromium.org/660393002/diff/40001/tools/auto_bisect/bisect...
File tools/auto_bisect/bisect_perf_regression.py (right):

https://codereview.chromium.org/660393002/diff/40001/tools/auto_bisect/bisect...
tools/auto_bisect/bisect_perf_regression.py:62: 
I think this blank line isn't necessary according to the style guide; the style
just says something like "two blank lines between top level functions or
classes".

https://codereview.chromium.org/660393002/diff/40001/tools/auto_bisect/bisect...
tools/auto_bisect/bisect_perf_regression.py:3202: logging.info('')
This one I'm not really sure about, maybe it should be a print. Here, we're
expecting that the user is running the bisect script locally and they set
target_platform to "cros". We're expecting this message to be printed to the
console, followed by the sudo password prompt.

Note: In the past year, virtually nobody has done this. Everything CrOS related
is basically untested.

https://codereview.chromium.org/660393002/diff/40001/tools/auto_bisect/bisect...
tools/auto_bisect/bisect_perf_regression.py:3270: loggingFormat = '%(message)s'
"logging_format" is probably better here, even though camelCase is used inside
the logging module.

https://codereview.chromium.org/660393002/diff/40001/tools/auto_bisect/bisect...
tools/auto_bisect/bisect_perf_regression.py:3327: logging.warn('Error: %s',
e.message)
We definitely want to make sure that this gets printed to stdout, because what's
printed here is part of the Results.

(RuntimeError is thrown for fatal errors, and whenever it's thrown, we skip to
the Results step and print an error message.)

[RobertoCN, 2014-10-20 21:01:03]

https://codereview.chromium.org/660393002/diff/40001/tools/auto_bisect/bisect...
File tools/auto_bisect/bisect_perf_regression.py (right):

https://codereview.chromium.org/660393002/diff/40001/tools/auto_bisect/bisect...
tools/auto_bisect/bisect_perf_regression.py:62: 
On 2014/10/20 18:26:00, qyearsley wrote:
> I think this blank line isn't necessary according to the style guide; the
style
> just says something like "two blank lines between top level functions or
> classes".

Done.

https://codereview.chromium.org/660393002/diff/40001/tools/auto_bisect/bisect...
tools/auto_bisect/bisect_perf_regression.py:3202: logging.info('')
On 2014/10/20 18:26:00, qyearsley wrote:
> This one I'm not really sure about, maybe it should be a print. Here, we're
> expecting that the user is running the bisect script locally and they set
> target_platform to "cros". We're expecting this message to be printed to the
> console, followed by the sudo password prompt.
> 
> Note: In the past year, virtually nobody has done this. Everything CrOS
related
> is basically untested.

Done.

https://codereview.chromium.org/660393002/diff/40001/tools/auto_bisect/bisect...
tools/auto_bisect/bisect_perf_regression.py:3270: loggingFormat = '%(message)s'
On 2014/10/20 18:26:00, qyearsley wrote:
> "logging_format" is probably better here, even though camelCase is used inside
> the logging module.

Done.

https://codereview.chromium.org/660393002/diff/40001/tools/auto_bisect/bisect...
tools/auto_bisect/bisect_perf_regression.py:3327: logging.warn('Error: %s',
e.message)
On 2014/10/20 18:26:00, qyearsley wrote:
> We definitely want to make sure that this gets printed to stdout, because
what's
> printed here is part of the Results.
> 
> (RuntimeError is thrown for fatal errors, and whenever it's thrown, we skip to
> the Results step and print an error message.)

How about we print it and also log it?
Also how is this instead of using the % op?
  print 'Error: ', e.message

[RobertoCN, 2014-10-21 20:33:38]

Can I commit this now?

[RobertoCN, 2014-10-21 20:37:42]

robertocn@chromium.org changed reviewers:
+ sergiyb@chromium.org

[qyearsley, 2014-10-21 21:51:27]

Yep, I didn't have any more comments, should be good to commit.

https://codereview.chromium.org/660393002/diff/40001/tools/auto_bisect/bisect...
File tools/auto_bisect/bisect_perf_regression.py (right):

https://codereview.chromium.org/660393002/diff/40001/tools/auto_bisect/bisect...
tools/auto_bisect/bisect_perf_regression.py:3327: logging.warn('Error: %s',
e.message)
On 2014/10/20 21:01:03, robertocn wrote:
> On 2014/10/20 18:26:00, qyearsley wrote:
> > We definitely want to make sure that this gets printed to stdout, because
> what's
> > printed here is part of the Results.
> > 
> > (RuntimeError is thrown for fatal errors, and whenever it's thrown, we skip
to
> > the Results step and print an error message.)
> 
> How about we print it and also log it?
> Also how is this instead of using the % op?
>   print 'Error: ', e.message

Printing as well as logging would probably be fine, although we want to be sure
that only one copy goes to stdout and thus into the Results step text.

[qyearsley, 2014-10-21 21:51:42]

On 2014/10/21 21:51:27, qyearsley wrote:
> Yep, I didn't have any more comments, should be good to commit.
> 
>
https://codereview.chromium.org/660393002/diff/40001/tools/auto_bisect/bisect...
> File tools/auto_bisect/bisect_perf_regression.py (right):
> 
>
https://codereview.chromium.org/660393002/diff/40001/tools/auto_bisect/bisect...
> tools/auto_bisect/bisect_perf_regression.py:3327: logging.warn('Error: %s',
> e.message)
> On 2014/10/20 21:01:03, robertocn wrote:
> > On 2014/10/20 18:26:00, qyearsley wrote:
> > > We definitely want to make sure that this gets printed to stdout, because
> > what's
> > > printed here is part of the Results.
> > > 
> > > (RuntimeError is thrown for fatal errors, and whenever it's thrown, we
skip
> to
> > > the Results step and print an error message.)
> > 
> > How about we print it and also log it?
> > Also how is this instead of using the % op?
> >   print 'Error: ', e.message
> 
> Printing as well as logging would probably be fine, although we want to be
sure
> that only one copy goes to stdout and thus into the Results step text.

lgtm

[prasadv, 2014-10-21 23:33:21]

lgtm

[RobertoCN, 2014-10-25 01:00:14]

The CQ bit was checked by robertocn@chromium.org

[RobertoCN, 2014-10-25 01:00:39]

The CQ bit was unchecked by robertocn@chromium.org

[Sergiy Byelozyorov, 2014-10-27 12:38:31]

Can this be commit now please?

[qyearsley, 2014-10-27 16:35:44]

The CQ bit was checked by qyearsley@chromium.org

[qyearsley, 2014-10-27 16:36:35]

The CQ bit was unchecked by qyearsley@chromium.org

[qyearsley, 2014-10-27 16:36:40]

The CQ bit was checked by qyearsley@chromium.org

[commit-bot: I haz the power, 2014-10-27 16:37:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/660393002/100001

[qyearsley, 2014-10-27 16:37:52]

On 2014/10/27 12:38:31, Sergiy Byelozyorov wrote:
> Can this be commit now please?

I remember Roberto was about to commit this but thought it would be wiser not to
commit at the end of Friday when we wouldn't be around to see the following
bisect jobs.

I wonder whether I can commit it by checking the box?

[commit-bot: I haz the power, 2014-10-27 17:21:56]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2014-10-27 17:22:54]

Patchset 6 (id:??) landed as
https://crrev.com/82992f77001ee3fecb35ddb2fc38d5ce716e0fcb
Cr-Commit-Position: refs/heads/master@{#301389}