Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2781953002:
Allow atrace over adb tcp/ip (Closed)
DescriptionAllow atrace over adb tcp/ip
atrace did not work with adb over tcp/ip. Systrace assumed that devices with an IP serial are GCE instances, and this PR removes the GCE ADB Wrapper creation logic.
This issue originated from issue 2774353003.
Review-Url: https://codereview.chromium.org/2781953002
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/f0f964bdb9ce5deb589a3df34845950ba77f4dda
Patch Set 1 #
Total comments: 2
Patch Set 2 : Remove GceAdbWrapper creation logic, to allow atrace over tcp-ip #Messages
Total messages: 27 (17 generated)
sahilja@google.com changed reviewers: + ccraik@google.com
Description was changed from ========== Allow atrace over adb tcp/ip atrace did not work with adb over tcp/ip. Systrace assumed that devices with an IP serial are GCE instances, and this PR allows the user to --disable-gce which removes this assumption. ========== to ========== Allow atrace over adb tcp/ip atrace did not work with adb over tcp/ip. Systrace assumed that devices with an IP serial are GCE instances, and this PR allows the user to --disable-gce which removes this assumption. This issue originated from issue 2774353003. ==========
The CQ bit was checked by sahilja@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
+John
jbudorick@chromium.org changed reviewers: + jbudorick@chromium.org
Ben: can we just rip out the GceAdbWrapper creation logic & revisit it when we revisit the gce stuff? https://codereview.chromium.org/2781953002/diff/1/devil/devil/android/device_... File devil/devil/android/device_utils.py (right): https://codereview.chromium.org/2781953002/diff/1/devil/devil/android/device_... devil/devil/android/device_utils.py:298: default_retries=_DEFAULT_RETRIES, disable_gce=False): I don't think this is the right way to handle this; I think we'd instead want the locations that are using the GCE functionality to pass a GceAdbWrapper directly.
On 2017/03/28 20:36:03, jbudorick wrote: > Ben: can we just rip out the GceAdbWrapper creation logic & revisit it when we > revisit the gce stuff? Go for it
On 2017/03/28 21:12:52, bpastene wrote: > On 2017/03/28 20:36:03, jbudorick wrote: > > Ben: can we just rip out the GceAdbWrapper creation logic & revisit it when we > > revisit the gce stuff? > > Go for it Thanks Ben! I'll remove the creation of GceAdbWrapper. I'll leave it to you to implement a different strategy for deciding whether or not a device is a GceInstance.
Description was changed from ========== Allow atrace over adb tcp/ip atrace did not work with adb over tcp/ip. Systrace assumed that devices with an IP serial are GCE instances, and this PR allows the user to --disable-gce which removes this assumption. This issue originated from issue 2774353003. ========== to ========== Allow atrace over adb tcp/ip atrace did not work with adb over tcp/ip. Systrace assumed that devices with an IP serial are GCE instances, and this PR removes the GCE ADB Wrapper creation logic. This issue originated from issue 2774353003. ==========
The CQ bit was checked by sahilja@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...
https://codereview.chromium.org/2781953002/diff/1/devil/devil/android/device_... File devil/devil/android/device_utils.py (right): https://codereview.chromium.org/2781953002/diff/1/devil/devil/android/device_... devil/devil/android/device_utils.py:298: default_retries=_DEFAULT_RETRIES, disable_gce=False): On 2017/03/28 20:36:03, jbudorick wrote: > I don't think this is the right way to handle this; I think we'd instead want > the locations that are using the GCE functionality to pass a GceAdbWrapper > directly. Done.
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by sahilja@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 sahilja@google.com
The CQ bit was checked by sahilja@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": 20001, "attempt_start_ts": 1491337185804110,
"parent_rev": "f751c1cb3afa8bd07c1cc663c32bbd79a0c69044", "commit_rev":
"f0f964bdb9ce5deb589a3df34845950ba77f4dda"}
Message was sent while issue was closed.
Description was changed from ========== Allow atrace over adb tcp/ip atrace did not work with adb over tcp/ip. Systrace assumed that devices with an IP serial are GCE instances, and this PR removes the GCE ADB Wrapper creation logic. This issue originated from issue 2774353003. ========== to ========== Allow atrace over adb tcp/ip atrace did not work with adb over tcp/ip. Systrace assumed that devices with an IP serial are GCE instances, and this PR removes the GCE ADB Wrapper creation logic. This issue originated from issue 2774353003. Review-Url: https://codereview.chromium.org/2781953002 Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapu... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/external/github.com/catapult-project/catapu... |
