Issue #3571: Use bugbug to classify issues

tests/unit/test_webhook.py

@@ -13,14 +13,13 @@
 
 import flask
 import pytest
-from requests.exceptions import HTTPError
-from requests.models import Response
+from requests.exceptions import ConnectionError
 
 import webcompat
+
 from webcompat.db import Site
 from webcompat.helpers import to_bytes
-from webcompat.webhooks import helpers
-from webcompat.webhooks.model import WebHookIssue
+from webcompat.webhooks import helpers, ml
 
 
 # The key is being used for testing and computing the signature.
@@ -455,12 +454,30 @@ def test_prepare_rejected_issue(self):
         self.assertEqual(type(actual), dict)
         self.assertEqual(actual, expected)
 
-    @patch('webcompat.webhooks.helpers.make_classification_request')
-    def test_get_issue_classification(self, mock_classification):
-        """Test that api is called only once with response code is 200."""
-        mock_classification.return_value.status_code = 200
-        helpers.get_issue_classification(12345)
-        mock_classification.assert_called_once()
+    @patch('webcompat.webhooks.ml.make_classification_request')
+    def test_get_issue_classification(self, mock_class):
+        """Make only one request if it returns 200 status code right away.
+
+        If make_classification_request function returns 200 status code,
+        make sure that get_issue_classification is not calling it again.
+        """
+        mock_class.return_value.status_code = 200
+        ml.get_issue_classification(12345)
+        mock_class.assert_called_once()
+
+    @patch('time.sleep', return_value=None)
+    @patch('webcompat.webhooks.ml.make_classification_request')
+    def test_get_issue_classification_exception(self, mock_class, mock_time):
+        """Poll bugbug and raise an exception if request limit exceeded
+
+        If make_classification_request function returns 202 status code,
+        call get_issue_classification again until exception occurs.
+        """
+        mock_class.return_value.status_code = 202
+        with pytest.raises(ConnectionError):
+            ml.get_issue_classification(12345)
+
+        assert mock_class.call_count == 4
 
 
 if __name__ == '__main__':

tests/unit/test_webhook_model.py

@@ -13,7 +13,6 @@
 
 import pytest
 from requests.exceptions import HTTPError
-from requests.models import Response
 
 import webcompat
 from tests.unit.test_webhook import event_data
@@ -76,7 +75,6 @@ def test_model_instance():
 @patch('webcompat.webhooks.model.make_request')
 def test_close_private_issue(mock_mr):
     """Test issue state and API request that is sent to GitHub."""
-    mock_mr.return_value.status_code = 200
     json_event, signature = event_data('private_issue_opened.json')
     payload = json.loads(json_event)
     issue = WebHookIssue.from_dict(payload)
@@ -105,7 +103,6 @@ def test_close_private_issue_fails(mock_mr):
 @patch('webcompat.webhooks.model.make_request')
 def test_comment_public_uri(mock_mr):
     """Test issue state and API request that is sent to GitHub."""
-    mock_mr.return_value.status_code = 200
     json_event, signature = event_data('private_issue_opened.json')
     payload = json.loads(json_event)
     issue = WebHookIssue.from_dict(payload)
@@ -120,7 +117,6 @@ def test_comment_public_uri(mock_mr):
 @patch('webcompat.webhooks.model.make_request')
 def test_comment_closed_reason(mock_mr):
     """Test comment API request that is sent to GitHub."""
-    mock_mr.return_value.status_code = 200
     json_event, signature = event_data('private_issue_opened.json')
     payload = json.loads(json_event)
     issue = WebHookIssue.from_dict(payload)
@@ -139,7 +135,6 @@ def test_comment_closed_reason(mock_mr):
 @patch('webcompat.webhooks.model.make_request')
 def test_moderate_public_issue(mock_mr):
     """Test issue state and API request that is sent to GitHub."""
-    mock_mr.return_value.status_code = 200
     json_event, signature = event_data('private_issue_opened.json')
     payload = json.loads(json_event)
     issue = WebHookIssue.from_dict(payload)
@@ -156,7 +151,6 @@ def test_moderate_public_issue(mock_mr):
 @patch('webcompat.webhooks.model.make_request')
 def test_closing_public_issues(mock_mr):
     """Test issue state and API request that is sent to GitHub."""
-    mock_mr.return_value.status_code = 200
     json_event, signature = event_data('private_issue_opened.json')
     payload = json.loads(json_event)
     issue = WebHookIssue.from_dict(payload)
@@ -211,7 +205,6 @@ def test_get_public_issue_number():
 @patch('webcompat.webhooks.model.make_request')
 def test_tag_as_public(mock_mr):
     """Test tagging an issue as public."""
-    mock_mr.return_value.status_code = 200
     json_event, signature = event_data('new_event_valid.json')
     payload = json.loads(json_event)
     issue = WebHookIssue.from_dict(payload)
@@ -317,7 +310,6 @@ def test_process_issue_action_scenarios(mock_mr, mock_classification):
         ('private_issue_opened.json', comment_added),
         ('public_milestone_needscontact.json', outreach_comment_added)
     ]
-    mock_mr.return_value.status_code = 200
     mock_classification.return_value = (
         {'prob': [0.03385603427886963, 0.9661439657211304], 'class': 1}
     )
