#507 Move web-hooks logic to its own async server
Merged 4 years ago by pingou. Opened 4 years ago by pingou.

file modified
+1

@@ -7,3 +7,4 @@ 

  recursive-include doc *

  recursive-include alembic *

  recursive-include ev-server *

+ recursive-include webhook-server *

file modified
+22

@@ -239,6 +239,28 @@ 

  running. This allows adjusting the port via the configuration file instead

  of hard-coding it in the code.

  

+ .. note:: The EventSource server requires a redis server (see ``Redis options``

+          below)

+ 

+ 

+ Web-hooks notifications

+ -----------------------

+ 

+ WEBHOOK

+ ~~~~~~~

+ 

+ This configuration key allows turning on or off web-hooks notifications for

+ this pagure instance.

+ 

+ Defaults to: ``False``.

+ 

+ .. note:: The Web-hooks server requires a redis server (see ``Redis options``

+          below)

+ 

+ 

+ Redis options

+ -------------

+ 

  REDIS_HOST

  ~~~~~~~~~~

  

file modified
+1

@@ -33,6 +33,7 @@ 

     install

     install_milter

     install_evs

+    install_webhooks

     configuration

     development

     contributing

file modified
+10 -5

@@ -20,12 +20,13 @@ 

  So installing it is as easy as:

  ::

  

-     dnf install pagure pagure-milters pagure-ev

+     dnf install pagure pagure-milters pagure-ev pagure-webhook

  

  or

  

  ::

-     yum install pagure pagure-milters pagure-ev

+ 

