PR#5452: Add pull request ID to push notification payload - pagure

pagure

#5452 Add pull request ID to push notification payload

Merged a month ago by ngompa. Opened 2 months ago by nforro.

nforro/pagure pr_id into master

Add pull request ID to push notification payload

Nikola Forró • a month ago

8ed510d

pagure/hooks/__init__.py

file modified

+24 -4

		`@@ -62,7 +62,15 @@`

		`@classmethod`
		`def runhook(`
		`- cls, session, username, hooktype, project, repotype, repodir, changes`
		`+ cls,`
		`+ session,`
		`+ username,`
		`+ hooktype,`
		`+ project,`
		`+ repotype,`
		`+ repodir,`
		`+ changes,`
		`+ pull_request,`
		`):`
		`"""Run a specific hook on a project.`

		`@@ -81,6 +89,8 @@`
		`changes (dict): A dict with keys being the ref to update, values`
		`being a tuple of (from, to).`
		`For example: {'refs/heads/master': (hash_from, hash_to), ...}`
		`+ pull_request (model.PullRequest or None): The pull request whose`
		`+ merge is initiating this hook run.`
		`"""`
		`if hooktype == "pre-receive":`
		`cls.pre_receive(`
		`@@ -90,6 +100,7 @@`
		`repotype=repotype,`
		`repodir=repodir,`
		`changes=changes,`
		`+ pull_request=pull_request,`
		`)`
		`elif hooktype == "update":`
		`cls.update(`
		`@@ -99,6 +110,7 @@`
		`repotype=repotype,`
		`repodir=repodir,`
		`changes=changes,`
		`+ pull_request=pull_request,`
		`)`

		`elif hooktype == "post-receive":`
		`@@ -109,12 +121,15 @@`
		`repotype=repotype,`
		`repodir=repodir,`
		`changes=changes,`
		`+ pull_request=pull_request,`
		`)`
		`else:`
		`raise ValueError('Invalid hook type "%s"' % hooktype)`

		`@staticmethod`
		`- def pre_receive(session, username, project, repotype, repodir, changes):`
		`+ def pre_receive(`
		`+ session, username, project, repotype, repodir, changes, pull_request`
		`+ ):`
		`"""Run the pre-receive tasks of a hook.`

		`For args, see BaseRunner.runhook.`
		`@@ -122,7 +137,9 @@`
		`pass`

		`@staticmethod`
		`- def update(session, username, project, repotype, repodir, changes):`
		`+ def update(`
		`+ session, username, project, repotype, repodir, changes, pull_request`
		`+ ):`
		`"""Run the update tasks of a hook.`

		`For args, see BaseRunner.runhook.`
		`@@ -131,7 +148,9 @@`
		`pass`

		`@staticmethod`
		`- def post_receive(session, username, project, repotype, repodir, changes):`
		`+ def post_receive(`
		`+ session, username, project, repotype, repodir, changes, pull_request`
		`+ ):`
		`"""Run the post-receive tasks of a hook.`

		`For args, see BaseRunner.runhook.`
		`@@ -384,6 +403,7 @@`
		`repotype=repotype,`
		`repodir=repodir,`
		`changes=changes,`
		`+ pull_request=pull_request,`
		`)`
		`except Exception as e:`
		`if hooktype != "pre-receive" or debug:`

pagure/hooks/default.py

file modified

+6 -2

		`@@ -163,7 +163,7 @@`


		`def send_notifications(`
		`- session, project, repodir, user, refname, revs, forced, oldrev`
		`+ session, project, repodir, user, refname, revs, forced, oldrev, pr_id`
		`):`
		`"""Send out-going notifications about the commits that have just been`
		`pushed.`
		`@@ -212,6 +212,7 @@`
		`repo=project.to_json(public=True)`
		`if not isinstance(project, six.string_types)`
		`else project,`
		`+ pull_request_id=pr_id,`
		`)`

		`# Send blink notification to any 3rd party plugins, if there are any`
		`@@ -313,7 +314,9 @@`
		`"""Runner for the default hook."""`

		`@staticmethod`
		`- def post_receive(session, username, project, repotype, repodir, changes):`
		`+ def post_receive(`
		`+ session, username, project, repotype, repodir, changes, pull_request`
		`+ ):`
		`"""Run the default post-receive hook.`

		`For args, see BaseRunner.runhook.`
		`@@ -442,6 +445,7 @@`
		`commits,`
		`forced,`
		`oldrev,`
		`+ pull_request.id if pull_request else None,`
		`)`

		`# Now display to the user if this isn't the default branch links to`

pagure/hooks/mail.py

file modified

+3 -1

		`@@ -70,7 +70,9 @@`

		`class MailRunner(BaseRunner):`
		`@staticmethod`
		`- def post_receive(session, username, project, repotype, repodir, changes):`
		`+ def post_receive(`
		`+ session, username, project, repotype, repodir, changes, pull_request`
		`+ ):`
		`"""Run the multimail post-receive hook.`

		`For args, see BaseRunner.runhook.`

pagure/hooks/mirror_hook.py

file modified

