[devil refactor] Prepare clients for DeviceUnreachableError

[devil refactor] Prepare clients for DeviceUnreachableError

The catapult CL [1] will introduce a change where running adb commands
may now raise device_errors.DeviceUnreachableError if the device is
no longer found.

This change adjusts clients that were hoping to catch this kind of
error as a device_errors.[Adb]CommandFailedError.

[1]: https://codereview.chromium.org/2808763004/

BUG=https://github.com/catapult-project/catapult/issues/3476

Review-Url: https://codereview.chromium.org/2811993002
Cr-Commit-Position: refs/heads/master@{#464383}
Committed: https://chromium.googlesource.com/chromium/src/+/46865bb32b95d0ce0f16e12c8a59bf30a6fd8da4

[jbudorick, 2017-04-11 16:16:35]

jbudorick@chromium.org changed reviewers:
+ jbudorick@chromium.org

[jbudorick, 2017-04-11 16:16:36]

lgtm

I imagine we'll want to revisit how specific DeviceUnreachableErrors are handled
in the future.

[perezju, 2017-04-12 10:34:23]

perezju@chromium.org changed reviewers:
+ dpranke@chromium.org, pasko@chromium.org

[perezju, 2017-04-12 10:34:23]

Please review changes in:

pasko: tools/cygprofile/profile_android_startup.py
dpranke: third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/android.py

thanks!

[pasko, 2017-04-12 12:59:17]

On 2017/04/12 10:34:23, perezju wrote:
> Please review changes in:
> 
> pasko: tools/cygprofile/profile_android_startup.py
> dpranke:
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/android.py
> 
> thanks!

For the commit message without looking at the actual change:

please make the BUG= a clickable link in this codereview. A link like this would
work for me:

BUG=https://github.com/catapult-project/catapult/issues/3476

If you have a common url shortener prefix in the catapult project, that would
work as well. Changing Rietveld would be less preferable :), since I also want
to be able to follow links in git log.

[perezju, 2017-04-12 13:29:48]

On 2017/04/12 12:59:17, pasko wrote:
> On 2017/04/12 10:34:23, perezju wrote:
> > Please review changes in:
> > 
> > pasko: tools/cygprofile/profile_android_startup.py
> > dpranke:
> third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/android.py
> > 
> > thanks!
> 
> For the commit message without looking at the actual change:
> 
> please make the BUG= a clickable link in this codereview. A link like this
would
> work for me:
> 
> BUG=https://github.com/catapult-project/catapult/issues/3476
> 
> If you have a common url shortener prefix in the catapult project, that would
> work as well. Changing Rietveld would be less preferable :), since I also want
> to be able to follow links in git log.

Would go/catabug/3476 be acceptable? Not sure if there is a standard practice
for chromium side CLs pointing to catapult issues?

[pasko, 2017-04-12 13:51:48]

forgot to say: the change in tools/cygprofile looks good

On 2017/04/12 13:29:48, perezju wrote:
> On 2017/04/12 12:59:17, pasko wrote:
> > On 2017/04/12 10:34:23, perezju wrote:
> > > Please review changes in:
> > > 
> > > pasko: tools/cygprofile/profile_android_startup.py
> > > dpranke:
> > third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/android.py
> > > 
> > > thanks!
> > 
> > For the commit message without looking at the actual change:
> > 
> > please make the BUG= a clickable link in this codereview. A link like this
> would
> > work for me:
> > 
> > BUG=https://github.com/catapult-project/catapult/issues/3476
> > 
> > If you have a common url shortener prefix in the catapult project, that
would
> > work as well. Changing Rietveld would be less preferable :), since I also
want
> > to be able to follow links in git log.
> 
> Would go/catabug/3476 be acceptable? Not sure if there is a standard practice
> for chromium side CLs pointing to catapult issues?

this still requires to copy and paste, and is for googlers only, so not good
enough imo. In the chromium git log I see commits in the same format as this
one, but occasionally links to github.com, just as I like. In the log the refs
to catapult also occasionally miss the '#' or put it into a different place. I
think a valid link would make it less error-prone as well. Is it too verbose for
you? If we want a broader agreement, what would be a proper mailing list?

[perezju, 2017-04-12 14:20:29]

Description was changed from

==========
[devil refactor] Prepare clients for DeviceUnreachableError

The catapult CL [1] will introduce a change where running adb commands
may now raise device_errors.DeviceUnreachableError if the device is
no longer found.