+     yum install pagure pagure-milters pagure-ev pagure-webhook

  

  The ``pagure`` package contains the core of the application and the doc server.

  (See the ``Overview`` page for a global overview of the structure of the

@@ -36,8 +37,12 @@ 

  

  The ``pagure-ev`` package contains the eventsource server.

  

- ..note:: The last two packages are optional, pagure would work fine without

-         them.

+ The ``pagure-webhook`` package contains the web-hook server.

+ 

+ 

+ .. note:: The last three packages are optional, pagure would work fine without

+         them but the live-update, the webhook and the comment by email

+         services will not work.

  

  * From the sources

  

@@ -91,7 +96,7 @@ 

      python setup.py build

      sudo python setup.py install

  

- ..note:: To install the eventsource server or the milter, refer to their

+ .. note:: To install the eventsource server or the milter, refer to their

          respective documentations.

  

  # Install the additional files as follow:

file modified
+1 -1

@@ -19,7 +19,7 @@ 

      python-trollius

      python-trollius-redis

  

- ..note:: We ship a systemd unit file for pagure_milter but we welcome patches

+ .. note:: We ship a systemd unit file for pagure_milter but we welcome patches

          for scripts for other init systems.

  

  

file modified
+4 -4

@@ -18,10 +18,10 @@ 

  

      python-pymilter

  

- ..note:: We ship a systemd unit file for pagure_milter but we welcome patches

+ .. note:: We ship a systemd unit file for pagure_milter but we welcome patches

          for scripts for other init systems.

  

- ..note:: It also requires a MTA, we used postfix.

+ .. note:: It also requires a MTA, we used postfix.

  

  

  * Create an alias ``reply``

@@ -57,9 +57,9 @@ 

  |              Source                  |                   Destination                     |

  +======================================+===================================================+

  | ``milters/comment_email_milter.py``  | ``/usr/share//pagure/comment_email_milter.py``    |

- +----------------------------------------+-------------------------------------------------+

+ +--------------------------------------+---------------------------------------------------+

  | ``milters/milter_tempfile.conf``     | ``/usr/lib/tmpfiles.d/pagure-milter.conf``        |

- +----------------------------------------+-------------------------------------------------+

+ +--------------------------------------+---------------------------------------------------+

  | ``milters/pagure_milter.service``    | ``/etc/systemd/system/pagure_milter.service``     |

  +--------------------------------------+---------------------------------------------------+

  

@@ -0,0 +1,49 @@ 

+ Installing pagure's web-hooks notification system

+ =================================================

+ 

+ Web-hooks are a notification system upon which a system makes a http POST

+ request with some data upon doing an action. This allows notifying a system

+ that an action has occured.

+ 

+ If you want more information feel free to check out the corresponding page

+ on wikipedia: `https://en.wikipedia.org/wiki/Webhook

+ <https://en.wikipedia.org/wiki/Webhook>`_.

+ 

+ Configure your system

+ ---------------------

+ 

+ * Install the required dependencies

+ 

+ ::

+ 

+     python-redis

+     python-trollius

+     python-trollius-redis

+ 

+ .. note:: We ship a systemd unit file for pagure_webhook but we welcome patches

+         for scripts for other init systems.

+ 

+ 

+ * Install the files of the web-hook server as follow:

+ 

+ +----------------------------------------------+----------------------------------------------------------+

+ |              Source                          |                       Destination                        |

+ +==============================================+==========================================================+

+ | ``webhook-server/pagure-webhook-server.py``  | ``/usr/libexec/pagure-webhook/pagure-webhook-server.py`` |

+ +----------------------------------------------+----------------------------------------------------------+

+ | ``webhook-server/pagure_webhook.service``    | ``/etc/systemd/system/pagure_webhook.service``           |

+ +----------------------------------------------+----------------------------------------------------------+

+ 

+ The first file is the script of the web-hook server itself.

+ 

+ The second file is the systemd service file.

+ 

+ 

+ * Activate the service and ensure it's started upon boot:

+ 

+ ::

+ 

+     systemctl enable redis

+     systemctl start redis

+     systemctl enable pagure_webhook

+     systemctl start pagure_webhook

file modified
+4 -2

@@ -85,8 +85,10 @@ 

  Web-hooks

  ---------

  

- Pagure supports sending notification about event happening on a project

- via [web-hooks|].

+ Pagure offers the option of sending notification about event happening on a

+ project via [web-hooks|https://en.wikipedia.org/wiki/Webhook]. This option

+ is off by default and can be turned on for a pagure instance in its

+ configuration file.

  

  The URL of the web-hooks can be entered in this field.

  

@@ -149,7 +149,7 @@ 

          subscriber = yield trollius.From(connection.start_subscribe())

  

          # Subscribe to channel.

-         yield trollius.From(subscriber.subscribe([obj.uid]))

+         yield trollius.From(subscriber.subscribe(['pagure.%s' % obj.uid]))

  

          # Inside a while loop, wait for incoming events.

          while True:

file modified
+14 -5

@@ -124,14 +124,16 @@ 

  

  ### List of blacklisted project names that can conflicts for pagure's URLs

  ### or other

- BLACKLISTED_PROJECTS = ['static', 'pv']

+ BLACKLISTED_PROJECTS = [

+     'static', 'pv', 'releases', 'new', 'api', 'settings',

+     'logout', 'login', 'users', 'groups']

  

  ### IP addresses allowed to access the internal endpoints

  ### These endpoints are used by the milter and are security sensitive, thus

  ### the IP filter

  IP_ALLOWED_INTERNAL = ['127.0.0.1', 'localhost', '::1']

  

- ### EventSource/Redis configuration

+ ### EventSource/Web-Hook/Redis configuration

  # The eventsource integration is what allows pagure to refresh the content

  # on your page when someone else comments on the ticket (and this without

  # asking you to reload the page.

@@ -140,9 +142,6 @@ 

  # https://ev.pagure.io or https://pagure.io:8080 or whatever you are using

  # (Note: the urls sent to it start with a '/' so no need to add one yourself)

  EVENTSOURCE_SOURCE = None

- REDIS_HOST = '0.0.0.0'

- REDIS_PORT = 6379

- REDIS_DB = 0

  # Port where the event source server is running (maybe be the same port

  # as the one specified in EVENTSOURCE_SOURCE or a different one if you

  # have something running in front of the server such as apache or stunnel).

@@ -151,6 +150,16 @@ 

  # at this port and will provide information about the number of active

  # connections running on the first (main) event source server

  #EV_STATS_PORT = 8888

+ # Web-hook can be turned on or off allowing using them for notifications, or

+ # not.

+ WEBHOOK = False

+ 

+ ### Redis configuration

+ # A redis server is required for both the Event-Source server or the web-hook

+ # server.

+ REDIS_HOST = '0.0.0.0'

+ REDIS_PORT = 6379

+ REDIS_DB = 0

I think it would be good to have a REDIS_WEBHOOK_DB and REDIS_ESS_DB, so people can use a different one per service?
Maybe even make the _HOST and _PORT per service to allow for more redundancy/flexibility.

Isn't that a little over the top?

I mean the idea is interesting, but the usage of redis by pagure currently is such that I'm not sure the flexibility gain is worth the effort/complexity this adds to the config file.

After reading the docs, only making REDIS_DB changeable would make no sense at all since pub/sub goes across databases.

Perhaps it would be an idea to prefix the pub-sub topics into something like pagure.issue.projectA.idX, pagure.pr.projectB.idY and pagure.hook, to make sure the same redis server doesn't need to be unique to Pagure?

Maybe even, as suggested in Redis documentation, pagure.production.*?

Since you're changing everything anyway, this would be a great moment to make this change.

  

  # Authentication related configuration option

  

file modified
+36 -3

@@ -80,6 +80,7 @@ 

  system and possibilities to create new projects, fork existing ones and

  create/merge pull-requests across or within projects.

  

+ 

  %package milters

  Summary:            Milter to integrate pagure with emails

  BuildArch:          noarch

@@ -91,8 +92,6 @@ 

  # It would work with sendmail but we configure things (like the tempfile)

  # to work with postfix

  Requires:           postfix

- 

- 

  %description milters

  Milters (Mail filters) allowing the integration of pagure and emails.

  This is useful for example to allow commenting on a ticket by email.

@@ -111,7 +110,23 @@ 

  Requires(postun): systemd

  %description ev

  Pagure comes with an eventsource server allowing live update of the pages

- supporting it. This packages provides it.

+ supporting it. This package provides it.

+ 

+ 

+ %package webhook

+ Summary:   Web-Hook server for pagure

+ BuildArch: noarch

+ 

+ BuildRequires:      systemd-devel

+ Requires:  python-redis

+ Requires:  python-trollius

+ Requires:  python-trollius-redis

+ Requires(post): systemd

+ Requires(preun): systemd

+ Requires(postun): systemd

+ %description ev

+ Pagure comes with an webhook server allowing http callbacks for any action

+ done on a project. This package provides it.

  

  

  %prep

@@ -165,21 +180,33 @@ 

  install -m 644 ev-server/pagure_ev.service \

      $RPM_BUILD_ROOT/%{_unitdir}/pagure_ev.service

  

+ # Install the web-hook

+ mkdir -p $RPM_BUILD_ROOT/%{_libexecdir}/pagure-webhook

+ install -m 755 webhook-server/pagure-webhook-server.py \

+     $RPM_BUILD_ROOT/%{_libexecdir}/pagure-webhook/pagure-webhook-server.py

+ install -m 644 webhook-server/pagure_webhook.service \

+     $RPM_BUILD_ROOT/%{_unitdir}/pagure_webhook.service

  

  %post milters

  %systemd_post pagure_milter.service

  %post ev

  %systemd_post pagure_ev.service

+ %post webhook

+ %systemd_post pagure_webhook.service

  

  %preun milters

  %systemd_preun pagure_milter.service

  %preun ev

  %systemd_preun pagure_ev.service

+ %preun webhook

+ %systemd_preun pagure_webhook.service

  

  %postun milters

  %systemd_postun_with_restart pagure_milter.service

  %postun ev

  %systemd_postun_with_restart pagure_ev.service

+ %postun webhook

+ %systemd_postun_with_restart pagure_webhook.service

  

  

  %files

@@ -211,6 +238,12 @@ 

  %{_unitdir}/pagure_ev.service

  

  

+ %files webhook

+ %license LICENSE

+ %{_libexecdir}/pagure-webhook/

+ %{_unitdir}/pagure_webhook.service

+ 

+ 

  %changelog

  * Fri Nov 20 2015 Pierre-Yves Chibon <pingou@pingoured.fr> - 0.1.33-1

  - Update to 0.1.33

file modified
+4 -5

@@ -25,7 +25,6 @@ 

  

  import flask

  import pygit2

- import redis

  import werkzeug

  from pagure.flask_fas_openid import FAS

  from functools import wraps

@@ -58,12 +57,12 @@ 

  FAS = FAS(APP)

  SESSION = pagure.lib.create_session(APP.config['DB_URL'])

  REDIS = None

- if APP.config['EVENTSOURCE_SOURCE']:

-     POOL = redis.ConnectionPool(

+ if APP.config['EVENTSOURCE_SOURCE'] or APP.config['WEBHOOK']:

+     pagure.lib.set_redis(

          host=APP.config['REDIS_HOST'],

          port=APP.config['REDIS_PORT'],

-         db=APP.config['REDIS_DB'])

-     REDIS = redis.StrictRedis(connection_pool=POOL)

+         db=APP.config['REDIS_DB']

+     )

  

  if not APP.debug:

      APP.logger.addHandler(pagure.mail_logging.get_mail_handler(

@@ -69,6 +69,7 @@ 

  

  # Redis configuration

  EVENTSOURCE_SOURCE = None

+ WEBHOOK = False

  REDIS_HOST = '0.0.0.0'

  REDIS_PORT = 6379

  REDIS_DB = 0

file modified
+97 -65

@@ -22,6 +22,7 @@ 

  import uuid

  

  import bleach

+ import redis

  import sqlalchemy

  import sqlalchemy.schema

  from datetime import timedelta

@@ -44,6 +45,16 @@ 

  # pylint: disable=R0913

  

  

+ REDIS = None

+ 

+ 

+ def set_redis(host, port, db):

+     """ Set the redis connection with the specified information. """

+     global REDIS

+     pool = redis.ConnectionPool(host=host, port=port, db=db)

+     REDIS = redis.StrictRedis(connection_pool=pool)

+ 

+ 

  def __get_user(session, key):

      """ Searches for a user in the database for a given username or email.

      """

@@ -188,7 +199,7 @@ 

  

  

  def add_issue_comment(session, issue, comment, user, ticketfolder,

-                       notify=True, redis=None):

+                       notify=True):

      ''' Add a comment to an issue. '''

      user_obj = __get_user(session, user)

  

@@ -215,17 +226,18 @@ 

                  issue=issue.to_json(public=True),

                  project=issue.project.to_json(public=True),

                  agent=user_obj.username,

-             )

+             ),

+             redis=REDIS,

          )

  

