| OLD | NEW |
| 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 Loading... |
| 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 Loading... |
| 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 Loading... |
| 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) |
| OLD | NEW |