Conduit: bugzilla.py module for accessing BMO's API to add attachments (bug 1347930) r?mars draft
authorDavid Lawrence <dkl@mozilla.com>
Mon, 20 Mar 2017 16:06:52 -0400
changeset 5531 33c3a1ca22d8076c901e3d86e0bb260e6b35f74c
parent 5516 76ec14c00783ad4d9634a10bb69cbd6ed915c5c8
child 5532 2e59a5f9a1aa54a9356cc50c6ed1f15559fe6995
push id172
push userdlawrence@mozilla.com
push dateMon, 20 Mar 2017 20:20:59 +0000
reviewersmars
bugs1347930
Conduit: bugzilla.py module for accessing BMO's API to add attachments (bug 1347930) r?mars - Updated to fix review comments MozReview-Commit-ID: 9VysGD61IG6
commitindex/commitindex/reviews/bugzilla.py
commitindex/tests/test_bmo_attachments.py
--- a/commitindex/commitindex/reviews/bugzilla.py
+++ b/commitindex/commitindex/reviews/bugzilla.py
@@ -1,38 +1,60 @@
 # 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/.
 
-# TODO:
-# 1. Load REST URL from system wide config
-# 2. New content_type for conduit attachments?
-# 3. Add comment_tags for conduit attachments?
-
 """Interface to a Bugzilla system."""
 
 
 from urllib.parse import quote
 import json
 import logging
 import requests
 
 
+logger = logging.getLogger(__name__)
+
+
 class Bugzilla(object):
     """
     Interface to a Bugzilla system.
+
+    TODO:
+    1. Load REST URL from system wide config
+    2. New content_type for conduit attachments?
+    3. Add comment_tags for conduit attachments?
     """
 
     def __init__(self, rest_url=None):
         self.rest_url = rest_url
         self.session = requests.Session()
-        self.logger = logging.getLogger(__name__)
 
     def call(self, method, path, data=None):
-        """Perform REST API call and decode JSON"""
+        """Perform REST API call and decode JSON.i
+
+        Generic call function that performs a REST API call to the
+        Bugzilla system and turns the JSON data returned into a
+        Python data object.
+
+        Args:
+            method: Request method such as GET/POST/PUT...
+            path: The resource path of the REST call.
+            data: Optional data for the POST method.
+
+        Returns:
+            A Python object, normally a dict, containing the converted
+            JSON data.
+
+        Raises:
+            BugzillaError: General error such as invalid JSON or Bugzilla
+            returned an error of its own. The code in the latter case will
+            pertain to the specific error code generated by Bugzilla.
+        """
+
         headers = {
             'Accept': 'application/json',
             'Content-Type': 'application/json'
         }
 
         if method == 'GET':
             response = self.session.get(self.rest_url + path, headers=headers)
 
@@ -40,76 +62,104 @@ class Bugzilla(object):
             response = self.session.post(self.rest_url + path, json=data,
                                          headers=headers)
 
         try:
             data = json.loads(response.content.decode('utf-8'))
         except:
             raise BugzillaError(400, "Error decoding JSON data")
 
-        if isinstance(data, dict) and 'error' in data and data['error']:
-            raise BugzillaError(data['code'], data['message'])
+        if 'error' in data:
+            raise BugzillaError(data['message'], data['code'])
 
         return data
 
     def is_bug_confidential(self, bug_id):
-        """Check if bug is confidential"""
-        self.logger.info('Checking if bug %d is confidential.', bug_id)
+        """Check if bug is confidential
+
+        Simple REST call checking if a given bug id is private or not.
+
+        Params:
+            bug_id: Integer ID of the bug to check.
+
+        Returns:
+            True if bug is private, False if public.
+
+        Raises:
+            BugzillaError: General error where the fault code and string will
+            pertain to the specific error code generated by Bugzilla.
+        """
+
         try:
             self.call('GET', '/bug/' + quote(str(bug_id)))
         except BugzillaError as error:
             if error.fault_code == 102:
                 return True
-        except:
-            raise BugzillaError(error.fault_code, error.fault_string)
+            raise BugzillaError(error.fault_string, error.fault_code)
 
         return False
 
     def valid_api_key(self, username, api_key):
-        """Check if API key is valid for specific username"""
-        self.logger.info('Checking valid API key for %s.', username)
+        """Check if API key is valid for specific username
+
+        Simple REST call to check if a given API key for a specified user
+        is a valid login.
+
+        Params:
+            username: The Bugzilla login for the user, normally their email
+            address.
+            api_key: The 40 character API key for the user.
+
+        Returns:
+            True if the api_key and username pair are a valid login,
+            False if nota
+
+        Raises:
+            BugzillaError: General error where the fault code and string will
+            pertain to the specific error code generated by Bugzilla.
+        """
+
         try:
             self.call('GET', '/valid_login?login=' + quote(username) +
                       '&api_key=' + quote(api_key))
         except BugzillaError as error:
             if error.fault_code == 306:
                 return False