-     if redis:

+     if REDIS:

          if issue.private:

-             redis.publish(issue.uid, json.dumps({

+             REDIS.publish('pagure.%s' % issue.uid, json.dumps({

                  'issue': 'private',

                  'comment_id': issue_comment.id,

              }))

          else:

-             redis.publish(issue.uid, json.dumps({

+             REDIS.publish('pagure.%s' % issue.uid, json.dumps({

                  'comment_id': issue_comment.id,

                  'comment_added': text2markdown(issue_comment.comment),

                  'comment_user': issue_comment.user.user,

@@ -237,7 +249,7 @@ 

      return 'Comment added'

  

  

- def add_tag_obj(session, obj, tags, user, ticketfolder, redis=None):

+ def add_tag_obj(session, obj, tags, user, ticketfolder):

      ''' Add a tag to an object (either an issue or a project). '''

      user_obj = __get_user(session, user)

  

@@ -290,11 +302,14 @@ 

                      project=obj.project.to_json(public=True),

                      tags=added_tags,

                      agent=user_obj.username,

-                 )

+                 ),

+                 redis=REDIS,

              )

  

-         if redis:

-             redis.publish(obj.uid, json.dumps({'added_tags': added_tags}))

+         # Send notification for the event-source server

+         if REDIS:

+             REDIS.publish('pagure.%s' % obj.uid, json.dumps(

+                 {'added_tags': added_tags}))

  

      if added_tags:

          return 'Tag added: %s' % ', '.join(added_tags)

@@ -302,8 +317,7 @@ 

          return 'Nothing to add'

  

  

- def add_issue_assignee(session, issue, assignee, user, ticketfolder,

-                        redis=None):

+ def add_issue_assignee(session, issue, assignee, user, ticketfolder):

      ''' Add an assignee to an issue, in other words, assigned an issue. '''

      user_obj = __get_user(session, user)

  

@@ -323,11 +337,14 @@ 

                      issue=issue.to_json(public=True),

                      project=issue.project.to_json(public=True),

                      agent=user_obj.username,

-                 )

+                 ),

+                 redis=REDIS,

              )

  

-         if redis:

-             redis.publish(issue.uid, json.dumps({'unassigned': '-'}))

+         # Send notification for the event-source server

+         if REDIS:

+             REDIS.publish('pagure.%s' % issue.uid, json.dumps(

+                 {'unassigned': '-'}))

  

          return 'Assignee reset'

      elif assignee is None and issue.assignee is None:

@@ -354,11 +371,13 @@ 

                      issue=issue.to_json(public=True),

                      project=issue.project.to_json(public=True),

                      agent=user_obj.username,

-                 )

+                 ),

+                 redis=REDIS,

              )

  

