Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2914843002:
Refactor fetch_benchmark_deps.py script to allow fetching all benchmark deps (Closed)
DescriptionRefactor fetch_benchmark_deps.py script to allow fetching all benchmark deps
After this:
fetch_benchmark_deps.py <benchmark foo>: fetch deps of benchmark foo
fetch_benchmark_deps.py: fetch all deps of all benchmarks
fetch_benchmark_deps.py -f: fetch all deps of all benchmarks without
prompt question
BUG=725516
TBR=eyaich@chromium.org
Review-Url: https://codereview.chromium.org/2914843002
Cr-Commit-Position: refs/heads/master@{#476891}
Committed: https://chromium.googlesource.com/chromium/src/+/ad2bd774f996150e57e1fc2ef26939f268bf6c37
Patch Set 1 #
Total comments: 2
Patch Set 2 : Keep stdout unchange for single benchmark fetching case #Patch Set 3 : update #
Messages
Total messages: 37 (21 generated)
Description was changed from ========== Refactor fetch_benchmark_deps.py script to allow fetching all benchmark deps BUG= ========== to ========== Refactor fetch_benchmark_deps.py script to allow fetching all benchmark deps BUG=725516 ==========
nednguyen@google.com changed reviewers: + laszio@chromium.org
Description was changed from ========== Refactor fetch_benchmark_deps.py script to allow fetching all benchmark deps BUG=725516 ========== to ========== Refactor fetch_benchmark_deps.py script to allow fetching all benchmark deps After this: fetch_benchmark_deps.py <benchmark foo>: fetch deps of benchmark foo fetch_benchmark_deps.py: fetch all deps of all benchmarks fetch_benchmark_deps.py -f: fetch all deps of all benchmarks without prompt question BUG=725516 ==========
laszio@google.com changed reviewers: + laszio@google.com
https://codereview.chromium.org/2914843002/diff/1/tools/perf/fetch_benchmark_... File tools/perf/fetch_benchmark_deps.py (right): https://codereview.chromium.org/2914843002/diff/1/tools/perf/fetch_benchmark_... tools/perf/fetch_benchmark_deps.py:63: print 'Fetch dependencies for benchmark %s:' % benchmark.Name() Some autotest tests in Chrome OS relies on the outputs (what's printed) of fetch_benchmark_deps.py to further process the downloaded files. For example, they do something like: $ fetch_benchmark_deps.py blabla | xargs -I__FILES__ scp __FILES__ DUT This will break the assumption / interface. https://codereview.chromium.org/2914843002/diff/1/tools/perf/fetch_benchmark_... tools/perf/fetch_benchmark_deps.py:99: 'Press enter to continue...') Same here. An easy workaround could be printing prompts to stderr. I can also help change corresponding parts in autotest if you prefer defining a better interface.
> Same here. An easy workaround could be printing prompts to stderr. Another alternative could be defining a wildcard that will cause everything to be downloaded, or even a regex so that the logics (download one benchmark or all) can be merged. I'm not sure if that could be over engineering, though.
On 2017/06/01 18:48:44, laszio wrote: > https://codereview.chromium.org/2914843002/diff/1/tools/perf/fetch_benchmark_... > File tools/perf/fetch_benchmark_deps.py (right): > > https://codereview.chromium.org/2914843002/diff/1/tools/perf/fetch_benchmark_... > tools/perf/fetch_benchmark_deps.py:63: print 'Fetch dependencies for benchmark > %s:' % benchmark.Name() > Some autotest tests in Chrome OS relies on the outputs (what's printed) of > fetch_benchmark_deps.py to further process the downloaded files. For example, > they do something like: > > $ fetch_benchmark_deps.py blabla | xargs -I__FILES__ scp __FILES__ DUT > > This will break the assumption / interface. > > https://codereview.chromium.org/2914843002/diff/1/tools/perf/fetch_benchmark_... > tools/perf/fetch_benchmark_deps.py:99: 'Press enter to continue...') > Same here. An easy workaround could be printing prompts to stderr. I can also > help change corresponding parts in autotest if you prefer defining a better > interface. Hmhh I see
The CQ bit was checked by nednguyen@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/06/02 02:28:28, nednguyen wrote: > On 2017/06/01 18:48:44, laszio wrote: > > > https://codereview.chromium.org/2914843002/diff/1/tools/perf/fetch_benchmark_... > > File tools/perf/fetch_benchmark_deps.py (right): > > > > > https://codereview.chromium.org/2914843002/diff/1/tools/perf/fetch_benchmark_... > > tools/perf/fetch_benchmark_deps.py:63: print 'Fetch dependencies for benchmark > > %s:' % benchmark.Name() > > Some autotest tests in Chrome OS relies on the outputs (what's printed) of > > fetch_benchmark_deps.py to further process the downloaded files. For example, > > they do something like: > > > > $ fetch_benchmark_deps.py blabla | xargs -I__FILES__ scp __FILES__ DUT > > > > This will break the assumption / interface. > > > > > https://codereview.chromium.org/2914843002/diff/1/tools/perf/fetch_benchmark_... > > tools/perf/fetch_benchmark_deps.py:99: 'Press enter to continue...') > > Same here. An easy workaround could be printing prompts to stderr. I can also > > help change corresponding parts in autotest if you prefer defining a better > > interface. > > Hmhh I see I keep the stdout unchanged for single benchmark deps fetching case, PTAL
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
On 2017/06/02 02:32:14, nednguyen wrote: > On 2017/06/02 02:28:28, nednguyen wrote: > > On 2017/06/01 18:48:44, laszio wrote: > > > > > > https://codereview.chromium.org/2914843002/diff/1/tools/perf/fetch_benchmark_... > > > File tools/perf/fetch_benchmark_deps.py (right): > > > > > > > > > https://codereview.chromium.org/2914843002/diff/1/tools/perf/fetch_benchmark_... > > > tools/perf/fetch_benchmark_deps.py:63: print 'Fetch dependencies for > benchmark > > > %s:' % benchmark.Name() > > > Some autotest tests in Chrome OS relies on the outputs (what's printed) of > > > fetch_benchmark_deps.py to further process the downloaded files. For > example, > > > they do something like: > > > > > > $ fetch_benchmark_deps.py blabla | xargs -I__FILES__ scp __FILES__ DUT > > > > > > This will break the assumption / interface. > > > > > > > > > https://codereview.chromium.org/2914843002/diff/1/tools/perf/fetch_benchmark_... > > > tools/perf/fetch_benchmark_deps.py:99: 'Press enter to continue...') > > > Same here. An easy workaround could be printing prompts to stderr. I can > also > > > help change corresponding parts in autotest if you prefer defining a better > > > interface. > > > > Hmhh I see > > I keep the stdout unchanged for single benchmark deps fetching case, PTAL LGTM
The CQ bit was checked by nednguyen@google.com
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a full super star committer. Committers are members of the group "project-chromium-committers". Note that this has nothing to do with OWNERS files.
Description was changed from ========== Refactor fetch_benchmark_deps.py script to allow fetching all benchmark deps After this: fetch_benchmark_deps.py <benchmark foo>: fetch deps of benchmark foo fetch_benchmark_deps.py: fetch all deps of all benchmarks fetch_benchmark_deps.py -f: fetch all deps of all benchmarks without prompt question BUG=725516 ========== to ========== Refactor fetch_benchmark_deps.py script to allow fetching all benchmark deps After this: fetch_benchmark_deps.py <benchmark foo>: fetch deps of benchmark foo fetch_benchmark_deps.py: fetch all deps of all benchmarks fetch_benchmark_deps.py -f: fetch all deps of all benchmarks without prompt question BUG=725516 TBR=eyaich@chromium.org ==========
The CQ bit was checked by nednguyen@google.com
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by nednguyen@google.com
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by nednguyen@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from laszio@google.com Link to the patchset: https://codereview.chromium.org/2914843002/#ps40001 (title: "update")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by nednguyen@google.com
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496463589245350,
"parent_rev": "deeb6adc549e8b435048262eaa6efce141e6e26c", "commit_rev":
"ad2bd774f996150e57e1fc2ef26939f268bf6c37"}
Message was sent while issue was closed.
Description was changed from ========== Refactor fetch_benchmark_deps.py script to allow fetching all benchmark deps After this: fetch_benchmark_deps.py <benchmark foo>: fetch deps of benchmark foo fetch_benchmark_deps.py: fetch all deps of all benchmarks fetch_benchmark_deps.py -f: fetch all deps of all benchmarks without prompt question BUG=725516 TBR=eyaich@chromium.org ========== to ========== Refactor fetch_benchmark_deps.py script to allow fetching all benchmark deps After this: fetch_benchmark_deps.py <benchmark foo>: fetch deps of benchmark foo fetch_benchmark_deps.py: fetch all deps of all benchmarks fetch_benchmark_deps.py -f: fetch all deps of all benchmarks without prompt question BUG=725516 TBR=eyaich@chromium.org Review-Url: https://codereview.chromium.org/2914843002 Cr-Commit-Position: refs/heads/master@{#476891} Committed: https://chromium.googlesource.com/chromium/src/+/ad2bd774f996150e57e1fc2ef269... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/ad2bd774f996150e57e1fc2ef269... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
