|
|
DescriptionScript to load a list of URLs in a loop and save traces.
BUG=582080
Committed: https://crrev.com/6f4d9bc54affbe4eb8a26adc5121481093e1e886
Cr-Commit-Position: refs/heads/master@{#372111}
Patch Set 1 #
Total comments: 6
Patch Set 2 : addressed comments by mattcary #Patch Set 3 : silence pylint for unused vars #
Messages
Total messages: 19 (9 generated)
Description was changed from ========== Script to load a list of URLs in a loop and save traces. BUG=582080 ========== to ========== Script to load a list of URLs in a loop and save traces. BUG=582080 ==========
pasko@chromium.org changed reviewers: + lizeb@chromium.org, mattcary@chromium.org
PTaL. Presubmit failed in content_classification_lens_unittest. Ignored it for now: I am as stubborn as to reject using pip. Later we can probably do one of: 1. install the package with install-build-deps.sh 2. disable this presubmit test 3. add the package to third_party
lgtm https://codereview.chromium.org/1645003003/diff/1/tools/android/loading/run_s... File tools/android/loading/run_sandwich.py (right): https://codereview.chromium.org/1645003003/diff/1/tools/android/loading/run_s... tools/android/loading/run_sandwich.py:71: with open(file_name, 'w') as f: Could use gzip.GzipFile if we get concerned about space ever. https://codereview.chromium.org/1645003003/diff/1/tools/android/loading/run_s... tools/android/loading/run_sandwich.py:87: parser.add_argument('--repeat', default='1', Setting type=int avoids the int(args.repeat) conversion further down, and may give better error messages if you pass in a non-int for the argument (eg, not just an error is thrown, maybe floating point is handled correctly). https://codereview.chromium.org/1645003003/diff/1/tools/android/loading/run_s... tools/android/loading/run_sandwich.py:100: for iteration in xrange(0, int(args.repeat)): 0, unnecessary for xrange.
On 2016/01/28 15:35:35, mattcary wrote: > lgtm > > https://codereview.chromium.org/1645003003/diff/1/tools/android/loading/run_s... > File tools/android/loading/run_sandwich.py (right): > > https://codereview.chromium.org/1645003003/diff/1/tools/android/loading/run_s... > tools/android/loading/run_sandwich.py:71: with open(file_name, 'w') as f: > Could use gzip.GzipFile if we get concerned about space ever. > > https://codereview.chromium.org/1645003003/diff/1/tools/android/loading/run_s... > tools/android/loading/run_sandwich.py:87: parser.add_argument('--repeat', > default='1', > Setting type=int avoids the int(args.repeat) conversion further down, and may > give better error messages if you pass in a non-int for the argument (eg, not > just an error is thrown, maybe floating point is handled correctly). > > https://codereview.chromium.org/1645003003/diff/1/tools/android/loading/run_s... > tools/android/loading/run_sandwich.py:100: for iteration in xrange(0, > int(args.repeat)): > 0, unnecessary for xrange. lgtm. Any reason why we don't care about the network track? I would guess that it would definitively be interesting here.
lizeb: mattcary: thanks for quick review On 2016/01/28 15:45:41, Benoit L wrote: > Any reason why we don't care about the network track? I would guess that it > would definitively be interesting here. Just avoiding unnecessary dependency for now. Maybe it is interesting, but also maybe we will get all the necessary data from netlog in the trace. https://codereview.chromium.org/1645003003/diff/1/tools/android/loading/run_s... File tools/android/loading/run_sandwich.py (right): https://codereview.chromium.org/1645003003/diff/1/tools/android/loading/run_s... tools/android/loading/run_sandwich.py:71: with open(file_name, 'w') as f: On 2016/01/28 15:35:35, mattcary wrote: > Could use gzip.GzipFile if we get concerned about space ever. Thanks, noticed that it is done in another script. The current size is about 5MiB per page, not concerned yet, will change as necessary. https://codereview.chromium.org/1645003003/diff/1/tools/android/loading/run_s... tools/android/loading/run_sandwich.py:87: parser.add_argument('--repeat', default='1', On 2016/01/28 15:35:34, mattcary wrote: > Setting type=int avoids the int(args.repeat) conversion further down, and may > give better error messages if you pass in a non-int for the argument (eg, not > just an error is thrown, maybe floating point is handled correctly). oh nice, done https://codereview.chromium.org/1645003003/diff/1/tools/android/loading/run_s... tools/android/loading/run_sandwich.py:100: for iteration in xrange(0, int(args.repeat)): On 2016/01/28 15:35:35, mattcary wrote: > 0, unnecessary for xrange. Done
The CQ bit was checked by pasko@chromium.org
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/1645003003/#ps20001 (title: "addressed comments by mattcary")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1645003003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1645003003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
pylint complains about unused variables, silenced by renaming to '_' and removing
The CQ bit was checked by pasko@chromium.org
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/1645003003/#ps40001 (title: "silence pylint for unused vars")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1645003003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1645003003/40001
Message was sent while issue was closed.
Description was changed from ========== Script to load a list of URLs in a loop and save traces. BUG=582080 ========== to ========== Script to load a list of URLs in a loop and save traces. BUG=582080 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Script to load a list of URLs in a loop and save traces. BUG=582080 ========== to ========== Script to load a list of URLs in a loop and save traces. BUG=582080 Committed: https://crrev.com/6f4d9bc54affbe4eb8a26adc5121481093e1e886 Cr-Commit-Position: refs/heads/master@{#372111} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/6f4d9bc54affbe4eb8a26adc5121481093e1e886 Cr-Commit-Position: refs/heads/master@{#372111} |