-         if redis:

-             redis.publish(issue.uid, json.dumps(

+         # Send notification for the event-source server

+         if REDIS:

+             REDIS.publish('pagure.%s' % issue.uid, json.dumps(

                  {'assigned': assignee_obj.to_json(public=True)}))

  

          return 'Issue assigned'

@@ -385,7 +404,8 @@ 

                  request=request.to_json(public=True),

                  project=request.project.to_json(public=True),

                  agent=user_obj.username,

-             )

+             ),

+             redis=REDIS,

          )

  

          return 'Request reset'

@@ -412,14 +432,15 @@ 

                  request=request.to_json(public=True),

                  project=request.project.to_json(public=True),

                  agent=user_obj.username,

-             )

+             ),

+             redis=REDIS,

          )

  

          return 'Request assigned'

  

  

  def add_issue_dependency(

-         session, issue, issue_blocked, user, ticketfolder, redis=None):

+         session, issue, issue_blocked, user, ticketfolder):

      ''' Add a dependency between two issues. '''

      user_obj = __get_user(session, user)

  

@@ -457,16 +478,18 @@ 

                      project=issue.project.to_json(public=True),

                      added_dependency=issue_blocked.id,

                      agent=user_obj.username,

-                 )

+                 ),

+                 redis=REDIS,

              )

  

-         if redis:

-             redis.publish(issue.uid, json.dumps({

+         # Send notification for the event-source server

+         if REDIS:

+             REDIS.publish('pagure.%s' % issue.uid, json.dumps({

                  'added_dependency': issue_blocked.id,

                  'issue_uid': issue.uid,

                  'type': 'children',

              }))

-             redis.publish(issue_blocked.uid, json.dumps({

+             REDIS.publish('pagure.%s' % issue_blocked.uid, json.dumps({

                  'added_dependency': issue.id,

                  'issue_uid': issue_blocked.uid,

                  'type': 'parent',

@@ -476,7 +499,7 @@ 

  

  

  def remove_issue_dependency(

-         session, issue, issue_blocked, user, ticketfolder, redis=None):

+         session, issue, issue_blocked, user, ticketfolder):

      ''' Remove a dependency between two issues. '''

      user_obj = __get_user(session, user)

  

@@ -515,16 +538,18 @@ 

                      project=issue.project.to_json(public=True),

                      removed_dependency=child_del,

                      agent=user_obj.username,

-                 )

+                 ),

+                 redis=REDIS,

              )

  

-         if redis:

-             redis.publish(issue.uid, json.dumps({

+         # Send notification for the event-source server

+         if REDIS:

+             REDIS.publish('pagure.%s' % issue.uid, json.dumps({

                  'removed_dependency': child_del,

                  'issue_uid': issue.uid,

                  'type': 'children',

              }))

-             redis.publish(issue_blocked.uid, json.dumps({

+             REDIS.publish('pagure.%s' % issue_blocked.uid, json.dumps({

                  'removed_dependency': issue.id,

                  'issue_uid': issue_blocked.uid,

                  'type': 'parent',

@@ -566,14 +591,15 @@ 

              project=project.to_json(public=True),

              tags=removed_tags,

              agent=user_obj.username,

-         )

+         ),

+         redis=REDIS,

      )

  

      return msgs

  

  

  def remove_tags_obj(

-         session, obj, tags, ticketfolder, user, redis=None):

+         session, obj, tags, ticketfolder, user):

      ''' Removes the specified tag(s) of a given object. '''

      user_obj = __get_user(session, user)

  

@@ -599,11 +625,13 @@ 

                  project=obj.project.to_json(public=True),

                  tags=removed_tags,

                  agent=user_obj.username,

-             )

+             ),

+             redis=REDIS,

          )

  

-         if redis:

-             redis.publish(obj.uid, json.dumps(

+         # Send notification for the event-source server

+         if REDIS:

+             REDIS.publish('pagure.%s' % obj.uid, json.dumps(

                  {'removed_tags': removed_tags}))

  

      return 'Removed tag: %s' % ', '.join(removed_tags)

@@ -668,7 +696,8 @@ 

                  old_tag=old_tag,

                  new_tag=new_tag,

                  agent=user_obj.username,

-             )

+             ),

+             redis=REDIS,

          )

  

      return msgs

@@ -701,7 +730,8 @@ 

              project=project.to_json(public=True),

              new_user=new_user_obj.username,

              agent=user_obj.username,

-         )

+         ),

+         redis=REDIS,

      )

  

      return 'User added'

@@ -748,15 +778,15 @@ 

              project=project.to_json(public=True),

              new_group=group_obj.group_name,

              agent=user,

-         )

+         ),

+         redis=REDIS,

      )

  

      return 'Group added'

  

  

  def add_pull_request_comment(session, request, commit, filename, row,

-                              comment, user, requestfolder, notify=True,

-                              redis=None):