@@ -424,7 +416,6 @@ def test_process_issue_action_not_closed_scenarios(mock_close, mock_mr, mock_cla
 @patch('webcompat.webhooks.model.make_request')
 def test_classify_issue_probability_high(mock_mr, mock_classification):
     """Test classifying an issue and adding a label."""
-    mock_mr.return_value.status_code = 200
     mock_classification.return_value = (
         {'prob': [0.03385603427886963, 0.9761439657211304], 'class': 1}
     )
@@ -474,3 +465,18 @@ def test_classify_issue_needsdiagnosis_true(mock_mr, mock_classification):
     issue = WebHookIssue.from_dict(payload)
     issue.classify()
     mock_mr.assert_not_called()
+
+
+@patch('webcompat.webhooks.ml.make_classification_request')
+@patch('webcompat.webhooks.model.make_request')
+def test_classify_issue_service_exception(mock_mr, mock_classification, caplog):        # noqa
+    """Test that ml server error exception handled gracefully."""
+    caplog.set_level(logging.INFO)
+    mock_classification.side_effect = HTTPError()
+    json_event, signature = event_data('private_issue_opened.json')
+    payload = json.loads(json_event)
+    issue = WebHookIssue.from_dict(payload)
+    with webcompat.app.test_request_context():
+        rv = issue.process_issue_action()
+        assert rv == oops
+        assert 'classification failed' in caplog.text

webcompat/webhooks/helpers.py

@@ -10,8 +10,6 @@
 import json
 import logging
 import re
-import requests
-import time
 
 from webcompat import app
 from webcompat.db import Site
@@ -34,8 +32,6 @@
 IOS_BROWSERS = ['browser-firefox-ios', ]
 PUBLIC_REPO = app.config['ISSUES_REPO_URI']
 PRIVATE_REPO = app.config['PRIVATE_REPO_URI']
-BUGBUG_HTTP_SERVER = app.config['BUGBUG_HTTP_SERVER']
-CLASSIFIER_PATH = app.config['CLASSIFIER_PATH']
 
 
 def extract_metadata(body):
@@ -234,35 +230,3 @@ def prepare_rejected_issue():
     payload_request['state'] = 'closed'
     payload_request['milestone'] = invalid_id
     return payload_request
-
-
-def make_classification_request(issue_number):
-    url = f"{BUGBUG_HTTP_SERVER}/{CLASSIFIER_PATH}/{issue_number}"
-    headers = {"X-Api-Key": "webcompat"}
-    response = requests.get(url, headers=headers)
-    response.raise_for_status()
-    return response
-
-
-def get_issue_classification(issue_number, retry_count=4, retry_sleep=3):
-    """Get issue classification from bugbug.
-
-    As classification happens in the background we need to make a second
-    request to get results.
-
-    The service returns 202 status if request is still in process
-    and 200 status if the issue is classified
-    """
-    for _ in range(retry_count):
-        response = make_classification_request(issue_number)
-
-        if response.status_code == 202:
-            time.sleep(retry_sleep)
-        else:
-            break
-    else:
-        total_sleep = retry_count * retry_sleep
-        msg = f"Couldn't classify issue {issue_number} in {total_sleep} seconds, aborting"  # noqa
-        raise Exception(msg)
-
-    return response.json()

webcompat/webhooks/ml.py

@@ -0,0 +1,50 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+"""Helpers methods for machine learning classification."""
+
+import requests
+import time
+
+from requests.exceptions import ConnectionError
+
+from webcompat import app
+
+BUGBUG_HTTP_SERVER = app.config['BUGBUG_HTTP_SERVER']
+CLASSIFIER_PATH = app.config['CLASSIFIER_PATH']
+
+
+def make_classification_request(issue_number):
+    """Make a request to bugbug http service."""
+    url = f"{BUGBUG_HTTP_SERVER}/{CLASSIFIER_PATH}/{issue_number}"
+    headers = {"X-Api-Key": "webcompat"}
+    response = requests.get(url, headers=headers)
+    response.raise_for_status()
+    return response
+
+
+def get_issue_classification(issue_number, retry_count=4, retry_sleep=3):
+    """Get issue classification from bugbug.
+
+    As classification happens in the background we need to make a second
+    request to get results, so we're polling the endpoint.
+
+    The service returns 202 status if request is still in process
+    and 200 status if the issue is classified
+    """
+    for _ in range(retry_count):
+        response = make_classification_request(issue_number)
+
+        if response.status_code == 202:
+            time.sleep(retry_sleep)
+        else:
+            break
+    else:
+        total_sleep = retry_count * retry_sleep
+        msg = f"Couldn't classify issue {issue_number} in {total_sleep} seconds, aborting"  # noqa
+        raise ConnectionError(msg)
+
+    return response.json()

webcompat/webhooks/model.py

@@ -7,12 +7,9 @@
 """WebCompat Issue Model for webhooks."""
 
 from dataclasses import dataclass
-from dataclasses import field
-from typing import Any
-from typing import Dict
 from typing import List
 
-from requests.exceptions import HTTPError
+from requests.exceptions import HTTPError, ConnectionError
 
 from webcompat import app
 from webcompat.webhooks.helpers import extract_metadata
@@ -23,7 +20,7 @@
 from webcompat.webhooks.helpers import oops
 from webcompat.webhooks.helpers import prepare_rejected_issue
 from webcompat.webhooks.helpers import repo_scope
-from webcompat.webhooks.helpers import get_issue_classification
+from webcompat.webhooks.ml import get_issue_classification
 from webcompat.issues import moderation_template
 
 PUBLIC_REPO = app.config['ISSUES_REPO_URI']
@@ -303,7 +300,7 @@ def process_issue_action(self):
 
             try:
                 self.classify()
-            except HTTPError as e:
+            except (HTTPError, ConnectionError) as e:
                 msg_log(f'classification failed ({e})', self.number)
                 return oops()