bug 881561 - correlations service should not error for missing correl…

…ations report, r=rhelmer

socorro/external/http/correlations.py

@@ -18,6 +18,10 @@ class DownloadError(Exception):
     pass
 
 
+class NotFoundError(Exception):
+    pass
+
+
 def file_age(f):
     return int(time.time() - os.stat(f)[stat.ST_MTIME])
 
@@ -41,7 +45,11 @@ def get(self, **kwargs):
         ]
         params = external_common.parse_arguments(filters, kwargs)
 
-        content = self._get_content(params)
+        try:
+            content = self._get_content(params)
+        except NotFoundError, msg:
+            self.config.logger.info('Failed to download %s' % msg)
+            return
 
         data = self._parse_content(
             content,
@@ -107,6 +115,8 @@ def _download(url_start):
             response = requests.get(url_start + '.txt.gz', verify=False)
             if response.status_code == 200:
                 return response.content
+            elif response.status_code == 404:
+                raise NotFoundError(url_start + '(.txt|.txt.gz)')
             else:
                 raise DownloadError(url_start + '(.txt|.txt.gz)')
 

socorro/unittest/external/http/test_correlations.py

@@ -27,17 +27,26 @@ def __init__(self, content=None, status_code=200):
 
 class TestCorrelations(unittest.TestCase):
 
-    @staticmethod
-    def _get_model(overrides):
+    def setUp(self):
+        self.temp_dirs = []
+
+    def tearDown(self):
+        for temp_dir in self.temp_dirs:
+            shutil.rmtree(temp_dir)
+
+    def _get_model(self, overrides):
+        new_temp_dir = tempfile.mkdtemp()
+        self.temp_dirs.append(new_temp_dir)
         config_values = {
             'base_url': 'http://crashanalysis.com',
-            'save_root': '',
+            'save_root': new_temp_dir,
             'save_download': True,
             'save_seconds': 1000,
         }
         config_values.update(overrides)
         cls = correlations.Correlations
         config = DotDict()
+        config.logger = mock.Mock()
         config.http = DotDict()
         config.http.correlations = DotDict(config_values)
         return cls(config)
@@ -71,6 +80,86 @@ def mocked_get(url, **kwargs):
         self.assertEqual(result['reason'], 'EXCEPTION_ACCESS_VIOLATION_READ')
         self.assertEqual(len(result['load'].splitlines()), 17)
 
+    @mock.patch('requests.get')
+    def test_failing_download_no_error(self, rget):
+
+        def mocked_get(url, **kwargs):
+            return Response('', 404)
+
+        rget.side_effect = mocked_get
+
+        model = self._get_model({
+            'base_url': 'http://doesntmatter/',
+        })
+
+        base_params = {
+            'platform': 'Windows NT',
+            'product': 'Firefox',
+            'report_type': 'core-counts',
+            'version': '24.0a2',
+        }
+        signature = 'js::types::IdToTypeId(int)'
+        result = model.get(**dict(base_params, signature=signature))
+        self.assertEqual(result, None)
+
+    @mock.patch('requests.get')
+    def test_failing_download_should_not_cached(self, rget):
+
+        calls = []
+
+        def mocked_get(url, **kwargs):
+            calls.append(url)
+            # the first two calls should fail
+            # (it's two because of the .txt and the .txt.gz attempt)
+            if len(calls) < 3:
+                return Response('', 404)
+            else:
+                return Response(SAMPLE_CORE_COUNTS)
+
+        rget.side_effect = mocked_get
+
+        model = self._get_model({
+            'base_url': 'http://doesntmatter/',
+            'save_download': True,
+        })
+
+        base_params = {
+            'platform': 'Windows NT',
+            'product': 'Firefox',
+            'report_type': 'core-counts',
+            'version': '24.0a1',
+        }
+        signature = 'js::types::IdToTypeId(int)'
+        result = model.get(**dict(base_params, signature=signature))
+        self.assertEqual(result, None)
+        # let's pretend we wait a while and try again, then it shouldn't
+        # have cached the second time
+        result = model.get(**dict(base_params, signature=signature))
+        # See sample-core-counts.txt why I chose these tests
+        self.assertEqual(result['count'], 2551)
+
+    @mock.patch('requests.get')
+    def test_failing_download_raised_error(self, rget):
+
+        def mocked_get(url, **kwargs):
+            return Response('Crap!', 500)
+
+        rget.side_effect = mocked_get
+
+        model = self._get_model({
+            'base_url': 'http://doesntmatter/',
+        })
+
+        base_params = {
+            'platform': 'Windows NT',
+            'product': 'Firefox',
+            'report_type': 'core-counts',
+            'version': '24.0a4',
+        }
+        signature = 'js::types::IdToTypeId(int)'
+        params = dict(base_params, signature=signature)
+        self.assertRaises(correlations.DownloadError, model.get, **params)
+
     @mock.patch('requests.get')
     def test_download_signature_last_in_platform(self, rget):
         """look for a signature that is last under that platform"""
@@ -311,7 +400,7 @@ def mocked_get(url, **kwargs):
         params = {
             'product': 'Firefox',
             'report_type': 'core-counts',
-            'version': '24.0a1',
+            'version': '24.0a3',
         }
         result = model.get(**dict(params, platforms=['OS/2']))
         self.assertEqual(result['total'], 0)