+                              comment, user, requestfolder, notify=True):

      ''' Add a comment to a pull-request. '''

      user_obj = __get_user(session, user)

  

@@ -778,8 +808,9 @@ 

      if notify:

          pagure.lib.notify.notify_pull_request_comment(pr_comment, user_obj)

  

-     if redis:

-         redis.publish(request.uid, json.dumps({

+     # Send notification for the event-source server

+     if REDIS:

+         REDIS.publish('pagure.%s' % request.uid, json.dumps({

              'request_id': len(request.comments),

              'comment_added': text2markdown(pr_comment.comment),

              'comment_user': pr_comment.user.user,

@@ -796,7 +827,8 @@ 

          msg=dict(

              pullrequest=request.to_json(public=True),

              agent=user_obj.username,

-         )

+         ),

+         redis=REDIS,

      )

  

      return 'Comment added'

@@ -838,7 +870,8 @@ 

              pullrequest=request.to_json(public=True),

              flag=pr_flag.to_json(public=True),

              agent=user_obj.username,

-         )

+         ),

+         redis=REDIS,

      )

  

      return 'Flag %s' % action

@@ -916,7 +949,7 @@ 

          msg=dict(

              project=project.to_json(public=True),

              agent=user_obj.username,

-         )

+         ),

      )

  

      return 'Project "%s" created' % name

@@ -959,7 +992,8 @@ 

                  issue=issue.to_json(public=True),

                  project=issue.project.to_json(public=True),

                  agent=user_obj.username,

-             )

+             ),

+             redis=REDIS,

          )

  

      return issue

@@ -986,7 +1020,8 @@ 

                  issue=issue.to_json(public=True),

                  project=issue.project.to_json(public=True),

                  agent=user_obj.username,

-             )

+             ),

+             redis=REDIS,

          )

  

      return issue

@@ -1033,15 +1068,15 @@ 

          msg=dict(

              pullrequest=request.to_json(public=True),

              agent=user_obj.username,

-         )

+         ),

+         redis=REDIS,

      )

  

      return request

  

  

  def edit_issue(session, issue, ticketfolder, user,

-                title=None, content=None, status=None, private=False,

-                redis=None):

+                title=None, content=None, status=None, private=False):

      ''' Edit the specified issue.

      '''

      user_obj = __get_user(session, user)

@@ -1079,17 +1114,18 @@ 

                  project=issue.project.to_json(public=True),

                  fields=edit,

                  agent=user_obj.username,

-             )

+             ),

+             redis=REDIS,

          )

  

-     if redis and edit:

+     if REDIS and edit:

          if issue.private:

-             redis.publish(issue.uid, json.dumps({

+             REDIS.publish('pagure.%s' % issue.uid, json.dumps({

                  'issue': 'private',

                  'fields': edit,

              }))

          else:

-             redis.publish(issue.uid, json.dumps({

+             REDIS.publish('pagure.%s' % issue.uid, json.dumps({

                  'fields': edit,

                  'issue': issue.to_json(public=True, with_comments=False),

              }))

@@ -1137,7 +1173,8 @@ 

                  project=repo.to_json(public=True),

                  fields=update,

                  agent=user_obj.username,

-             )

+             ),

+             redis=REDIS,

          )

  

          return 'Edited successfully settings of repo: %s' % repo.fullname

@@ -1221,7 +1258,7 @@ 

          msg=dict(

              project=project.to_json(public=True),

              agent=user_obj.username,

-         )

+         ),

      )

  

      return 'Repo "%s" cloned to "%s/%s"' % (repo.name, user, repo.name)

@@ -1700,7 +1737,8 @@ 

              pullrequest=request.to_json(public=True),

              merged=merged,

              agent=user_obj.username,

-         )

+         ),

+         redis=REDIS,

      )

  

  

@@ -1908,7 +1946,7 @@ 

              hashhex, query)

  

  

- def update_tags(session, obj, tags, username, ticketfolder, redis=None):

+ def update_tags(session, obj, tags, username, ticketfolder):

      """ Update the tags of a specified object (adding or removing them).

      This object can be either an issue or a project.

  

@@ -1927,7 +1965,6 @@ 

                  tags=toadd,

                  user=username,

                  ticketfolder=ticketfolder,

-                 redis=redis,

              )

          )

  

@@ -1939,7 +1976,6 @@ 

                  tags=torm,

                  user=username,

                  ticketfolder=ticketfolder,

-                 redis=redis,

              )

          )

      session.commit()

@@ -1948,7 +1984,7 @@ 

  

  

  def update_dependency_issue(

-         session, repo, issue, depends, username, ticketfolder, redis=None):

+         session, repo, issue, depends, username, ticketfolder):

      """ Update the dependency of a specified issue (adding or removing them)

  

      """

@@ -1975,7 +2011,6 @@ 

                  issue_blocked=issue,

                  user=username,

                  ticketfolder=ticketfolder,

-                 redis=redis,

              )

          )

  

@@ -1997,7 +2032,6 @@ 

                  issue_blocked=issue_depend,

                  user=username,

                  ticketfolder=ticketfolder,

-                 redis=redis,

              )

          )

  

@@ -2006,7 +2040,7 @@ 

  

  

  def update_blocked_issue(

-         session, repo, issue, blocks, username, ticketfolder, redis=None):

+         session, repo, issue, blocks, username, ticketfolder):

      """ Update the upstream dependency of a specified issue (adding or

      removing them)

  

