Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(8)

Issue 3011223003: [pinpoint] Fix race condition in building + isolate finding. (Closed)

Created:
2 months ago by dtu
Modified:
2 months ago
Reviewers:
perezju, shatch, sullivan
CC:
catapult-reviews_chromium.org, perf-dashboard-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
catapult
Visibility:
Public.

Description

[pinpoint] Fix race condition in building + isolate finding. Look for the isolate AFTER the build is completed. Also extract find_isolate._Poll() into a composed method. Review-Url: https://chromiumcodereview.appspot.com/3011223003 Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/c5be5cb62e76fd095f59167f68dbc88693eca540

Patch Set 1 #

Total comments: 2

Patch Set 2 : _CheckCompleted #

Unified diffs Side-by-side diffs Delta from patch set Stats (+41 lines, -22 lines) Patch
M dashboard/dashboard/pinpoint/models/quest/find_isolate.py View 1 1 chunk +41 lines, -22 lines 0 comments Download

Messages

Total messages: 11 (6 generated)
dtu
2 months ago (2017-09-13 07:02:24 UTC) #3
perezju
lgtm w/nit https://codereview.chromium.org/3011223003/diff/1/dashboard/dashboard/pinpoint/models/quest/find_isolate.py File dashboard/dashboard/pinpoint/models/quest/find_isolate.py (right): https://codereview.chromium.org/3011223003/diff/1/dashboard/dashboard/pinpoint/models/quest/find_isolate.py#newcode65 dashboard/dashboard/pinpoint/models/quest/find_isolate.py:65: True iff the isolate was found and ...
2 months ago (2017-09-13 13:46:38 UTC) #4
dtu
https://codereview.chromium.org/3011223003/diff/1/dashboard/dashboard/pinpoint/models/quest/find_isolate.py File dashboard/dashboard/pinpoint/models/quest/find_isolate.py (right): https://codereview.chromium.org/3011223003/diff/1/dashboard/dashboard/pinpoint/models/quest/find_isolate.py#newcode65 dashboard/dashboard/pinpoint/models/quest/find_isolate.py:65: True iff the isolate was found and the execution ...
2 months ago (2017-09-13 15:16:05 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/3011223003/20001
2 months ago (2017-09-13 15:16:45 UTC) #8
commit-bot: I haz the power
2 months ago (2017-09-13 16:10:52 UTC) #11
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...

Powered by Google App Engine
This is Rietveld efc10ee0f