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

Side by Side Diff: appengine/findit/crash/findit.py

Issue 2605943002: Removing the mutation in the factories for getting dep repositories (Closed)
Patch Set: Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 # Copyright 2016 The Chromium Authors. All rights reserved. 1 # Copyright 2016 The Chromium Authors. All rights reserved.
2 # Use of this source code is governed by a BSD-style license that can be 2 # Use of this source code is governed by a BSD-style license that can be
3 # found in the LICENSE file. 3 # found in the LICENSE file.
4 4
5 import copy 5 import copy
6 import json 6 import json
7 import logging 7 import logging
8 8
9 from google.appengine.ext import ndb 9 from google.appengine.ext import ndb
10 10
11 from common import appengine_util 11 from common import appengine_util
12 from common import chrome_dependency_fetcher 12 from common.chrome_dependency_fetcher import ChromeDependencyFetcher
13 from common import constants 13 from common import constants
14 from crash.crash_report import CrashReport 14 from crash.crash_report import CrashReport
15 from libs import time_util 15 from libs import time_util
16 from model import analysis_status 16 from model import analysis_status
17 from model.crash.crash_config import CrashConfig 17 from model.crash.crash_config import CrashConfig
18 18
19 19
20 # TODO(http://crbug.com/659346): since most of our unit tests are 20 # TODO(http://crbug.com/659346): since most of our unit tests are
21 # FinditForFracas-specific, wrengr moved them to findit_for_chromecrash_test.py. 21 # FinditForFracas-specific, wrengr moved them to findit_for_chromecrash_test.py.
22 # However, now we're missing coverage for most of this file (due to the 22 # However, now we're missing coverage for most of this file (due to the
23 # buggy way coverage is computed). Need to add a bunch of new unittests 23 # buggy way coverage is computed). Need to add a bunch of new unittests
24 # to get coverage back up. 24 # to get coverage back up.
25 25
26 # TODO: this class depends on ndb stuff, and should therefore move to 26 # TODO: this class depends on ndb stuff, and should therefore move to
27 # cr-culprit-finder/service/predator as part of the big reorganization. 27 # cr-culprit-finder/service/predator as part of the big reorganization.
28 # This class should be renamed to avoid confustion between Findit and Predator. 28 # This class should be renamed to avoid confustion between Findit and Predator.
29 # Think of a good name (e.g.'PredatorApp') for this class. 29 # Think of a good name (e.g.'PredatorApp') for this class.
30 class Findit(object): 30 class Findit(object):
31 def __init__(self, repository): 31 def __init__(self, get_repository):
32 """ 32 """
33 Args: 33 Args:
34 repository (Repository): the Git repository for getting CLs to classify. 34 get_repository (callable): a function from DEP urls to ``Repository``
35 objects, so we can get changelogs and blame for each dep. Notably,
36 to keep the code here generic, we make no assumptions about
37 which subclass of ``Repository`` this function returns. Thus,
38 it is up to the caller to decide what class to return and handle
39 any other arguments that class may require (e.g., an http client
40 for ``GitilesRepository``).
35 """ 41 """
36 self._repository = repository 42 self._get_repository = get_repository
43 self._dep_fetcher = ChromeDependencyFetcher(get_repository)
37 # TODO(http://crbug.com/659354): because self.client is volatile, 44 # TODO(http://crbug.com/659354): because self.client is volatile,
38 # we need some way of updating the Azelea instance whenever the 45 # we need some way of updating the Predator instance whenever the
39 # config changes. How to do that cleanly? 46 # config changes. How to do that cleanly?
40 self._predator = None 47 self._predator = None
41 self._stacktrace_parser = None 48 self._stacktrace_parser = None
42 49
43 # This is a class method because it should be the same for all 50 # This is a class method because it should be the same for all
44 # instances of this class. We can in fact call class methods on 51 # instances of this class. We can in fact call class methods on
45 # instances (not just on the class itself), so we could in principle 52 # instances (not just on the class itself), so we could in principle
46 # get by with just this method. However, a @classmethod is treated 53 # get by with just this method. However, a @classmethod is treated
47 # syntactically like a method, thus we'd need to have the ``()`` at the 54 # syntactically like a method, thus we'd need to have the ``()`` at the
48 # end, unlike for a @property. Thus we have both the class method and 55 # end, unlike for a @property. Thus we have both the class method and
(...skipping 153 matching lines...) Expand 10 before | Expand all | Expand 10 after
202 Args: 209 Args:
203 model (CrashAnalysis): The model containing the stack_trace string 210 model (CrashAnalysis): The model containing the stack_trace string
204 to be parsed. 211 to be parsed.
205 212
206 Returns: 213 Returns:
207 On success, returns a Stacktrace object; on failure, returns None. 214 On success, returns a Stacktrace object; on failure, returns None.
208 """ 215 """
209 # Use up-to-date ``top_n`` in self.config to filter top n frames. 216 # Use up-to-date ``top_n`` in self.config to filter top n frames.
210 stacktrace = self._stacktrace_parser.Parse( 217 stacktrace = self._stacktrace_parser.Parse(
211 model.stack_trace, 218 model.stack_trace,
212 chrome_dependency_fetcher.ChromeDependencyFetcher( 219 self._dep_fetcher.GetDependency(model.crashed_version, model.platform),
213 self._repository).GetDependency( 220 model.signature,
214 model.crashed_version, 221 self.config.get('top_n'))
215 model.platform),
216 model.signature, self.config.get('top_n'))
217 if not stacktrace: 222 if not stacktrace:
218 logging.warning('Failed to parse the stacktrace %s', model.stack_trace) 223 logging.warning('Failed to parse the stacktrace %s', model.stack_trace)
219 return None 224 return None
220 225
221 return stacktrace 226 return stacktrace
222 227
223 def ProcessResultForPublishing(self, result, key): 228 def ProcessResultForPublishing(self, result, key):
224 """Client specific processing of result data for publishing.""" 229 """Client specific processing of result data for publishing."""
225 raise NotImplementedError() 230 raise NotImplementedError()
226 231
(...skipping 39 matching lines...) Expand 10 before | Expand all | Expand 10 after
266 stacktrace = self.ParseStacktrace(model) 271 stacktrace = self.ParseStacktrace(model)
267 if stacktrace is None: 272 if stacktrace is None:
268 return None 273 return None
269 274
270 return self._predator.FindCulprit(CrashReport( 275 return self._predator.FindCulprit(CrashReport(
271 crashed_version = model.crashed_version, 276 crashed_version = model.crashed_version,
272 signature = model.signature, 277 signature = model.signature,
273 platform = model.platform, 278 platform = model.platform,
274 stacktrace = stacktrace, 279 stacktrace = stacktrace,
275 regression_range = model.regression_range)) 280 regression_range = model.regression_range))
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698