+3 -1

		`@@ -67,7 +67,9 @@`
		`"""Runner for the mirror hook."""`

		`@staticmethod`
		`- def post_receive(session, username, project, repotype, repodir, changes):`
		`+ def post_receive(`
		`+ session, username, project, repotype, repodir, changes, pull_request`
		`+ ):`
		`"""Run the default post-receive hook.`

		`For args, see BaseRunner.runhook.`

pagure/hooks/pagure_force_commit.py

file modified

+3 -1

		`@@ -63,7 +63,9 @@`
		`"""Runner for the hook blocking force push."""`

		`@staticmethod`
		`- def pre_receive(session, username, project, repotype, repodir, changes):`
		`+ def pre_receive(`
		`+ session, username, project, repotype, repodir, changes, pull_request`
		`+ ):`
		`"""Run the pre-receive tasks of a hook.`

		`For args, see BaseRunner.runhook.`

pagure/hooks/pagure_hook.py

file modified

+3 -1

		`@@ -214,7 +214,9 @@`
		`"""Runner for the pagure's specific git hook."""`

		`@staticmethod`
		`- def post_receive(session, username, project, repotype, repodir, changes):`
		`+ def post_receive(`
		`+ session, username, project, repotype, repodir, changes, pull_request`
		`+ ):`
		`"""Run the default post-receive hook.`

		`For args, see BaseRunner.runhook.`

pagure/hooks/pagure_no_new_branches.py

file modified

+3 -1

		`@@ -59,7 +59,9 @@`
		`"""Runner for the hook blocking new branches from being created."""`

		`@staticmethod`
		`- def pre_receive(session, username, project, repotype, repodir, changes):`
		`+ def pre_receive(`
		`+ session, username, project, repotype, repodir, changes, pull_request`
		`+ ):`
		`"""Run the pre-receive tasks of a hook.`

		`For args, see BaseRunner.runhook.`

pagure/hooks/pagure_request_hook.py

file modified

+3 -1

		`@@ -64,7 +64,9 @@`
		`"""`

		`@staticmethod`
		`- def post_receive(session, username, project, repotype, repodir, changes):`
		`+ def post_receive(`
		`+ session, username, project, repotype, repodir, changes, pull_request`
		`+ ):`
		`"""Run the default post-receive hook.`

		`For args, see BaseRunner.runhook.`

pagure/hooks/pagure_ticket_hook.py

file modified

+3 -1

		`@@ -65,7 +65,9 @@`
		`"""Runner for the git hook updating the DB of tickets on push."""`

		`@staticmethod`
		`- def post_receive(session, username, project, repotype, repodir, changes):`
		`+ def post_receive(`
		`+ session, username, project, repotype, repodir, changes, pull_request`
		`+ ):`
		`"""Run the post-receive tasks of a hook.`

		`For args, see BaseRunner.runhook.`

pagure/hooks/pagure_unsigned_commits.py

file modified

+3 -1

		`@@ -64,7 +64,9 @@`
		`"""Runner for the hook blocking unsigned commits."""`

		`@staticmethod`
		`- def pre_receive(session, username, project, repotype, repodir, changes):`
		`+ def pre_receive(`
		`+ session, username, project, repotype, repodir, changes, pull_request`
		`+ ):`
		`"""Run the pre-receive tasks of a hook.`

		`For args, see BaseRunner.runhook.`

pagure/hooks/rtd.py

file modified

+3 -1

		`@@ -101,7 +101,9 @@`

		`class RtdRunner(BaseRunner):`
		`@staticmethod`
		`- def post_receive(session, username, project, repotype, repodir, changes):`
		`+ def post_receive(`
		`+ session, username, project, repotype, repodir, changes, pull_request`
		`+ ):`
		`"""Perform the RTD Post Receive hook.`

		`For arguments, see BaseRunner.runhook.`

tests/test_pagure_send_notification.py

file modified

		`@@ -74,6 +74,7 @@`
		`[sha],`
		`False,`
		`oldsha,`
		`+ None,`
		`)`
		`(_, args, kwargs) = fedmsg.mock_calls[0]`
		`self.assertEqual(args[1], "git.receive")`
		`@@ -88,6 +89,7 @@`
		`"folder1/folder2/fileŠ": "A",`
		`},`
		`)`
		`+ self.assertIsNone(args[2]["pull_request_id"])`


		`if __name__ == "__main__":`

nforro commented 2 months ago

Packit now consumes org.fedoraproject.prod.pagure.git.receive fedmsg messages, but unlike previously used org.fedoraproject.prod.git.receive there is no indication whether a push is a result of a pull request merge. It turns out there is no reliable way to get that information and to find the matching pull request. We could start consuming also org.fedoraproject.prod.pagure.pull-request.closed messages but we would still need to exclude related org.fedoraproject.prod.pagure.git.receive messages (as a PR merge generates both) and that would be prone to race conditions, so adding ID of a pull request whose merge triggered the event directly to org.fedoraproject.prod.pagure.git.receive payload seems like a better solution.

ngompa commented a month ago

Doesn't this require extending pagure-messages first?

nforro commented a month ago

Doesn't this require extending pagure-messages first?

Oh, I wasn't aware of that, thanks for the pointer! So I suppose GitReceiveV1 schema should have been modified as well before #5435 was merged (yet somehow it works). Let me fix it.