From 638bb48d641f2c4a1f3b13cb6899366cf9ba83b7 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 08 2018 09:17:13 +0000 Subject: [PATCH 1/2] Merge the fedmsg notifications on commit logic into the default hook This way we can finally split the web-hooks notifications from the fedmsg ones while not duplicating the entire logic around how to build the payload sent. Fixes https://pagure.io/pagure/issue/2413 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/hooks/fedmsg.py b/pagure/hooks/fedmsg.py index c9f0e6b..f2fd681 100644 --- a/pagure/hooks/fedmsg.py +++ b/pagure/hooks/fedmsg.py @@ -86,9 +86,13 @@ class Fedmsg(BaseHook): :arg project: a ``pagure.model.Project`` object to which the hook should be installed + This no longer does anything as the code has now been merged into + the default hook. So we still need this for people to opt in/out of + sending fedmsg notifications on commit push, but other than that + this plugin doesn't do much anymore. + ''' - repopaths = [get_repo_path(project)] - cls.base_install(repopaths, dbobj, 'fedmsg', 'fedmsg_hook.py') + pass @classmethod def remove(cls, project): @@ -97,6 +101,10 @@ class Fedmsg(BaseHook): :arg project: a ``pagure.model.Project`` object to which the hook should be installed + This no longer does anything as the code has now been merged into + the default hook. So we still need this for people to opt in/out of + sending fedmsg notifications on commit push, but other than that + this plugin doesn't do much anymore. + ''' - repopaths = [get_repo_path(project)] - cls.base_remove(repopaths, 'fedmsg') + pass diff --git a/pagure/hooks/files/default_hook.py b/pagure/hooks/files/default_hook.py index e54c21e..686c8a4 100644 --- a/pagure/hooks/files/default_hook.py +++ b/pagure/hooks/files/default_hook.py @@ -6,6 +6,7 @@ from __future__ import print_function import os +import logging import sys import pygit2 @@ -15,6 +16,7 @@ import pagure.flask_app # noqa: E402 import pagure.exceptions # noqa: E402 import pagure.lib.link # noqa: E402 import pagure.lib.tasks # noqa: E402 +import pagure.lib.tasks_services # noqa: E402 if 'PAGURE_CONFIG' not in os.environ \ @@ -23,9 +25,85 @@ if 'PAGURE_CONFIG' not in os.environ \ _config = pagure.config.config +_log = logging.getLogger(__name__) abspath = os.path.abspath(os.environ['GIT_DIR']) +def send_fedmsg_notifications(project, topic, msg): + ''' If the user asked for fedmsg notifications on commit, this will + do it. + ''' + import fedmsg + config = fedmsg.config.load_config([], None) + config['active'] = True + config['endpoints']['relay_inbound'] = config['relay_inbound'] + fedmsg.init(name='relay_inbound', **config) + + pagure.lib.notify.log( + project=project, + topic=topic, + msg=msg, + redis=None, # web-hook notification are handled separately + ) + + +def send_webhook_notifications(project, topic, msg): + ''' If the user asked for webhook notifications on commit, this will + do it. + ''' + + pagure.lib.tasks_services.webhook_notification.delay( + topic=topic, + msg=msg, + namespace=project.namespace, + name=project.name, + user=project.user.username if project.is_fork else None, + ) + + +def send_notifications(session, project, revs): + ''' Send out-going notifications about the commits that have just been + pushed. + ''' + + auths = set() + for rev in revs: + email = pagure.lib.git.get_author_email(rev, abspath) + name = pagure.lib.git.get_author(rev, abspath) + author = pagure.lib.search_user(session, email=email) or name + auths.add(author) + + authors = [] + for author in auths: + if isinstance(author, basestring): + author = author + else: + author = author.to_json(public=True) + authors.append(author) + + if revs: + revs.reverse() + print("* Publishing information for %i commits" % len(revs)) + + fedmsg_hook = pagure.lib.plugins.get_plugin('Fedmsg') + fedmsg_hook.db_object() + + if project.fedmsg_hook.active: + try: + print(" - to fedmsg") + send_fedmsg_notifications(project, topic, msg) + except Exception as err: + _log.exception( + 'Error sending fedmsg notifications on commit push') + if project.settings.get('Web-hooks') and not project.private: + try: + print(" - to web-hooks") + send_webhook_notifications(project, topic, msg) + except Exception as err: + _log.exception( + 'Error sending web-hook notifications on commit push') + + def run_as_post_receive_hook(): repo = pagure.lib.git.get_repo_name(abspath) @@ -65,6 +143,13 @@ def run_as_post_receive_hook(): print("Deleting a reference/branch, so we won't run the " "pagure hook") return + elif set(oldrev) == set(['0']): + oldrev = '^%s' % oldrev + elif pagure.lib.git.is_forced_push(oldrev, newrev, abspath): + forced = True + base = pagure.lib.git.get_base_revision(oldrev, newrev, abspath) + if base: + oldrev = base[0] refname = refname.replace('refs/heads/', '') commits = pagure.lib.git.get_revs_between( @@ -76,6 +161,10 @@ def run_as_post_receive_hook(): else: print('Sending to redis to send commit notification emails') + # This is logging the commit to the log table in the DB so we can + # render commits in the calendar heatmap. + # It is also sending emails about commits to people using the + # 'watch' feature to be made aware of new commits. pagure.lib.tasks_services.log_commit_send_notifications.delay( name=repo, commits=commits, @@ -118,6 +207,10 @@ def run_as_post_receive_hook(): refname) ) print() + # This one is sending fedmsg and web-hook notifications for project + # that set them up + send_notifications(session, project, commits) + # Schedule refresh of all opened PRs parent = project.parent or project diff --git a/pagure/hooks/files/fedmsg_hook.py b/pagure/hooks/files/fedmsg_hook.py deleted file mode 100755 index d17fb97..0000000 --- a/pagure/hooks/files/fedmsg_hook.py +++ /dev/null @@ -1,100 +0,0 @@ -#!/usr/bin/env python - -from __future__ import print_function - -import os -import sys - -import fedmsg -import fedmsg.config - - -if 'PAGURE_CONFIG' not in os.environ \ - and os.path.exists('/etc/pagure/pagure.cfg'): - os.environ['PAGURE_CONFIG'] = '/etc/pagure/pagure.cfg' - - -import pagure # noqa: E402 -import pagure.lib.git # noqa: E402 - -abspath = os.path.abspath(os.environ['GIT_DIR']) - - -print("Emitting a message to the fedmsg bus.") -config = fedmsg.config.load_config([], None) -config['active'] = True -config['endpoints']['relay_inbound'] = config['relay_inbound'] -fedmsg.init(name='relay_inbound', **config) - -_config = pagure.config.config - -seen = [] - -session = pagure.lib.create_session(_config['DB_URL']) - -# Read in all the rev information git-receive-pack hands us. -for line in sys.stdin.readlines(): - (oldrev, newrev, refname) = line.strip().split(' ', 2) - - forced = False - if set(newrev) == set(['0']): - print("Deleting a reference/branch, so we won't run the " - "pagure hook") - break - elif set(oldrev) == set(['0']): - print("New reference/branch") - oldrev = '^%s' % oldrev - elif pagure.lib.git.is_forced_push(oldrev, newrev, abspath): - forced = True - base = pagure.lib.git.get_base_revision(oldrev, newrev, abspath) - if base: - oldrev = base[0] - - revs = pagure.lib.git.get_revs_between( - oldrev, newrev, abspath, refname, forced=forced) - - project_name = pagure.lib.git.get_repo_name(abspath) - username = pagure.lib.git.get_username(abspath) - namespace = pagure.lib.git.get_repo_namespace(abspath) - project = pagure.lib._get_project( - session, project_name, username, namespace=namespace, - case=_config.get('CASE_SENSITIVE', False)) - - if not project: - project = project_name - - auths = set() - for rev in revs: - email = pagure.lib.git.get_author_email(rev, abspath) - name = pagure.lib.git.get_author(rev, abspath) - author = pagure.lib.search_user(session, email=email) or name - auths.add(author) - - authors = [] - for author in auths: - if isinstance(author, basestring): - author = author - else: - author = author.to_json(public=True) - authors.append(author) - - if revs: - revs.reverse() - print("* Publishing information for %i commits" % len(revs)) - pagure.lib.notify.log( - project=project, - topic="git.receive", - msg=dict( - total_commits=len(revs), - start_commit=revs[0], - end_commit=revs[-1], - branch=refname, - forced=forced, - authors=list(authors), - agent=os.environ['GL_USER'], - repo=project.to_json(public=True) - if not isinstance(project, basestring) else project, - ), - redis=pagure.lib.REDIS, - ) -session.close() From c11a662f29e36213690b3f32ba96020b035146bc Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 08 2018 09:17:13 +0000 Subject: [PATCH 2/2] Move around the code informing the user about the PR URLs This code informs the user about the URLs they can use to create a new PR with the code they just pushed, or review the existing PR integrating this new code. It should make it easier to read this code and follow the logic. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/hooks/files/default_hook.py b/pagure/hooks/files/default_hook.py index 686c8a4..8f87046 100644 --- a/pagure/hooks/files/default_hook.py +++ b/pagure/hooks/files/default_hook.py @@ -104,6 +104,44 @@ def send_notifications(session, project, revs): 'Error sending web-hook notifications on commit push') +def inform_pull_request_urls(project, commits, refname, default_branch): + ''' Inform the user about the URLs to open a new pull-request or visit + the existing one. + ''' + target_repo = project + if project.is_fork: + target_repo = project.parent + + if commits and refname != default_branch\ + and target_repo.settings.get('pull_requests', True): + print() + prs = pagure.lib.search_pull_requests( + session, + project_id_from=project.id, + status='Open', + branch_from=refname, + ) + # Link to existing PRs if there are any + seen = len(prs) != 0 + for pr in prs: + print('View pull-request for %s' % refname) + print(' %s/%s/pull-request/%s' % ( + _config['APP_URL'].rstrip('/'), + pr.project.url_path, + pr.id) + ) + # If no existing PRs, provide the link to open one + if not seen: + print('Create a pull-request for %s' % refname) + print(' %s/%s/diff/%s..%s' % ( + _config['APP_URL'].rstrip('/'), + project.url_path, + default_branch, + refname) + ) + print() + + def run_as_post_receive_hook(): repo = pagure.lib.git.get_repo_name(abspath) @@ -175,42 +213,13 @@ def run_as_post_receive_hook(): username=username, ) - target_repo = project - if project.is_fork: - target_repo = project.parent - - if commits and refname != default_branch\ - and target_repo.settings.get('pull_requests', True): - print() - prs = pagure.lib.search_pull_requests( - session, - project_id_from=project.id, - status='Open', - branch_from=refname, - ) - # Link to existing PRs if there are any - seen = len(prs) != 0 - for pr in prs: - print('View pull-request for %s' % refname) - print(' %s/%s/pull-request/%s' % ( - _config['APP_URL'].rstrip('/'), - pr.project.url_path, - pr.id) - ) - # If no existing PRs, provide the link to open one - if not seen: - print('Create a pull-request for %s' % refname) - print(' %s/%s/diff/%s..%s' % ( - _config['APP_URL'].rstrip('/'), - project.url_path, - default_branch, - refname) - ) - print() # This one is sending fedmsg and web-hook notifications for project # that set them up send_notifications(session, project, commits) + # Now display to the user if this isn't the default branch links to + # open a new pr or review the existing one + inform_pull_request_urls(project, commits, refname, default_branch) # Schedule refresh of all opened PRs parent = project.parent or project