Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1305933002:
[Android] Add email alerts on unauthorized devices. (Closed)
|
Created:
5 years, 4 months ago by jbudorick Modified:
5 years, 4 months ago Reviewers:
ghost stip (do not use), rnephew (Wrong account), navabi1, rnephew (Reviews Here), navabi, mikecase (-- gone --) CC:
chromium-reviews, jbudorick+watch_chromium.org, klundberg+watch_chromium.org, yfriedman+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Android] Add email alerts on unauthorized devices.
BUG=523059
Committed: https://crrev.com/44f3ca54fa3c9680f042589655ff44594425ba37
Cr-Commit-Position: refs/heads/master@{#344754}
Patch Set 1 #
Total comments: 7
Messages
Total messages: 30 (10 generated)
jbudorick@chromium.org changed reviewers: + navabi@chromium.org, stip@chromium.org
This is the hard way of signing up for an email list.
jbudorick@chromium.org changed reviewers: + mikecase@chromium.org
jbudorick@chromium.org changed reviewers: + rnephew@chromium.org
jbudorick@chromium.org changed reviewers: + navabi@google.com
+other Armand
rnephew@google.com changed reviewers: + rnephew@google.com
https://codereview.chromium.org/1305933002/diff/1/build/android/buildbot/bb_d... File build/android/buildbot/bb_device_status_check.py (right): https://codereview.chromium.org/1305933002/diff/1/build/android/buildbot/bb_d... build/android/buildbot/bb_device_status_check.py:177: 'jbudorick@chromium.org', I'm not sure I like your email hardcoded in here, but its your inbox... RIP Inbox. https://codereview.chromium.org/1305933002/diff/1/build/android/pylib/device/... File build/android/pylib/device/adb_wrapper.py (right): https://codereview.chromium.org/1305933002/diff/1/build/android/pylib/device/... build/android/pylib/device/adb_wrapper.py:173: def Devices(cls, desired_state=_READY_STATE, long_list=False, Is is_ready used anywhere else that isn't changed in this cl? I did a quick search on chromium cs and I think it is used in a unit test for device utils. https://code.google.com/p/chromium/codesearch#search/&q=Devices%5C(%20is_read...
https://codereview.chromium.org/1305933002/diff/1/build/android/buildbot/bb_d... File build/android/buildbot/bb_device_status_check.py (right): https://codereview.chromium.org/1305933002/diff/1/build/android/buildbot/bb_d... build/android/buildbot/bb_device_status_check.py:177: 'jbudorick@chromium.org', On 2015/08/21 at 02:25:12, rnephew wrote: > I'm not sure I like your email hardcoded in here, but its your inbox... > RIP Inbox. bundled in the inbox, mercifully. https://codereview.chromium.org/1305933002/diff/1/build/android/pylib/device/... File build/android/pylib/device/adb_wrapper.py (right): https://codereview.chromium.org/1305933002/diff/1/build/android/pylib/device/... build/android/pylib/device/adb_wrapper.py:173: def Devices(cls, desired_state=_READY_STATE, long_list=False, On 2015/08/21 at 02:25:12, rnephew wrote: > Is is_ready used anywhere else that isn't changed in this cl? > > I did a quick search on chromium cs and I think it is used in a unit test for device utils. > > https://code.google.com/p/chromium/codesearch#search/&q=Devices%5C(%20is_read... same name, mocking a function in AdbWrapper rather than a property of Devices
https://codereview.chromium.org/1305933002/diff/1/build/android/buildbot/bb_d... File build/android/buildbot/bb_device_status_check.py (right): https://codereview.chromium.org/1305933002/diff/1/build/android/buildbot/bb_d... build/android/buildbot/bb_device_status_check.py:177: 'jbudorick@chromium.org', On 2015/08/21 at 02:27:38, jbudorick wrote: > On 2015/08/21 at 02:25:12, rnephew wrote: > > I'm not sure I like your email hardcoded in here, but its your inbox... > > RIP Inbox. > > bundled in the inbox, mercifully. Also, I intend this (along with the other email alerts in here) to be short-lived, hence the TODO below
On 2015/08/21 02:28:31, jbudorick wrote: > https://codereview.chromium.org/1305933002/diff/1/build/android/buildbot/bb_d... > File build/android/buildbot/bb_device_status_check.py (right): > > https://codereview.chromium.org/1305933002/diff/1/build/android/buildbot/bb_d... > build/android/buildbot/bb_device_status_check.py:177: mailto:, > On 2015/08/21 at 02:27:38, jbudorick wrote: > > On 2015/08/21 at 02:25:12, rnephew wrote: > > > I'm not sure I like your email hardcoded in here, but its your inbox... > > > RIP Inbox. > > > > bundled in the inbox, mercifully. > > Also, I intend this (along with the other email alerts in here) to be > short-lived, hence the TODO below lgtm
The CQ bit was checked by jbudorick@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1305933002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1305933002/1
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. Only full committers are accepted. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
On 2015/08/21 02:31:39, commit-bot: I haz the power wrote: > No L-G-T-M from a valid reviewer yet. Only full committers are accepted. > Even if an L-G-T-M may have been provided, it was from a non-committer, > _not_ a full super star committer. > See http://www.chromium.org/getting-involved/become-a-committer > Note that this has nothing to do with OWNERS files. lgtm from correct account.
The CQ bit was checked by jbudorick@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1305933002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1305933002/1
Same comment as Randy. I don't like sending the alerts to your email since (1) You'll get spammed and (2) no one else will have easy access to the alerts (I guess they could add their email as well here). How often are devices unauthorized? Should we create a dashboard to keep track of the status of all of the devices on our bots? https://codereview.chromium.org/1305933002/diff/1/build/android/buildbot/bb_d... File build/android/buildbot/bb_device_status_check.py (right): https://codereview.chromium.org/1305933002/diff/1/build/android/buildbot/bb_d... build/android/buildbot/bb_device_status_check.py:177: 'jbudorick@chromium.org', Shouldn't this be sent to the infra-lab people and not you? Is the chrome-android-device-alert@google.com email the infra-labs people?
devices aren't usually unauthorized. the M SDK roll + broken 'adb get-state' killed _a lot_ of the device fleet, though. :( https://codereview.chromium.org/1305933002/diff/1/build/android/buildbot/bb_d... File build/android/buildbot/bb_device_status_check.py (right): https://codereview.chromium.org/1305933002/diff/1/build/android/buildbot/bb_d... build/android/buildbot/bb_device_status_check.py:177: 'jbudorick@chromium.org', On 2015/08/21 at 02:40:53, mikecase wrote: > Shouldn't this be sent to the infra-lab people and not you? > Is the chrome-android-device-alert@google.com email the infra-labs people? yes
lgtm
On 2015/08/21 at 02:45:24, jbudorick wrote: > devices aren't usually unauthorized. the M SDK roll + broken 'adb get-state' killed _a lot_ of the device fleet, though. :( From my cq run: Unauthorized devices on build5-b4 chrome-bot to jbudorick, chrome-android-device-alert 8:02 PM The following devices are offline on build5-b4 06c114b0003b7fbc 05281056f0d7c4ac 06ae4e9f003b6d7f 06ab56b2003bee71 05b38e36f0d88cd4 Detected on linux_android_rel_ng #58779 > > https://codereview.chromium.org/1305933002/diff/1/build/android/buildbot/bb_d... > File build/android/buildbot/bb_device_status_check.py (right): > > https://codereview.chromium.org/1305933002/diff/1/build/android/buildbot/bb_d... > build/android/buildbot/bb_device_status_check.py:177: 'jbudorick@chromium.org', > On 2015/08/21 at 02:40:53, mikecase wrote: > > Shouldn't this be sent to the infra-lab people and not you? > > Is the chrome-android-device-alert@google.com email the infra-labs people? > > yes
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by jbudorick@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1305933002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1305933002/1
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/44f3ca54fa3c9680f042589655ff44594425ba37 Cr-Commit-Position: refs/heads/master@{#344754} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