@@ -2034,7 +2068,6 @@ 

                  issue_blocked=issue_block,

                  user=username,

                  ticketfolder=ticketfolder,

-                 redis=redis,

              )

          )

          session.commit()

@@ -2058,7 +2091,6 @@ 

                  issue_blocked=issue,

                  user=username,

                  ticketfolder=ticketfolder,

-                 redis=redis,

              )

          )

  

file modified
+1 -1

@@ -399,7 +399,7 @@ 

          if tags:

              pagure.lib.add_tag_obj(

                  session, project, tags=tags, user=user.username,

-                 ticketfolder=None, redis=None)

+                 ticketfolder=None)

  

      return project

  

file modified
+10 -47

@@ -11,24 +11,19 @@ 

  

  import datetime

  import hashlib

- import hmac

  import json

  import urlparse

  import smtplib

- import six

  import time

- import uuid

  import warnings

  

  import flask

  import requests

+ import six

  import pagure

  

  from email.mime.text import MIMEText

- from kitchen.text.converters import to_bytes

- 

  

- _i = 0

  

  REPLY_MSG = 'To reply, visit the link below'

  if pagure.APP.config['EVENTSOURCE_SOURCE']:

@@ -49,53 +44,21 @@ 

          warnings.warn(str(err))

  

  

- def log(project, topic, msg):

+ def log(project, topic, msg, redis=None):

      ''' This is the place where we send notifications to user about actions

      occuring in pagure.

      '''

      # Send fedmsg notification (if fedmsg is there and set-up)

      fedmsg_publish(topic, msg)

  

-     # Send web-hooks notification

-     if not isinstance(project, basestring) \

-             and project.settings.get('Web-hooks'):  # pragma: no cover

-         global _i

-         _i += 1

-         year = datetime.datetime.now().year

-         if isinstance(topic, six.text_type):

-             topic = to_bytes(topic, encoding='utf8', nonstring="passthru")

-         msg = dict(

-             topic=topic.decode('utf-8'),

-             msg=msg,

-             timestamp=int(time.time()),

-             msg_id=str(year) + '-' + str(uuid.uuid4()),

-             i=_i,

-         )

- 

-         content = json.dumps(msg)

-         hashhex = hmac.new(

-             str(project.hook_token), content, hashlib.sha1).hexdigest()

-         headers = {

-             'X-Pagure-Topic': topic,

-             'X-Pagure-Signature': hashhex

-         }

-         msg = flask.json.dumps(msg)

-         for url in project.settings.get('Web-hooks').split('\n'):

-             url = url.strip()

-             try:

-                 req = requests.post(

-                     url,

-                     headers=headers,

-                     data={'payload': msg}

-                 )

-                 if not req:

-                     raise pagure.exceptions.PagureException(

-                         'An error occured while querying: %s - '

-                         'Error code: %s' % (url, req.status_code))

-             except (requests.exceptions.RequestException, Exception) as err:

-                 raise pagure.exceptions.PagureException(

-                     'An error occured while querying: %s - Error: %s' % (

-                         url, err))

+     if redis:

+         redis.publish(

+             'pagure.hook',

+             json.dumps({

+                 'project': project.fullname,

+                 'topic': topic,

+                 'msg': msg,

+             }))

  

  

  def _clean_emails(emails, user):

@@ -68,6 +68,8 @@ 

      <input type="submit" value="Make Default"/>

    </form>

    </div>

+ 

+   {% if config.get('WEBHOOK') %}

    <div>

      <h3>Private web-hook key</h3>

      <p>

@@ -95,6 +97,7 @@ 

      {{ form.csrf_token }}

    </form>

    </div>

+   {% endif %}

  

    <div>

      <h3>API key</h3>

@@ -169,6 +172,7 @@ 

      {% for key in repo.settings | sort %}

      {% if not config.get('ENABLE_TICKETS', True) and key in ['issue_tracker'] %}

      {% elif not config.get('DOC_APP_URL') and key in ['project_documentation'] %}

+     {% elif not config.get('WEBHOOK') and key in ['Web-hooks'] %}

      {% else %}

      <tr>

        <td><label for="{{ key }}">Activate {{ key | replace('_', ' ') }}</label></td>

file modified
+1 -3

@@ -20,8 +20,7 @@ 

  import pagure.lib

  import pagure.lib.git

  import pagure.forms

- from pagure import (APP, REDIS, SESSION, LOG, cla_required,

-                     is_repo_admin)

+ from pagure import (APP, SESSION, LOG, cla_required, is_repo_admin)

  

  

  # pylint: disable=E1101

@@ -445,7 +444,6 @@ 

                  comment=comment,

                  user=flask.g.fas_user.username,

                  requestfolder=APP.config['REQUESTS_FOLDER'],

-                 redis=REDIS,

              )

              SESSION.commit()

              if not is_js:

file modified
+3 -9

@@ -21,7 +21,7 @@ 

  import pagure.doc_utils

  import pagure.lib

  import pagure.forms

