sandwich: Makes pull_sandwich_metrics.py a sub-command.

sandwich: Makes pull_sandwich_metrics.py a sub-command.

BUG=582080
Committed: https://crrev.com/04c9261d40089b8454379f35df33e908b2ddbbb0
Cr-Commit-Position: refs/heads/master@{#378467}

[gabadie, 2016-02-25 15:15:34]

gabadie@chromium.org changed reviewers:
+ lizeb@chromium.org, mattcary@chromium.org, pasko@chromium.org

[gabadie, 2016-02-25 15:15:35]

Hi guys,

This change continue the sanwich refactor by moving the pull_sandwich_metrics.py
main() into a sandwich.py sub-command.

[Benoit L, 2016-02-25 16:02:13]

lgtm, with a nit and a question.

https://codereview.chromium.org/1726403005/diff/1/tools/android/loading/pull_...
File tools/android/loading/pull_sandwich_metrics.py (right):

https://codereview.chromium.org/1726403005/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:10: import argparse
unnecessary import.

https://codereview.chromium.org/1726403005/diff/1/tools/android/loading/sandw...
File tools/android/loading/sandwich.py (right):

https://codereview.chromium.org/1726403005/diff/1/tools/android/loading/sandw...
tools/android/loading/sandwich.py:436: with open(args.metrics_csv_path, 'w') as
csv_file:
Any reason to choose CSV rather than JSON?

[gabadie, 2016-02-25 16:55:06]

Thanks benoit.

https://codereview.chromium.org/1726403005/diff/1/tools/android/loading/pull_...
File tools/android/loading/pull_sandwich_metrics.py (right):

https://codereview.chromium.org/1726403005/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:10: import argparse
On 2016/02/25 16:02:12, Benoit L wrote:
> unnecessary import.

My bad. Done.

https://codereview.chromium.org/1726403005/diff/1/tools/android/loading/sandw...
File tools/android/loading/sandwich.py (right):

https://codereview.chromium.org/1726403005/diff/1/tools/android/loading/sandw...
tools/android/loading/sandwich.py:436: with open(args.metrics_csv_path, 'w') as
csv_file:
On 2016/02/25 16:02:13, Benoit L wrote:
> Any reason to choose CSV rather than JSON?

The main motivation of using CSV here is to be able to open it directly with a
tabler and make some iteration quickly to fetch some usefull stats from it. This
way we are independent of the ipython notebook, handy for debuging purposes.

[mattcary, 2016-02-26 09:34:58]

lgtm

[pasko, 2016-02-26 14:07:14]

lgtm with small nits

https://codereview.chromium.org/1726403005/diff/20001/tools/android/loading/s...
File tools/android/loading/sandwich.py (right):

https://codereview.chromium.org/1726403005/diff/20001/tools/android/loading/s...
tools/android/loading/sandwich.py:310: help='Web page replay archive to patch.')
nit: maybe better to say: to 'generate or patch'?

https://codereview.chromium.org/1726403005/diff/20001/tools/android/loading/s...
tools/android/loading/sandwich.py:363: help='Pulls metrics in CSV from a run\'s
loading trace.')
Extracts metrics from a loading trace and saves as CSV.

https://codereview.chromium.org/1726403005/diff/20001/tools/android/loading/s...
tools/android/loading/sandwich.py:364:
create_cache_parser.add_argument('--traces-directory', required=True,
nit: trace-directory

[gabadie, 2016-03-01 10:20:27]

Landing once blocking dependency landed.

https://codereview.chromium.org/1726403005/diff/20001/tools/android/loading/s...
File tools/android/loading/sandwich.py (right):

https://codereview.chromium.org/1726403005/diff/20001/tools/android/loading/s...
tools/android/loading/sandwich.py:310: help='Web page replay archive to patch.')
On 2016/02/26 14:07:13, pasko wrote:
> nit: maybe better to say: to 'generate or patch'?

No because for this sub command, we are only patching.

https://codereview.chromium.org/1726403005/diff/20001/tools/android/loading/s...
tools/android/loading/sandwich.py:363: help='Pulls metrics in CSV from a run\'s
loading trace.')
On 2016/02/26 14:07:13, pasko wrote:
> Extracts metrics from a loading trace and saves as CSV.

Done.

https://codereview.chromium.org/1726403005/diff/20001/tools/android/loading/s...
tools/android/loading/sandwich.py:364:
create_cache_parser.add_argument('--traces-directory', required=True,
On 2016/02/26 14:07:14, pasko wrote:
> nit: trace-directory

Done.

[pasko, 2016-03-01 15:42:51]

still lgtm, intention to land ack

[gabadie, 2016-03-01 16:40:33]

The CQ bit was checked by gabadie@chromium.org

[gabadie, 2016-03-01 16:40:34]

The patchset sent to the CQ was uploaded after l-g-t-m from lizeb@chromium.org,
mattcary@chromium.org
Link to the patchset: https://codereview.chromium.org/1726403005/#ps40001
(title: "Addresses pasko's nits")

[commit-bot: I haz the power, 2016-03-01 16:40:47]

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

[commit-bot: I haz the power, 2016-03-01 17:29:32]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-01 17:31:59]

Description was changed from

==========
sandwich: Makes pull_sandwich_metrics.py a sub-command.

BUG=582080
==========

to

==========
sandwich: Makes pull_sandwich_metrics.py a sub-command.

BUG=582080

Committed: https://crrev.com/04c9261d40089b8454379f35df33e908b2ddbbb0
Cr-Commit-Position: refs/heads/master@{#378467}
==========

[commit-bot: I haz the power, 2016-03-01 17:32:00]

Patchset 3 (id:??) landed as
https://crrev.com/04c9261d40089b8454379f35df33e908b2ddbbb0
Cr-Commit-Position: refs/heads/master@{#378467}