Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(648)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1946513003: [Findit] Modify the handler for fracas input message. (Closed)

Created:
4 years, 7 months ago by Sharu Jiang
Modified:
4 years, 7 months ago
CC:
chromium-reviews, infra-reviews+infra_chromium.org, mmandlis
Base URL:
https://chromium.googlesource.com/infra/infra.git@master
Target Ref:
refs/heads/master
Project:
infra
Visibility:
Public.

Description

[Findit] Modify the handler for fracas input message. BUG=608925 Committed: https://chromium.googlesource.com/infra/infra/+/d28587d3860e5c9def92c64f469eaf27f7d96e44

Patch Set 1 #

Total comments: 4

Patch Set 2 : Use crash_identifiers as key. #

Total comments: 2

Patch Set 3 : Address comment. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+232 lines, -124 lines) Patch
M appengine/findit/crash/fracas_crash_pipeline.py View 1 2 7 chunks +43 lines, -36 lines 2 comments Download
M appengine/findit/crash/test/fracas_crash_pipeline_test.py View 1 2 2 chunks +82 lines, -37 lines 0 comments Download
M appengine/findit/handlers/crash/fracas_crash.py View 1 2 chunks +36 lines, -11 lines 0 comments Download
M appengine/findit/handlers/crash/test/fracas_crash_test.py View 1 2 chunks +33 lines, -9 lines 0 comments Download
M appengine/findit/model/crash/crash_analysis.py View 1 2 chunks +7 lines, -1 line 0 comments Download
M appengine/findit/model/crash/fracas_crash_analysis.py View 1 2 2 chunks +13 lines, -19 lines 0 comments Download
M appengine/findit/model/crash/test/fracas_crash_analysis_test.py View 1 1 chunk +18 lines, -11 lines 0 comments Download

Messages

Total messages: 20 (9 generated)
Sharu Jiang
PTAL :)
4 years, 7 months ago (2016-05-03 18:32:24 UTC) #3
Martin Barbella
No issues other than the naming nit. https://codereview.chromium.org/1946513003/diff/1/appengine/findit/crash/fracas_crash_pipeline.py File appengine/findit/crash/fracas_crash_pipeline.py (right): https://codereview.chromium.org/1946513003/diff/1/appengine/findit/crash/fracas_crash_pipeline.py#newcode64 appengine/findit/crash/fracas_crash_pipeline.py:64: analysis.crashed_version, analysis.versions_to_historical) ...
4 years, 7 months ago (2016-05-03 18:43:09 UTC) #5
stgao
https://codereview.chromium.org/1946513003/diff/1/appengine/findit/handlers/crash/fracas_crash.py File appengine/findit/handlers/crash/fracas_crash.py (right): https://codereview.chromium.org/1946513003/diff/1/appengine/findit/handlers/crash/fracas_crash.py#newcode63 appengine/findit/handlers/crash/fracas_crash.py:63: fracas_crash_pipeline.ScheduleNewAnalysisForCrash( ``crash_identifiers`` should be passed over, used as the ...
4 years, 7 months ago (2016-05-03 18:58:34 UTC) #6
Sharu Jiang
https://codereview.chromium.org/1946513003/diff/1/appengine/findit/crash/fracas_crash_pipeline.py File appengine/findit/crash/fracas_crash_pipeline.py (right): https://codereview.chromium.org/1946513003/diff/1/appengine/findit/crash/fracas_crash_pipeline.py#newcode64 appengine/findit/crash/fracas_crash_pipeline.py:64: analysis.crashed_version, analysis.versions_to_historical) On 2016/05/03 18:43:09, Martin Barbella wrote: > ...
4 years, 7 months ago (2016-05-03 23:48:03 UTC) #7
stgao
lgtm after comment is addressed. https://codereview.chromium.org/1946513003/diff/20001/appengine/findit/model/crash/fracas_crash_analysis.py File appengine/findit/model/crash/fracas_crash_analysis.py (right): https://codereview.chromium.org/1946513003/diff/20001/appengine/findit/model/crash/fracas_crash_analysis.py#newcode27 appengine/findit/model/crash/fracas_crash_analysis.py:27: crash_identifiers['channel'], crash_identifiers['platform'], This solution ...
4 years, 7 months ago (2016-05-04 00:12:10 UTC) #9
Martin Barbella
+1 to Shuotao's comment. Otherwise lgtm.
4 years, 7 months ago (2016-05-04 00:21:51 UTC) #10
Sharu Jiang
https://codereview.chromium.org/1946513003/diff/20001/appengine/findit/model/crash/fracas_crash_analysis.py File appengine/findit/model/crash/fracas_crash_analysis.py (right): https://codereview.chromium.org/1946513003/diff/20001/appengine/findit/model/crash/fracas_crash_analysis.py#newcode27 appengine/findit/model/crash/fracas_crash_analysis.py:27: crash_identifiers['channel'], crash_identifiers['platform'], On 2016/05/04 00:12:10, stgao wrote: > This ...
4 years, 7 months ago (2016-05-04 00:57:17 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1946513003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1946513003/60001
4 years, 7 months ago (2016-05-04 01:16:55 UTC) #15
commit-bot: I haz the power
Committed patchset #3 (id:60001) as https://chromium.googlesource.com/infra/infra/+/d28587d3860e5c9def92c64f469eaf27f7d96e44
4 years, 7 months ago (2016-05-04 01:20:54 UTC) #17
stgao
https://codereview.chromium.org/1946513003/diff/60001/appengine/findit/crash/fracas_crash_pipeline.py File appengine/findit/crash/fracas_crash_pipeline.py (right): https://codereview.chromium.org/1946513003/diff/60001/appengine/findit/crash/fracas_crash_pipeline.py#newcode111 appengine/findit/crash/fracas_crash_pipeline.py:111: print 'lala' nit: leftover of local debugging?
4 years, 7 months ago (2016-05-04 05:55:43 UTC) #18
Sharu Jiang
4 years, 7 months ago (2016-05-04 18:34:49 UTC) #20
Message was sent while issue was closed. 
https://codereview.chromium.org/1946513003/diff/60001/appengine/findit/crash/...
File appengine/findit/crash/fracas_crash_pipeline.py (right):

https://codereview.chromium.org/1946513003/diff/60001/appengine/findit/crash/...
appengine/findit/crash/fracas_crash_pipeline.py:111: print 'lala'
On 2016/05/04 05:55:43, stgao wrote:
> nit: leftover of local debugging?

Sorry, commit it in a hurry, forgot to delete it. Will delete in next cl :)

Powered by Google App Engine
This is Rietveld 408576698