- from pagure import (APP, SESSION, REDIS, LOG, __get_file_in_tree,

+ from pagure import (APP, SESSION, LOG, __get_file_in_tree,

                      cla_required, is_repo_admin, authenticated)

  

  

@@ -135,7 +135,6 @@ 

                      comment=comment,

                      user=flask.g.fas_user.username,

                      ticketfolder=APP.config['TICKETS_FOLDER'],

-                     redis=REDIS,

                  )

                  SESSION.commit()

                  if message and not is_js:

@@ -146,8 +145,8 @@ 

                  messages = pagure.lib.update_tags(

                      SESSION, issue, tags,

                      username=flask.g.fas_user.username,

-                     ticketfolder=APP.config['TICKETS_FOLDER'],

-                     redis=REDIS)

+                     ticketfolder=APP.config['TICKETS_FOLDER']

+                 )

                  if not is_js:

                      for message in messages:

                          flask.flash(message)

@@ -159,7 +158,6 @@ 

                  assignee=assignee or None,

                  user=flask.g.fas_user.username,

                  ticketfolder=APP.config['TICKETS_FOLDER'],

-                 redis=REDIS,

              )

              if message and not is_js:

                  SESSION.commit()

@@ -175,7 +173,6 @@ 

                          private=issue.private,

                          user=flask.g.fas_user.username,

                          ticketfolder=APP.config['TICKETS_FOLDER'],

-                         redis=REDIS,

                      )

                      SESSION.commit()

                      if message:

@@ -186,7 +183,6 @@ 

                  SESSION, repo, issue, depends,

                  username=flask.g.fas_user.username,

                  ticketfolder=APP.config['TICKETS_FOLDER'],

-                 redis=REDIS,

              )

              if not is_js:

                  for message in messages:

@@ -197,7 +193,6 @@ 

                  SESSION, repo, issue, blocks,

                  username=flask.g.fas_user.username,

                  ticketfolder=APP.config['TICKETS_FOLDER'],

-                 redis=REDIS,

              )

              if not is_js:

                  for message in messages:

@@ -618,7 +613,6 @@ 

                  user=flask.g.fas_user.username,

                  ticketfolder=APP.config['TICKETS_FOLDER'],

                  private=private,

-                 redis=REDIS,

              )

              SESSION.commit()

  

file modified
+4 -1

@@ -868,7 +868,7 @@ 

                  tags=[t.strip() for t in form.tags.data.split(',')],

                  username=flask.g.fas_user.username,

                  ticketfolder=None,

-                 redis=None)

+             )

              SESSION.add(repo)

              SESSION.commit()

              flask.flash('Project updated')

@@ -986,6 +986,9 @@ 

  def new_repo_hook_token(repo, username=None):

      """ Re-generate a hook token for the present project.

      """

+     if not pagure.APP.config.get('WEBHOOK', False):

+         flask.abort(404)

+ 

      if admin_session_timedout():

          flask.flash('Action canceled, try it again', 'error')

          url = flask.url_for(

@@ -114,7 +114,7 @@ 

          # Adding a tag

          output = pagure.lib.update_tags(

              self.session, repo, 'infra', 'pingou',

-             ticketfolder=None, redis=None)

+             ticketfolder=None)

          self.assertEqual(output, ['Tag added: infra'])

  

          # Check after adding

@@ -1520,6 +1520,7 @@ 

  

          user = tests.FakeUser()

          with tests.user_set(pagure.APP, user):

+             pagure.APP.config['WEBHOOK'] = True

              output = self.app.get('/new/')

              self.assertEqual(output.status_code, 200)

              self.assertTrue('<h2>New project</h2>' in output.data)

@@ -1538,11 +1539,14 @@ 

              self.assertEqual(output.status_code, 302)

              ast.return_value = False

  

+             pagure.APP.config['WEBHOOK'] = False

+ 

          repo = pagure.lib.get_project(self.session, 'test')

          self.assertEqual(repo.hook_token, 'aaabbbccc')

  

          user.username = 'pingou'

          with tests.user_set(pagure.APP, user):

+             pagure.APP.config['WEBHOOK'] = True

              output = self.app.post('/test/hook_token')

              self.assertEqual(output.status_code, 400)

  

@@ -1558,6 +1562,7 @@ 

              self.assertIn(

                  '<li class="message">New hook token generated</li>',

                  output.data)

+             pagure.APP.config['WEBHOOK'] = False

  

          repo = pagure.lib.get_project(self.session, 'test')

          self.assertNotEqual(repo.hook_token, 'aaabbbccc')

@@ -0,0 +1,161 @@ 

+ #!/usr/bin/env python

+ 

+ """

+  (c) 2015 - Copyright Red Hat Inc

+ 

+  Authors:

+    Pierre-Yves Chibon <pingou@pingoured.fr>

+ 

+ 

+ This server listens to message sent via redis and send the corresponding

+ web-hook request.

+ 

+ Using this mechanism, we no longer block the main application if the

+ receiving end is offline or so.

+ 

+ """

+ 

+ import datetime

+ import hashlib

+ import hmac

+ import json

+ import logging

+ import os

+ import requests

+ import time

+ import uuid

+ 

+ import six

+ import trollius

+ import trollius_redis

+ 

+ from kitchen.text.converters import to_bytes

+ 

+ 

+ log = logging.getLogger(__name__)

+ 

+ 

+ if 'PAGURE_CONFIG' not in os.environ \

+         and os.path.exists('/etc/pagure/pagure.cfg'):

+     print 'Using configuration file `/etc/pagure/pagure.cfg`'

