Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2577463003:
Enable writing to logdog for tombstones.py. (Closed)
|
Created:
4 years ago by BigBossZhiling Modified:
4 years ago Reviewers:
jbudorick CC:
chromium-reviews, mikecase+watch_chromium.org, jbudorick+watch_chromium.org, agrieve+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionEnable writing to logdog for tombstones.py.
Previously tombstones are saved as a long string test result object.
Tombstones are usually huge, so we have decided to write tombstones to
logdog and save the url, instead of the whole tombstones, as part of
test result object.
BUG=605572
Committed: https://crrev.com/15465f226d8aefffc34940cf9f96f766101bb2f4
Cr-Commit-Position: refs/heads/master@{#438665}
Patch Set 1 #
Total comments: 2
Patch Set 2 : moved the logdog logic to a new method #
Total comments: 17
Patch Set 3 : fixes #Patch Set 4 : fixes #
Total comments: 2
Patch Set 5 : changed open_text to text #Messages
Total messages: 17 (6 generated)
Description was changed from ========== Enable writing to logdog for tombstones.py. Previously tombstones are saved as a long string test result object. Tombstones are usually huge, so we have decided to write tombstones to logdog and save the url, instead of the whole tombstones, as part of test result object. BUG=605572 ========== to ========== Enable writing to logdog for tombstones.py. Previously tombstones are saved as a long string test result object. Tombstones are usually huge, so we have decided to write tombstones to logdog and save the url, instead of the whole tombstones, as part of test result object. BUG=605572 ==========
hzl@google.com changed reviewers: + jbudorick@chromium.org
https://codereview.chromium.org/2577463003/diff/1/build/android/tombstones.py File build/android/tombstones.py (right): https://codereview.chromium.org/2577463003/diff/1/build/android/tombstones.py... build/android/tombstones.py:258: else: This should be handled by a separate function that either takes the resolved tombstones as a parameter or calls ResolveTombstones to get them, not by a flag passed to ResolveTombstones. (I think I prefer the first one.)
moved to a new method https://codereview.chromium.org/2577463003/diff/1/build/android/tombstones.py File build/android/tombstones.py (right): https://codereview.chromium.org/2577463003/diff/1/build/android/tombstones.py... build/android/tombstones.py:258: else: On 2016/12/14 01:34:04, jbudorick wrote: > This should be handled by a separate function that either takes the resolved > tombstones as a parameter or calls ResolveTombstones to get them, not by a flag > passed to ResolveTombstones. (I think I prefer the first one.) Done.
https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... File build/android/tombstones.py (right): https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:109: nit: +1 blank line https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:176: nit: +1 blank line https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:222: nit: +1 blank line https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:235: nit: +1 blank line https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:252: nit: +1 blank line https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:259: """ nit: Add a Returns: doc here. https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:263: logdog_stream = stream_client.open_text(stream_name) Do this with a context manager: with stream_client.text(stream_name) as logdog_stream: ... and then you can omit the .close() call. https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:275: nit: +1 blank line https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:329: nit: +1 blank line
https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... File build/android/tombstones.py (right): https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:109: On 2016/12/14 17:17:19, jbudorick wrote: > nit: +1 blank line Done. https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:176: On 2016/12/14 17:17:19, jbudorick wrote: > nit: +1 blank line Done. https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:222: On 2016/12/14 17:17:19, jbudorick wrote: > nit: +1 blank line Done. https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:235: On 2016/12/14 17:17:19, jbudorick wrote: > nit: +1 blank line Done. https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:259: """ On 2016/12/14 17:17:19, jbudorick wrote: > nit: Add a Returns: doc here. Done. https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:263: logdog_stream = stream_client.open_text(stream_name) On 2016/12/14 17:17:19, jbudorick wrote: > Do this with a context manager: > > with stream_client.text(stream_name) as logdog_stream: > ... > > and then you can omit the .close() call. Done. https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:275: On 2016/12/14 17:17:19, jbudorick wrote: > nit: +1 blank line Done. https://codereview.chromium.org/2577463003/diff/20001/build/android/tombstone... build/android/tombstones.py:329: On 2016/12/14 17:17:19, jbudorick wrote: > nit: +1 blank line Done.
https://codereview.chromium.org/2577463003/diff/60001/build/android/tombstone... File build/android/tombstones.py (right): https://codereview.chromium.org/2577463003/diff/60001/build/android/tombstone... build/android/tombstones.py:274: with stream_client.open_text(stream_name) as logdog_stream: stream_client.text(stream_name), not stream_client.open_text(stream_name): https://codesearch.chromium.org/chromium/src/tools/swarming_client/libs/logdo...
https://codereview.chromium.org/2577463003/diff/60001/build/android/tombstone... File build/android/tombstones.py (right): https://codereview.chromium.org/2577463003/diff/60001/build/android/tombstone... build/android/tombstones.py:274: with stream_client.open_text(stream_name) as logdog_stream: On 2016/12/14 17:58:44, jbudorick wrote: > stream_client.text(stream_name), not stream_client.open_text(stream_name): > https://codesearch.chromium.org/chromium/src/tools/swarming_client/libs/logdo... Done.
lgtm
The CQ bit was checked by hzl@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": 80001, "attempt_start_ts": 1481752105869340,
"parent_rev": "f6590f24fc1523447a08a60b67a72e83af504b51", "commit_rev":
"09a1ebbe4b19718c0608befa5ba4136c362ad06b"}
Message was sent while issue was closed.
Description was changed from ========== Enable writing to logdog for tombstones.py. Previously tombstones are saved as a long string test result object. Tombstones are usually huge, so we have decided to write tombstones to logdog and save the url, instead of the whole tombstones, as part of test result object. BUG=605572 ========== to ========== Enable writing to logdog for tombstones.py. Previously tombstones are saved as a long string test result object. Tombstones are usually huge, so we have decided to write tombstones to logdog and save the url, instead of the whole tombstones, as part of test result object. BUG=605572 Review-Url: https://codereview.chromium.org/2577463003 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Enable writing to logdog for tombstones.py. Previously tombstones are saved as a long string test result object. Tombstones are usually huge, so we have decided to write tombstones to logdog and save the url, instead of the whole tombstones, as part of test result object. BUG=605572 Review-Url: https://codereview.chromium.org/2577463003 ========== to ========== Enable writing to logdog for tombstones.py. Previously tombstones are saved as a long string test result object. Tombstones are usually huge, so we have decided to write tombstones to logdog and save the url, instead of the whole tombstones, as part of test result object. BUG=605572 Committed: https://crrev.com/15465f226d8aefffc34940cf9f96f766101bb2f4 Cr-Commit-Position: refs/heads/master@{#438665} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/15465f226d8aefffc34940cf9f96f766101bb2f4 Cr-Commit-Position: refs/heads/master@{#438665} |
