New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 659345 link

Starred by 1 user
Status: Fixed
Owner:
█ wrengr@chromium.org
Last visit > 30 days ago
Closed: Nov 2016
Cc:
aarya@google.com
█ kateso...@chromium.org
mbarbe...@chromium.org
infe...@chromium.org
st...@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 644372



Sign in to add a comment

[Predator] Try to break the ScheduleNewAnalysis cycle

Project Member Reported by wrengr@chromium.org, Oct 25 2016 
Right now there's a dependency cycle between ./crash/findit.py and ./crash/crash_pipeline.py thanks to ScheduleNewAnalysis (which is only called from ./handlers/crash/...). We should try to fix this if at all possible.

Comment 1 by wrengr@chromium.org, Oct 25 2016

Cc: kateso...@chromium.org st...@chromium.org mbarbe...@chromium.org aarya@google.com infe...@chromium.org

Comment 2 by wrengr@chromium.org, Oct 27 2016 

So one of the issues here, and why I organized things so that ScheduleNewAnalysis is a method of Findit, is that it calls a number of Findit methods (CheckPolicy, _NeedsNewAnalysis, client_id) and therefore already needs to take a Findit object as an argument. Thus, conceptually it makes sense for ScheduleNewAnalysis to also be a method on Findit.

Comment 3 by wrengr@chromium.org, Oct 27 2016

Status: Started (was: Assigned)

Comment 4 by wrengr@chromium.org, Oct 27 2016 

Fixed by moving ScheduleNewAnalysis to crash_handler.py, the only place it's used. https://codereview.chromium.org/2455053004/
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 3 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra.git/+/b74c6a6cd22bfc0e9ed6baddd089ce394644a395

commit b74c6a6cd22bfc0e9ed6baddd089ce394644a395
Author: wrengr <wrengr@chromium.org>
Date: Thu Nov 03 17:56:21 2016

Moving ScheduleNewAnalysis to break the cycle

BUG= 659345 

Review-Url: https://codereview.chromium.org/2455053004

[modify] https://crrev.com/b74c6a6cd22bfc0e9ed6baddd089ce394644a395/appengine/findit/crash/changelist_classifier.py
[modify] https://crrev.com/b74c6a6cd22bfc0e9ed6baddd089ce394644a395/appengine/findit/crash/crash_pipeline.py
[modify] https://crrev.com/b74c6a6cd22bfc0e9ed6baddd089ce394644a395/appengine/findit/crash/findit.py
[modify] https://crrev.com/b74c6a6cd22bfc0e9ed6baddd089ce394644a395/appengine/findit/crash/findit_for_chromecrash.py
[modify] https://crrev.com/b74c6a6cd22bfc0e9ed6baddd089ce394644a395/appengine/findit/crash/findit_for_clusterfuzz.py
[modify] https://crrev.com/b74c6a6cd22bfc0e9ed6baddd089ce394644a395/appengine/findit/crash/results.py
[modify] https://crrev.com/b74c6a6cd22bfc0e9ed6baddd089ce394644a395/appengine/findit/crash/test/crash_pipeline_test.py
[modify] https://crrev.com/b74c6a6cd22bfc0e9ed6baddd089ce394644a395/appengine/findit/crash/test/findit_for_chromecrash_test.py
[modify] https://crrev.com/b74c6a6cd22bfc0e9ed6baddd089ce394644a395/appengine/findit/crash/test/findit_test.py
[modify] https://crrev.com/b74c6a6cd22bfc0e9ed6baddd089ce394644a395/appengine/findit/handlers/crash/crash_handler.py
[modify] https://crrev.com/b74c6a6cd22bfc0e9ed6baddd089ce394644a395/appengine/findit/handlers/crash/test/crash_handler_test.py
[modify] https://crrev.com/b74c6a6cd22bfc0e9ed6baddd089ce394644a395/appengine/findit/lib/gitiles/change_log.py

Comment 6 by wrengr@chromium.org, Nov 3 2016

Status: Fixed (was: Started)
►

Sign in to add a comment