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

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

Issue 2673993003: [Predator] Pass config as argument to findit. (Closed)
Patch Set: Fix pylint. Created 3 years, 10 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
« no previous file with comments | « no previous file | appengine/findit/crash/findit.py » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 crash import monitoring 9 from crash import monitoring
10 10
11 from common import appengine_util 11 from common import appengine_util
12 from common import pubsub_util 12 from common import pubsub_util
13 from common.pipeline_wrapper import BasePipeline 13 from common.pipeline_wrapper import BasePipeline
14 from common.pipeline_wrapper import pipeline 14 from common.pipeline_wrapper import pipeline
15 from crash import findit_for_chromecrash 15 from crash import findit_for_chromecrash
16 from crash import findit_for_clusterfuzz 16 from crash import findit_for_clusterfuzz
17 from crash.type_enums import CrashClient 17 from crash.type_enums import CrashClient
18 from libs import time_util 18 from libs import time_util
19 from gae_libs.gitiles.cached_gitiles_repository import CachedGitilesRepository 19 from gae_libs.gitiles.cached_gitiles_repository import CachedGitilesRepository
20 from gae_libs.http.http_client_appengine import HttpClientAppengine 20 from gae_libs.http.http_client_appengine import HttpClientAppengine
21 from model import analysis_status 21 from model import analysis_status
22 from model.crash.crash_config import CrashConfig
22 23
23 24
24 # TODO(http://crbug.com/659346): write complete coverage tests for this. 25 # TODO(http://crbug.com/659346): write complete coverage tests for this.
25 def FinditForClientID(client_id, get_repository): # pragma: no cover 26 def FinditForClientID(client_id, get_repository, config): # pragma: no cover
26 """Construct a Findit object from a client id string specifying the class. 27 """Construct a Findit object from a client id string specifying the class.
27 28
28 We cannot pass Findit objects to the various methods in 29 We cannot pass Findit objects to the various methods in
29 ``crash.crash_pipeline``, because they are not JSON serializable. For 30 ``crash.crash_pipeline``, because they are not JSON serializable. For
30 now, we just serialize Findit objects as their ``client_id``, and then 31 now, we just serialize Findit objects as their ``client_id``, and then
31 use this function to reconstruct them. Alas, this means we will lose 32 use this function to reconstruct them. Alas, this means we will lose
32 various other information stored in the Findit object (i.e., stuff that 33 various other information stored in the Findit object (i.e., stuff that
33 comes from CrashConfig); which could lead to some hard-to-diagnose 34 comes from CrashConfig); which could lead to some hard-to-diagnose
34 coherency bugs, since the new Findit object will be based on the 35 coherency bugs, since the new Findit object will be based on the
35 CrashConfig at the time it's constructed, which may be different 36 CrashConfig at the time it's constructed, which may be different
36 than the CrashConfig at the time the previous Findit object was 37 than the CrashConfig at the time the previous Findit object was
37 constructed. In the future we should fix all this to serialize Findit 38 constructed. In the future we should fix all this to serialize Findit
38 objects in a more robust way. 39 objects in a more robust way.
39 """ 40 """
40 assert isinstance(client_id, (str, unicode)), ( 41 assert isinstance(client_id, (str, unicode)), (
41 'FinditForClientID: expected string or unicode, but got %s' 42 'FinditForClientID: expected string or unicode, but got %s'
42 % client_id.__class__.__name__) 43 % client_id.__class__.__name__)
43 # TODO(wrengr): it'd be nice to replace this with a single lookup in 44 # TODO(wrengr): it'd be nice to replace this with a single lookup in
44 # a dict; but that's buggy for some unknown reason. 45 # a dict; but that's buggy for some unknown reason.
45 if client_id == CrashClient.FRACAS: 46 if client_id == CrashClient.FRACAS:
46 cls = findit_for_chromecrash.FinditForFracas 47 cls = findit_for_chromecrash.FinditForFracas
47 elif client_id == CrashClient.CRACAS: # pragma: no cover 48 elif client_id == CrashClient.CRACAS: # pragma: no cover
48 cls = findit_for_chromecrash.FinditForCracas 49 cls = findit_for_chromecrash.FinditForCracas
49 elif client_id == CrashClient.CLUSTERFUZZ: # pragma: no cover 50 elif client_id == CrashClient.CLUSTERFUZZ: # pragma: no cover
50 cls = findit_for_clusterfuzz.FinditForClusterfuzz 51 cls = findit_for_clusterfuzz.FinditForClusterfuzz
51 else: # pragma: no cover 52 else: # pragma: no cover
52 raise ValueError('FinditForClientID: ' 53 raise ValueError('FinditForClientID: '
53 'unknown or unsupported client %s' % client_id) 54 'unknown or unsupported client %s' % client_id)
54 55
55 return cls(get_repository) 56 return cls(get_repository, config)
56 57
57 58
58 # Some notes about the classes below, for people who are not familiar 59 # Some notes about the classes below, for people who are not familiar
59 # with AppEngine pipelines: 60 # with AppEngine pipelines:
60 # 61 #
61 # The pipeline library is designed in a strange way which requires that 62 # The pipeline library is designed in a strange way which requires that
62 # all the ``__init__`` and ``run`` methods in this file take the exact 63 # all the ``__init__`` and ``run`` methods in this file take the exact
63 # same arguments. This arises from the interaction between a few 64 # same arguments. This arises from the interaction between a few
64 # different constraints. First, for any given pipeline, its ``__init__`` 65 # different constraints. First, for any given pipeline, its ``__init__``
65 # and ``run`` must take the same arguments. Second, all the objects that 66 # and ``run`` must take the same arguments. Second, all the objects that
(...skipping 20 matching lines...) Expand all
86 # processes, so we'll actually end up reconstructing the ``Findit`` object 87 # processes, so we'll actually end up reconstructing the ``Findit`` object
87 # twice. This also means ``run`` can't store anything in the pipeline 88 # twice. This also means ``run`` can't store anything in the pipeline
88 # object and expect it to still be available in the ``finalized`` method. 89 # object and expect it to still be available in the ``finalized`` method.
89 90
90 class CrashBasePipeline(BasePipeline): 91 class CrashBasePipeline(BasePipeline):
91 def __init__(self, client_id, crash_identifiers): 92 def __init__(self, client_id, crash_identifiers):
92 super(CrashBasePipeline, self).__init__(client_id, crash_identifiers) 93 super(CrashBasePipeline, self).__init__(client_id, crash_identifiers)
93 self._crash_identifiers = crash_identifiers 94 self._crash_identifiers = crash_identifiers
94 self._findit = FinditForClientID( 95 self._findit = FinditForClientID(
95 client_id, 96 client_id,
96 CachedGitilesRepository.Factory(HttpClientAppengine())) 97 CachedGitilesRepository.Factory(HttpClientAppengine()),
98 CrashConfig.Get())
97 99
98 @property 100 @property
99 def client_id(self): # pragma: no cover 101 def client_id(self): # pragma: no cover
100 return self._findit.client_id 102 return self._findit.client_id
101 103
102 def run(self, *args, **kwargs): 104 def run(self, *args, **kwargs):
103 raise NotImplementedError() 105 raise NotImplementedError()
104 106
105 107
106 class CrashAnalysisPipeline(CrashBasePipeline): 108 class CrashAnalysisPipeline(CrashBasePipeline):
(...skipping 83 matching lines...) Expand 10 before | Expand all | Expand 10 after
190 if not analysis or not analysis.result or analysis.failed: 192 if not analysis or not analysis.result or analysis.failed:
191 logging.info('Can\'t publish result to %s because analysis failed:\n%s', 193 logging.info('Can\'t publish result to %s because analysis failed:\n%s',
192 self.client_id, repr(self._crash_identifiers)) 194 self.client_id, repr(self._crash_identifiers))
193 return 195 return
194 196
195 result = self._findit.GetPublishableResult(self._crash_identifiers, 197 result = self._findit.GetPublishableResult(self._crash_identifiers,
196 analysis) 198 analysis)
197 messages_data = [json.dumps(result, sort_keys=True)] 199 messages_data = [json.dumps(result, sort_keys=True)]
198 200
199 # TODO(http://crbug.com/659354): remove Findit's dependency on CrashConfig. 201 # TODO(http://crbug.com/659354): remove Findit's dependency on CrashConfig.
200 client_config = self._findit.config 202 client_config = self._findit.client_config
201 # TODO(katesonia): Clean string uses in config. 203 # TODO(katesonia): Clean string uses in config.
202 topic = client_config['analysis_result_pubsub_topic'] 204 topic = client_config['analysis_result_pubsub_topic']
203 pubsub_util.PublishMessagesToTopic(messages_data, topic) 205 pubsub_util.PublishMessagesToTopic(messages_data, topic)
204 logging.info('Published %s analysis result for %s', self.client_id, 206 logging.info('Published %s analysis result for %s', self.client_id,
205 repr(self._crash_identifiers)) 207 repr(self._crash_identifiers))
206 208
207 209
208 # TODO(http://crbug.com/659346): we misplaced the coverage test; find it! 210 # TODO(http://crbug.com/659346): we misplaced the coverage test; find it!
209 class CrashWrapperPipeline(BasePipeline): # pragma: no cover 211 class CrashWrapperPipeline(BasePipeline): # pragma: no cover
210 """Fire off pipelines to (1) do the analysis and (2) publish results. 212 """Fire off pipelines to (1) do the analysis and (2) publish results.
(...skipping 18 matching lines...) Expand all
229 N.B., due to the structure of AppEngine pipelines, this method must 231 N.B., due to the structure of AppEngine pipelines, this method must
230 accept the same arguments as are passed to ``__init__``; however, 232 accept the same arguments as are passed to ``__init__``; however,
231 because they were already passed to ``__init__`` there's no use in 233 because they were already passed to ``__init__`` there's no use in
232 recieving them here. Thus, we discard all the arguments to this method 234 recieving them here. Thus, we discard all the arguments to this method
233 (except for ``self``, naturally). 235 (except for ``self``, naturally).
234 """ 236 """
235 run_analysis = yield CrashAnalysisPipeline( 237 run_analysis = yield CrashAnalysisPipeline(
236 self._client_id, self._crash_identifiers) 238 self._client_id, self._crash_identifiers)
237 with pipeline.After(run_analysis): 239 with pipeline.After(run_analysis):
238 yield PublishResultPipeline(self._client_id, self._crash_identifiers) 240 yield PublishResultPipeline(self._client_id, self._crash_identifiers)
OLDNEW
« no previous file with comments | « no previous file | appengine/findit/crash/findit.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698