Updating /analyze script to use logdog, improve code style

Updating /analyze script to use logdog, improve code style

When the /analyze builder switched over to logdog the scripts for
retrieving the latest warnings and generating a report on the new ones
broke. This change updates the script to use the logdog tool to download
the results. It also moves the code to be much more inline with Google's
Python coding standards.

R=chengx@chromium.org
BUG=427616
Review-Url: https://codereview.chromium.org/2793453002
Cr-Commit-Position: refs/heads/master@{#461552}
Committed: https://chromium.googlesource.com/chromium/src/+/e8b33cd8b431de2e385e174c7ee96e3aedd971ee

[brucedawson, 2017-03-31 00:05:17]

The CQ bit was checked by brucedawson@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 00:06:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[brucedawson, 2017-03-31 00:07:39]

This change fixes the style of this script and fixes it to work with logdog.
Maybe I should have done two separate changes. I put comments in to the two
parts of the code that have changed in non-style ways.

PTAL.

https://codereview.chromium.org/2793453002/diff/1/tools/win/new_analyze_warni...
File tools/win/new_analyze_warnings/retrieve_warnings.py (right):

https://codereview.chromium.org/2793453002/diff/1/tools/win/new_analyze_warni...
tools/win/new_analyze_warnings/retrieve_warnings.py:64:
'%d/+/recipes/steps/compile/0/stdout' % build_number
This line changes, to generate a logdog compatible descriptor.

https://codereview.chromium.org/2793453002/diff/1/tools/win/new_analyze_warni...
tools/win/new_analyze_warnings/retrieve_warnings.py:81: print 'Retrieving raw
build results. Please wait.'
This whole section changes. The data is downloaded differently, the fixing up of
escaped HTML characters is no longer needed, and "status: SUCCESS" is no longer
in the results so it is no longer looked for.

[commit-bot: I haz the power, 2017-03-31 00:38:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-31 00:38:32]

Dry run: This issue passed the CQ dry run.

[chengx, 2017-04-03 21:32:13]

LGTM with a few nits w.r.t the variable names. 

I agree it may be better to at least use two patch sets, if not using two CLs.
However, given that this change is not complex, it should be fine using just one
patch set.

https://codereview.chromium.org/2793453002/diff/1/tools/win/new_analyze_warni...
File tools/win/new_analyze_warnings/retrieve_warnings.py (right):

https://codereview.chromium.org/2793453002/diff/1/tools/win/new_analyze_warni...
tools/win/new_analyze_warnings/retrieve_warnings.py:24: base_URL =
'http://build.chromium.org/p/chromium.fyi/builders/' + \
nit: I think "base_URL" should be all lowercase. This also applies to all other
variable names containing "URL".
https://google.github.io/styleguide/cppguide.html#Variable_Names

https://codereview.chromium.org/2793453002/diff/1/tools/win/new_analyze_warni...
tools/win/new_analyze_warnings/retrieve_warnings.py:91: sourcePath =
r'e:\b\build\slave\chromium_windows_analyze\build\src'
nit: sourcePath --> source_path

https://codereview.chromium.org/2793453002/diff/1/tools/win/new_analyze_warni...
tools/win/new_analyze_warnings/retrieve_warnings.py:92: destPath =
os.path.join(os.environ['ANALYZE_REPO'], 'src')
ditto. dest_path?

[brucedawson, 2017-04-03 21:54:50]

Thanks for the feedback and for finding the variables that I missed. Landing.

https://codereview.chromium.org/2793453002/diff/1/tools/win/new_analyze_warni...
File tools/win/new_analyze_warnings/retrieve_warnings.py (right):

https://codereview.chromium.org/2793453002/diff/1/tools/win/new_analyze_warni...
tools/win/new_analyze_warnings/retrieve_warnings.py:24: base_URL =
'http://build.chromium.org/p/chromium.fyi/builders/' + \
On 2017/04/03 21:32:13, chengx wrote:
> nit: I think "base_URL" should be all lowercase. This also applies to all
other
> variable names containing "URL".
> https://google.github.io/styleguide/cppguide.html#Variable_Names

I looked at the Python style guide and did a few stack overflow searches to
figure out whether it should be URL or url and it sounds like it depends on what
is being named and I *think* you are right, but it is amazingly hard to be sure.
The only mention of acronyms I could find in the linked cppguide was for
function names where Url is preferred, but that doesn't seem applicable.

Done.

https://codereview.chromium.org/2793453002/diff/1/tools/win/new_analyze_warni...
tools/win/new_analyze_warnings/retrieve_warnings.py:91: sourcePath =
r'e:\b\build\slave\chromium_windows_analyze\build\src'
On 2017/04/03 21:32:13, chengx wrote:
> nit: sourcePath --> source_path

Done.

https://codereview.chromium.org/2793453002/diff/1/tools/win/new_analyze_warni...
tools/win/new_analyze_warnings/retrieve_warnings.py:92: destPath =
os.path.join(os.environ['ANALYZE_REPO'], 'src')
On 2017/04/03 21:32:13, chengx wrote:
> ditto. dest_path?

Done.

[brucedawson, 2017-04-03 21:54:54]

The CQ bit was checked by brucedawson@chromium.org

[brucedawson, 2017-04-03 21:54:54]

The patchset sent to the CQ was uploaded after l-g-t-m from chengx@chromium.org
Link to the patchset: https://codereview.chromium.org/2793453002/#ps20001
(title: "Rename some more variables")

[commit-bot: I haz the power, 2017-04-03 21:55:31]

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, 2017-04-03 22:07:21]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1491256494549920,
"parent_rev": "fe52253d07c2dfed2fc3cb60e6bc2baf5e386327", "commit_rev":
"e8b33cd8b431de2e385e174c7ee96e3aedd971ee"}

[commit-bot: I haz the power, 2017-04-03 22:08:34]

Description was changed from

==========
Updating /analyze script to use logdog, improve code style

When the /analyze builder switched over to logdog the scripts for
retrieving the latest warnings and generating a report on the new ones
broke. This change updates the script to use the logdog tool to download
the results. It also moves the code to be much more inline with Google's
Python coding standards.

R=chengx@chromium.org
BUG=427616
==========

to

==========
Updating /analyze script to use logdog, improve code style

When the /analyze builder switched over to logdog the scripts for
retrieving the latest warnings and generating a report on the new ones
broke. This change updates the script to use the logdog tool to download
the results. It also moves the code to be much more inline with Google's
Python coding standards.

R=chengx@chromium.org
BUG=427616

Review-Url: https://codereview.chromium.org/2793453002
Cr-Commit-Position: refs/heads/master@{#461552}
Committed:
https://chromium.googlesource.com/chromium/src/+/e8b33cd8b431de2e385e174c7ee9...
==========

[commit-bot: I haz the power, 2017-04-03 22:08:37]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/e8b33cd8b431de2e385e174c7ee9...