+     os.environ['PAGURE_CONFIG'] = '/etc/pagure/pagure.cfg'

+ 

+ 

+ import pagure

+ import pagure.lib

+ from pagure.exceptions import PagureEvException

+ 

+ _i = 0

+ 

+ 

+ def call_web_hooks(project, topic, msg):

+     ''' Sends the web-hook notification. '''

+     log.info(

+         "Processing project: %s - topic: %s", project.fullname, topic)

+     log.debug('msg: %s', msg)

+ 

+     # Send web-hooks notification

+     global _i

+     _i += 1

+     year = datetime.datetime.now().year

+     if isinstance(topic, six.text_type):

+         topic = to_bytes(topic, encoding='utf8', nonstring="passthru")

+     msg = dict(

+         topic=topic.decode('utf-8'),

+         msg=msg,

+         timestamp=int(time.time()),

+         msg_id=str(year) + '-' + str(uuid.uuid4()),

+         i=_i,

+     )

+ 

+     content = json.dumps(msg)

+     hashhex = hmac.new(

+         str(project.hook_token), content, hashlib.sha1).hexdigest()

+     headers = {

+         'X-Pagure-Topic': topic,

+         'X-Pagure-Signature': hashhex

+     }

+     msg = json.dumps(msg)

+     for url in project.settings.get('Web-hooks').split('\n'):

+         url = url.strip()

+         log.info('Calling url %s' % url)

+         try:

+             req = requests.post(

+                 url,

+                 headers=headers,

+                 data={'payload': msg}

+             )

+             if not req:

+                 raise pagure.exceptions.PagureException(

+                     'An error occured while querying: %s - '

+                     'Error code: %s' % (url, req.status_code))

+         except (requests.exceptions.RequestException, Exception) as err:

+             raise pagure.exceptions.PagureException(

+                 'An error occured while querying: %s - Error: %s' % (

+                     url, err))

+ 

+ 

+ @trollius.coroutine

+ def handle_messages():

+         connection = yield trollius.From(trollius_redis.Connection.create(

+             host='0.0.0.0', port=6379, db=0))

+ 

+         # Create subscriber.

+         subscriber = yield trollius.From(connection.start_subscribe())

+ 

+         # Subscribe to channel.

+         yield trollius.From(subscriber.subscribe(['pagure.hook']))

+ 

+         # Inside a while loop, wait for incoming events.

+         while True:

+             reply = yield trollius.From(subscriber.next_published())

+             log.info(

+                 'Received: %s on channel: %s',

+                 repr(reply.value), reply.channel)

+             data = json.loads(reply.value)

+             username = None

+             if '/' in data['project']:

+                 username, projectname = data['project'].split('/', 1)

+             else:

+                 projectname = data['project']

+             project = pagure.lib.get_project(

+                 session=pagure.SESSION, name=projectname, user=username)

+             log.info('Got the project, going to the webhooks')

+             call_web_hooks(project, data['topic'], data['msg'])

+ 

+ 

+ def main():

+     server = None

+     try:

+         loop = trollius.get_event_loop()

+         tasks = [

+             trollius.async(handle_messages()),

+         ]

+         loop.run_until_complete(trollius.wait(tasks))

+         loop.run_forever()

+     except KeyboardInterrupt:

+         pass

+     except trollius.ConnectionResetError:

+         pass

+ 

+     log.info("End Connection")

+     loop.close()

+     log.info("End")

+ 

+ 

+ if __name__ == '__main__':

+     log = logging.getLogger("")

+     formatter = logging.Formatter(

+         "%(asctime)s %(levelname)s [%(module)s:%(lineno)d] %(message)s")

+ 

+     # setup console logging

+     log.setLevel(logging.DEBUG)

+     ch = logging.StreamHandler()

+     ch.setLevel(logging.DEBUG)

+ 

+     aslog = logging.getLogger("asyncio")

+     aslog.setLevel(logging.DEBUG)

+ 

+     ch.setFormatter(formatter)

+     log.addHandler(ch)

+     main()

@@ -0,0 +1,14 @@ 

+ [Unit]

+ Description=Pagure WebHook server (Allowing web-hook notifications)

+ After=redis.target

+ Documentation=https://pagure.io/pagure

+ 

+ [Service]

+ ExecStart=/usr/libexec/pagure-webhook/pagure-webhook-server.py

+ Type=simple

+ User=git

+ Group=git

+ Restart=on-failure

+ 

+ [Install]

+ WantedBy=multi-user.target

no initial comment

Maybe remark that you will not have the features they provide?

I think it would be good to have a REDIS_WEBHOOK_DB and REDIS_ESS_DB, so people can use a different one per service?
Maybe even make the _HOST and _PORT per service to allow for more redundancy/flexibility.

Isn't that a little over the top?

I mean the idea is interesting, but the usage of redis by pagure currently is such that I'm not sure the flexibility gain is worth the effort/complexity this adds to the config file.

After reading the docs, only making REDIS_DB changeable would make no sense at all since pub/sub goes across databases.

Perhaps it would be an idea to prefix the pub-sub topics into something like pagure.issue.projectA.idX, pagure.pr.projectB.idY and pagure.hook, to make sure the same redis server doesn't need to be unique to Pagure?

Maybe even, as suggested in Redis documentation, pagure.production.*?

Since you're changing everything anyway, this would be a great moment to make this change.

Perhaps rename this function?
In this case, it doesn't handle a client but instead handles webhook requests/triggers.

Looks good to me. :thumbsup: