[Tracker-discuss] [issue613] Commits notifications don't show up on bugs.p.o

[Berker Peksag]

Berker Peksag added the comment:

Here is a patch that tweaks some exception messages and adds logging.error() calls in GitHubHandler class.

_______________________________________________________
PSF Meta Tracker <metatracker at psf.upfronthosting.co.za>
<http://psf.upfronthosting.co.za/roundup/meta/issue613>
_______________________________________________________
-------------- next part --------------
diff --git a/roundup/github.py b/roundup/github.py
--- a/roundup/github.py
+++ b/roundup/github.py
@@ -41,20 +41,21 @@ class GitHubHandler:
 
     def dispatch(self):
         try:
             self.verify_request()
             self.validate_webhook_secret()
             self.extract()
         except (Unauthorised, MethodNotAllowed,
                 UnsupportedMediaType, Reject) as err:
+            logging.error(err, exc_info=True)
             raise
         except Exception as err:
             logging.error(err, exc_info=True)
-            raise Reject()
+            raise Reject(err)
 
     def extract(self):
         """
         This method is responsible for extracting information from GitHub event
         and decide what to do with it more. Currently it knows how to handle
         pull requests and comments.
         """
         event = self.get_event()
@@ -77,18 +78,19 @@ class GitHubHandler:
         """
         Validates request signature against SECRET_KEY environment variable.
         This verification is based on HMAC hex digest calculated from the sent
         payload. The value of SECRET_KEY should be exactly the same as the one
         configured in GitHub webhook secret field.
         """
         key = os.environ.get('SECRET_KEY')
         if key is None:
-            logging.error('Missing SECRET_KEY environment variable set!')
-            raise Reject()
+            msg = 'Missing SECRET_KEY environment variable set!'
+            logging.error(msg)
+            raise Reject(msg)
         data = str(self.form.value)
         signature = 'sha1=' + hmac.new(key, data,
                                        hashlib.sha1).hexdigest()
         header_signature = self.request.headers.get('X-Hub-Signature', '')
         if not compare_digest(signature, header_signature):
             raise Unauthorised()
 
     def verify_request(self):
@@ -97,17 +99,19 @@ class GitHubHandler:
         """
         method = self.env.get('REQUEST_METHOD', None)
         if method != 'POST':
             raise MethodNotAllowed()
         content_type = self.env.get('CONTENT_TYPE', None)
         if content_type != 'application/json':
             raise UnsupportedMediaType()
         if self.get_event() is None:
-            raise Reject()
+            msg = 'no X-GitHub-Event header found in the request headers'
+            logging.error(msg)
+            raise Reject(msg)
 
     def get_event(self):
         """
         Extracts GitHub event from header field.
         """
         return self.request.headers.get('X-GitHub-Event', None)
 
 
@@ -327,17 +331,17 @@ class Push(Event):
     """
     Class responsible for handling push events.
     """
 
     def get_github_username(self):
         """
         Extract GitHub username from a push event.
         """
-        return self.data.get('pusher', []).get('name', '')
+        return self.data.get('pusher', {}).get('name', '')
 
     def dispatch(self):
         """
         Main method responsible for responding to incoming GitHub event.
         """
         self.set_roundup_user()
         commits = self.data.get('commits', [])
         ref = self.data.get('ref', 'refs/heads/master')
@@ -352,17 +356,17 @@ class Push(Event):
                 if issue_id not in messages:
                     messages[issue_id] = (u'', False)
                 curr_msg, curr_close = messages[issue_id]
                 # we append the new message to the other and do binary OR
                 # on close, so that at least one information will actually
                 # close the issue
                 messages[issue_id] = (curr_msg + u'\n' + msg, curr_close or close)
         if not messages:
-            return
+            return 'zero messages created'
         for issue_id, (msg, close) in messages.iteritems():
             # add comments to appropriate issues...
             id = issue_id.encode('utf-8')
             try:
                 issue_msgs = self.db.issue.get(id, 'messages')
             except IndexError:
                 # See meta issue #613: the commit message might also include
                 # PR ids that shouldn't be included.