-        except:
-            raise BugzillaError(error.fault_code, error.fault_string)
+            raise BugzillaError(error.fault_string, error.fault_code)
 
         return True
 
     def create_attachment(self, bug_id, attach_data, api_key=None):
         """Create the attachment using the provided flags.
 
-        The `flags` parameter is an array of flags to set/update/clear.  This
-        array matches the Bugzilla flag API:
-        Setting:
-            {
-                'id': flag.id
-                'name': 'review',
-                'status': '?',
-                'requestee': reviewer.email
-            }
-        Clearing:
-            {
-                'id': flag.id,
-                'status': 'X'
-            }
+        Create a single attachment in Bugzilla using the REST API.
+
+        Params:
+            http://bmo.readthedocs.io/en/latest/api/core/v1/attachment.html#create-attachment
+
+        Returns:
+            Integer ID for new Bugzilla attachment.
+
+        Raises:
+            BugzillaError: General error where the fault code and string will
+            pertain to the specific error code generated by Bugzilla.
         """
 
-        self.logger.info('Posting attachment to bug %d.', bug_id)
-
         try:
             result = self.call('POST', '/bug/' + quote(str(bug_id)) +
                                '/attachment?api_key=' + quote(str(api_key)),
                                attach_data)
         except BugzillaError as error:
-            print('code: %d string: %s' % (error.fault_code, error.fault_string))
-            return None
+            logger.warning(
+                {
+                    'msg': error.fault_string,
+                    'code': error.fault_code
+                }, 'app.warning'
+            )
+            raise BugzillaError(error.fault_string, error.fault_code)
 
         return int(list(result['attachments'].keys())[0])
 
 
 class BugzillaError(Exception):
     """Generic Bugzilla Exception"""
     def __init__(self, msg, code=None):
         super(BugzillaError, self).__init__(msg)
--- a/commitindex/tests/test_bmo_attachments.py
+++ b/commitindex/tests/test_bmo_attachments.py
@@ -1,34 +1,38 @@
 # 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/.
+
 """
-Test repository handling logic.
+Mountebank test cases for commit-index
 """
 
 from commitindex.reviews.bugzilla import Bugzilla
 from testing import MountebankClient
 
 import pytest
 
 
 class FakeBugzilla:
     """Setups up the imposter test double emulating Bugzilla"""
     def __init__(self, mountebank_client):
+
         self.mountebank = mountebank_client
 
     @property
     def url(self):
         """Return fully qualified url for server"""
+
         # Copied from the project docker-compose.yml file.
-        return 'http://172.17.0.2:' + str(self.mountebank.imposter_port)
+        return 'http://mountebank:' + str(self.mountebank.imposter_port)
 
     def create_attachment(self, bug_id):
         """Create a attachment in the fake bugzilla server."""
+
         path = '/bug/' + str(bug_id) + '/attachment'
         self.mountebank.create_stub(
             [
                 {
                     "predicates": [{
                         "equals": {
                             "method": "POST",
                             "headers": {
@@ -68,57 +72,60 @@ class FakeBugzilla:
                 }
             ]
         )
 
 
 @pytest.fixture(scope='session')
 def mountebank():
     """Returns configured Mounteback client instance"""
+
     # The docker-compose internal DNS entry for the mountebank container
-    mountebank_host = "172.17.0.2"
+    mountebank_host = "mountebank"
     # Lifted from the docker-compose file
     mountebank_admin_port = 2525
     mountebank_imposter_port = 4000
 
     return MountebankClient(
         mountebank_host, mountebank_admin_port, mountebank_imposter_port
     )
 
 
 @pytest.fixture
 def bugzilla(request, mountebank):
     """Returns emulated Bugzilla service methods"""
+
     # NOTE: comment out the line below if you want mountebank to save your
     # requests and responses for inspection after the test suite completes.
     # You can manually clean up the imposters afterwards by sending HTTP
     # DELETE to the exposed mountebank admin port, documented in
     # docker-compose.yml, or by restarting the mountebank container. See
     # http://www.mbtest.org/docs/api/stubs for details.
     request.addfinalizer(mountebank.reset_imposters)
     return FakeBugzilla(mountebank)
 
 
 @pytest.mark.bugzilla
 def test_create_valid_attachment(bugzilla):
     """Tests adding an attachment to the Bugzilla service"""
+
     attach_data = {
-        "is_patch" : False,
-        "comment" : "This is a new attachment comment",
-        "summary" : "Test Attachment",
-        "content_type" : "text/plain",
-        "data" : "data to be encoded",
-        "file_name" : "test_attachment.patch",
-        "is_private" : False,
-        "flags" : [
+        "is_patch": False,
+        "comment": "This is a new attachment comment",
+        "summary": "Test Attachment",
+        "content_type": "text/plain",
+        "data": "data to be encoded",
+        "file_name": "test_attachment.patch",
+        "is_private": False,
+        "flags": [
             {
-                "name" : "review",
-                "status" : "?",
-                "requestee" : "dkl@mozilla.com",
-                "new" : True
+                "name": "review",
+                "status": "?",
+                "requestee": "dkl@mozilla.com",
+                "new": True
             }
         ]
     }
 
     bugzilla.create_attachment(1234)
     bug_test = Bugzilla(rest_url=bugzilla.url)
     result = bug_test.create_attachment(1234, attach_data, '12345')
-    assert isinstance(result, int)
+    assert result == 12345