Maciej Szulik added the comment:
Brett, the most worrying thing is missing `X-Hub-Signature` header in the
request from http://psf.upfronthosting.co.za/roundup/meta/msg3281. I've
uploaded tweaked patch from Berker but I've mostly switched to logging, we
don't want to simplify hacking our tracker, too much returning exact
information what went wrong. Most importantly I've added X-GitHub-Delivery, so
it's easier to track the actual requests on GH. Additionally I've fixed author
matching if committer is GitHub, we ignore that information (this happens when
using GH UI).
_______________________________________________________
PSF Meta Tracker <metatrac...@psf.upfronthosting.co.za>
<http://psf.upfronthosting.co.za/roundup/meta/issue613>
_______________________________________________________
diff --git a/roundup/github.py b/roundup/github.py
--- a/roundup/github.py
+++ b/roundup/github.py
@@ -46,8 +46,11 @@
self.extract()
except (Unauthorised, MethodNotAllowed,
UnsupportedMediaType, Reject) as err:
+ logging.error('X-GitHub-Delivery: %s', self.get_delivery())
+ logging.error(err, exc_info=True)
raise
except Exception as err:
+ logging.error('X-GitHub-Delivery: %s', self.get_delivery())
logging.error(err, exc_info=True)
raise Reject()
@@ -70,8 +73,11 @@
handler.dispatch()
elif event == 'push':
data = json.loads(self.form.value)
- handler = Push(self.db, data)
+ handler = Push(self.db, data, self.get_delivery())
handler.dispatch()
+ else:
+ logging.error('X-GitHub-Delivery: %s', self.get_delivery())
+ logging.error('Unknown event %s', event)
def validate_webhook_secret(self):
"""
@@ -102,6 +108,8 @@
if content_type != 'application/json':
raise UnsupportedMediaType()
if self.get_event() is None:
+ logging.error('X-GitHub-Delivery: %s', self.get_delivery())
+ logging.error('no X-GitHub-Event header found in the request
headers')
raise Reject()
def get_event(self):
@@ -110,6 +118,12 @@
"""
return self.request.headers.get('X-GitHub-Event', None)
+ def get_delivery(self):
+ """
+ Extracts GitHub delivery id.
+ """
+ return self.request.headers.get('X-GitHub-Delivery', None)
+
class Event(object):
"""
@@ -328,6 +342,11 @@
Class responsible for handling push events.
"""
+ def __init__(self, db, data, delivery):
+ self.db = db
+ self.data = data
+ self.delivery = delivery
+
def get_github_username(self):
"""
Extract GitHub username from a push event.
@@ -342,7 +361,7 @@
commits = self.data.get('commits', [])
ref = self.data.get('ref', 'refs/heads/master')
# messages dictionary maps issue number to a tuple containing
- # the message to be posted as a comment an boolean flag informing
+ # the message to be posted as a comment and a boolean flag informing
# if the issue should be 'closed'
messages = {}
# extract commit messages
@@ -357,6 +376,7 @@
# close the issue
messages[issue_id] = (curr_msg + u'\n' + msg, curr_close or
close)
if not messages:
+ logging.error('zero messages created for %s', self.delivery)
return
for issue_id, (msg, close) in messages.iteritems():
# add comments to appropriate issues...
@@ -398,9 +418,14 @@
if data['issue_id'] in messages:
continue
close = bool(data.get('verb'))
- author=commit.get('author', {}).get('name', '')
- committer=commit.get('committer', {}).get('name', '')
+ author = commit.get('author', {}).get('name', '')
+ committer = commit.get('committer', {}).get('name', '')
+ # committer should not be GitHub bot, fallback to original author
+ # in these cases, this happens when using GitHub UI
+ if committer == "GitHub":
+ committer = author
author_line = committer
+ # check if the author and committer are different persons
if author != committer:
author_line = u"{} ({})".format(committer, author)
messages[data['issue_id']] = (COMMENT_TEMPLATE.format(
diff --git a/test/data/pushevent6.txt b/test/data/pushevent6.txt
new file mode 100644
--- /dev/null
+++ b/test/data/pushevent6.txt
@@ -0,0 +1,11 @@
+POST /python-dev/pull_request HTTP/1.1
+Host: 3ab1787e.ngrok.io
+Accept: */*
+User-Agent: GitHub-Hookshot/6b02022
+X-GitHub-Delivery: 2076d100-0054-11e7-9584-1096a01ee5f0
+X-GitHub-Event: push
+content-type: application/json
+X-Hub-Signature: sha1=f7557283b2cb821aafcbb5b7f44001c237795c96
+Content-Length: 8426
+
+{"ref":"refs/heads/3.6","before":"21ce65aa67f0dc63002ab0a5fb21ef921cf5279e","after":"9d07aceedabcdc9826489f8b9baffff056283bb3","created":false,"deleted":false,"forced":false,"base_ref":null,"compare":"https://github.com/python/cpython/compare/21ce65aa67f0...9d07aceedabc","commits":[{"id":"9d07aceedabcdc9826489f8b9baffff056283bb3","tree_id":"bad6e533cdf6cdf15f9a6f5cda466e52e54d57b4","distinct":true,"message":"bpo-1:
Mention coverage.py in trace module documentation (GH-435)\n\n(cherry picked
from commit
5dfccb06dc513ae67fac5fee66356ad58a4de170)","timestamp":"2017-03-03T12:58:17-08:00","url":"https://github.com/python/cpython/commit/9d07aceedabcdc9826489f8b9baffff056283bb3","author":{"name":"Brett
Cannon","email":"brettcan...@users.noreply.github.com","username":"brettcannon"},"committer":{"name":"GitHub","email":"nore...@github.com","username":"web-flow"},"added":[],"removed":[],"modified":["Doc/library/trace.rst"]}],"head_commit":{"id":"9d07aceedabcdc9826489f8b9baffff056283b
b3","tree_id":"bad6e533cdf6cdf15f9a6f5cda466e52e54d57b4","distinct":true,"message":"bpo-1:
Mention coverage.py in trace module documentation (GH-435)\n\n(cherry picked
from commit
5dfccb06dc513ae67fac5fee66356ad58a4de170)","timestamp":"2017-03-03T12:58:17-08:00","url":"https://github.com/python/cpython/commit/9d07aceedabcdc9826489f8b9baffff056283bb3","author":{"name":"Brett
Cannon","email":"brettcan...@users.noreply.github.com","username":"brettcannon"},"committer":{"name":"GitHub","email":"nore...@github.com","username":"web-flow"},"added":[],"removed":[],"modified":["Doc/library/trace.rst"]},"repository":{"id":81598961,"name":"cpython","full_name":"python/cpython","owner":{"name":"python","email":"","login":"python","id":1525981,"avatar_url":"https://avatars0.githubusercontent.com/u/1525981?v=3","gravatar_id":"","url":"https://api.github.com/users/python","html_url":"https://github.com/python","followers_url":"https://api.github.com/users/python/followers","following_url":
"https://api.github.com/users/python/following{/other_user}","gists_url":"https://api.github.com/users/python/gists{/gist_id}","starred_url":"https://api.github.com/users/python/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/python/subscriptions","organizations_url":"https://api.github.com/users/python/orgs","repos_url":"https://api.github.com/users/python/repos","events_url":"https://api.github.com/users/python/events{/privacy}","received_events_url":"https://api.github.com/users/python/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/python/cpython","description":"The
Python programming
language","fork":false,"url":"https://github.com/python/cpython","forks_url":"https://api.github.com/repos/python/cpython/forks","keys_url":"https://api.github.com/repos/python/cpython/keys{/key_id}","collaborators_url":"https://api.github.com/repos/python/cpython/collaborators{/collaborator}","teams_url":"h
ttps://api.github.com/repos/python/cpython/teams","hooks_url":"https://api.github.com/repos/python/cpython/hooks","issue_events_url":"https://api.github.com/repos/python/cpython/issues/events{/number}","events_url":"https://api.github.com/repos/python/cpython/events","assignees_url":"https://api.github.com/repos/python/cpython/assignees{/user}","branches_url":"https://api.github.com/repos/python/cpython/branches{/branch}","tags_url":"https://api.github.com/repos/python/cpython/tags","blobs_url":"https://api.github.com/repos/python/cpython/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/python/cpython/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/python/cpython/git/refs{/sha}","trees_url":"https://api.github.com/repos/python/cpython/git/trees{/sha}","statuses_url":"https://api.github.com/repos/python/cpython/statuses/{sha}","languages_url":"https://api.github.com/repos/python/cpython/languages","stargazers_url":"https://api.github.com/repos/python
/cpython/stargazers","contributors_url":"https://api.github.com/repos/python/cpython/contributors","subscribers_url":"https://api.github.com/repos/python/cpython/subscribers","subscription_url":"https://api.github.com/repos/python/cpython/subscription","commits_url":"https://api.github.com/repos/python/cpython/commits{/sha}","git_commits_url":"https://api.github.com/repos/python/cpython/git/commits{/sha}","comments_url":"https://api.github.com/repos/python/cpython/comments{/number}","issue_comment_url":"https://api.github.com/repos/python/cpython/issues/comments{/number}","contents_url":"https://api.github.com/repos/python/cpython/contents/{+path}","compare_url":"https://api.github.com/repos/python/cpython/compare/{base}...{head}","merges_url":"https://api.github.com/repos/python/cpython/merges","archive_url":"https://api.github.com/repos/python/cpython/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/python/cpython/downloads","issues_url":"https://api.g
ithub.com/repos/python/cpython/issues{/number}","pulls_url":"https://api.github.com/repos/python/cpython/pulls{/number}","milestones_url":"https://api.github.com/repos/python/cpython/milestones{/number}","notifications_url":"https://api.github.com/repos/python/cpython/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/python/cpython/labels{/name}","releases_url":"https://api.github.com/repos/python/cpython/releases{/id}","deployments_url":"https://api.github.com/repos/python/cpython/deployments","created_at":1486754631,"updated_at":"2017-03-03T19:37:47Z","pushed_at":1488574698,"git_url":"git://github.com/python/cpython.git","ssh_url":"g...@github.com:python/cpython.git","clone_url":"https://github.com/python/cpython.git","svn_url":"https://github.com/python/cpython","homepage":"https://www.python.org/","size":191699,"stargazers_count":6378,"watchers_count":6378,"language":"Python","has_issues":false,"has_downloads":true,"has_wiki":false,"has_p
ages":false,"forks_count":521,"mirror_url":null,"open_issues_count":91,"forks":521,"open_issues":91,"watchers":6378,"default_branch":"master","stargazers":6378,"master_branch":"master","organization":"python"},"pusher":{"name":"brettcannon","email":"brettcan...@users.noreply.github.com"},"organization":{"login":"python","id":1525981,"url":"https://api.github.com/orgs/python","repos_url":"https://api.github.com/orgs/python/repos","events_url":"https://api.github.com/orgs/python/events","hooks_url":"https://api.github.com/orgs/python/hooks","issues_url":"https://api.github.com/orgs/python/issues","members_url":"https://api.github.com/orgs/python/members{/member}","public_members_url":"https://api.github.com/orgs/python/public_members{/member}","avatar_url":"https://avatars0.githubusercontent.com/u/1525981?v=3","description":"Repositories
related to the Python Programming
language"},"sender":{"login":"brettcannon","id":54418,"avatar_url":"https://avatars3.githubusercontent.com/
u/54418?v=3","gravatar_id":"","url":"https://api.github.com/users/brettcannon","html_url":"https://github.com/brettcannon","followers_url":"https://api.github.com/users/brettcannon/followers","following_url":"https://api.github.com/users/brettcannon/following{/other_user}","gists_url":"https://api.github.com/users/brettcannon/gists{/gist_id}","starred_url":"https://api.github.com/users/brettcannon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/brettcannon/subscriptions","organizations_url":"https://api.github.com/users/brettcannon/orgs","repos_url":"https://api.github.com/users/brettcannon/repos","events_url":"https://api.github.com/users/brettcannon/events{/privacy}","received_events_url":"https://api.github.com/users/brettcannon/received_events","type":"User","site_admin":false},"distinct_commits":[{"id":"9d07aceedabcdc9826489f8b9baffff056283bb3","tree_id":"bad6e533cdf6cdf15f9a6f5cda466e52e54d57b4","distinct":true,"message":"bpo-1:
Mention covera
ge.py in trace module documentation (GH-435)\n\n(cherry picked from commit
5dfccb06dc513ae67fac5fee66356ad58a4de170)","timestamp":"2017-03-03T12:58:17-08:00","url":"https://github.com/python/cpython/commit/9d07aceedabcdc9826489f8b9baffff056283bb3","author":{"name":"Brett
Cannon","email":"brettcan...@users.noreply.github.com","username":"brettcannon"},"committer":{"name":"GitHub","email":"nore...@github.com","username":"web-flow"},"added":[],"removed":[],"modified":["Doc/library/trace.rst"]}],"ref_name":"3.6"}
diff --git a/test/test_github.py b/test/test_github.py
--- a/test/test_github.py
+++ b/test/test_github.py
@@ -416,6 +416,19 @@
msgs = self.db.issue.get('1', 'messages')
self.assertEqual(len(msgs), 0)
+ def testPushEvent613(self):
+ dummy_client = self._make_client('pushevent6.txt')
+ handler = GitHubHandler(dummy_client)
+ handler.dispatch()
+ msgs = self.db.issue.get('1', 'messages')
+ self.assertEqual(len(msgs), 1)
+ content = self.db.msg.get(msgs[0], 'content')
+ self.assertIn("""New changeset
9d07aceedabcdc9826489f8b9baffff056283bb3 by Brett Cannon in branch '3.6':
+bpo-1: Mention coverage.py in trace module documentation (GH-435)
+https://github.com/python/cpython/commit/9d07aceedabcdc9826489f8b9baffff056283bb3
+""",
+ content)
+
def test_suite():
suite = unittest.TestSuite()
for l in list_backends():
_______________________________________________
Tracker-discuss mailing list
Tracker-discuss@python.org
https://mail.python.org/mailman/listinfo/tracker-discuss
Code of Conduct: https://www.python.org/psf/codeofconduct/