This change adjusts clients that were hoping to catch this kind of
error as a device_errors.[Adb]CommandFailedError.

[1]: https://codereview.chromium.org/2808763004/

BUG=catapult:#3476
==========

to

==========
[devil refactor] Prepare clients for DeviceUnreachableError

The catapult CL [1] will introduce a change where running adb commands
may now raise device_errors.DeviceUnreachableError if the device is
no longer found.

This change adjusts clients that were hoping to catch this kind of
error as a device_errors.[Adb]CommandFailedError.

[1]: https://codereview.chromium.org/2808763004/

BUG=https://github.com/catapult-project/catapult/issues/3476
==========

[perezju, 2017-04-12 14:21:44]

On 2017/04/12 13:51:48, pasko wrote:
> forgot to say: the change in tools/cygprofile looks good
> 
> On 2017/04/12 13:29:48, perezju wrote:
> > On 2017/04/12 12:59:17, pasko wrote:
> > > On 2017/04/12 10:34:23, perezju wrote:
> > > > Please review changes in:
> > > > 
> > > > pasko: tools/cygprofile/profile_android_startup.py
> > > > dpranke:
> > > third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/android.py
> > > > 
> > > > thanks!
> > > 
> > > For the commit message without looking at the actual change:
> > > 
> > > please make the BUG= a clickable link in this codereview. A link like this
> > would
> > > work for me:
> > > 
> > > BUG=https://github.com/catapult-project/catapult/issues/3476
> > > 
> > > If you have a common url shortener prefix in the catapult project, that
> would
> > > work as well. Changing Rietveld would be less preferable :), since I also
> want
> > > to be able to follow links in git log.
> > 
> > Would go/catabug/3476 be acceptable? Not sure if there is a standard
practice
> > for chromium side CLs pointing to catapult issues?
> 
> this still requires to copy and paste, and is for googlers only, so not good
> enough imo. In the chromium git log I see commits in the same format as this
> one, but occasionally links to http://github.com, just as I like. In the log
the refs
> to catapult also occasionally miss the '#' or put it into a different place. I
> think a valid link would make it less error-prone as well. Is it too verbose
for
> you? If we want a broader agreement, what would be a proper mailing list?

Don't have any strong opinions. Changed the BUG to point to the github issue.

[Dirk Pranke, 2017-04-13 01:25:45]

lgtm

[perezju, 2017-04-13 10:45:12]

The CQ bit was checked by perezju@chromium.org

[commit-bot: I haz the power, 2017-04-13 10:45:31]

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

[pasko, 2017-04-13 11:09:46]

On 2017/04/12 14:21:44, perezju wrote:
> Don't have any strong opinions. Changed the BUG to point to the github issue.

Thank you.

tools/cygprofile lgtm

[commit-bot: I haz the power, 2017-04-13 12:19:55]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1492080312941110, "parent_rev":
"a612e798921b4a374aafdb907ed9a38ed0a6c476", "commit_rev":
"46865bb32b95d0ce0f16e12c8a59bf30a6fd8da4"}

[commit-bot: I haz the power, 2017-04-13 12:20:47]

Description was changed from

==========
[devil refactor] Prepare clients for DeviceUnreachableError

The catapult CL [1] will introduce a change where running adb commands
may now raise device_errors.DeviceUnreachableError if the device is
no longer found.

This change adjusts clients that were hoping to catch this kind of
error as a device_errors.[Adb]CommandFailedError.

[1]: https://codereview.chromium.org/2808763004/

BUG=https://github.com/catapult-project/catapult/issues/3476
==========

to

==========
[devil refactor] Prepare clients for DeviceUnreachableError

The catapult CL [1] will introduce a change where running adb commands
may now raise device_errors.DeviceUnreachableError if the device is
no longer found.

This change adjusts clients that were hoping to catch this kind of
error as a device_errors.[Adb]CommandFailedError.

[1]: https://codereview.chromium.org/2808763004/

BUG=https://github.com/catapult-project/catapult/issues/3476

Review-Url: https://codereview.chromium.org/2811993002
Cr-Commit-Position: refs/heads/master@{#464383}
Committed:
https://chromium.googlesource.com/chromium/src/+/46865bb32b95d0ce0f16e12c8a59...
==========

[commit-bot: I haz the power, 2017-04-13 12:20:48]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/46865bb32b95d0ce0f16e12c8a59...