You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by je...@apache.org on 2015/02/02 17:40:37 UTC
[10/10] allura git commit: [#4542] ticket:715 Better error handling &
retries
[#4542] ticket:715 Better error handling & retries
Project: http://git-wip-us.apache.org/repos/asf/allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/allura/commit/aa638e80
Tree: http://git-wip-us.apache.org/repos/asf/allura/tree/aa638e80
Diff: http://git-wip-us.apache.org/repos/asf/allura/diff/aa638e80
Branch: refs/heads/ib/4542
Commit: aa638e80cc9784234111d568e25e36058d900782
Parents: 648f805
Author: Igor Bondarenko <je...@gmail.com>
Authored: Mon Feb 2 15:45:42 2015 +0000
Committer: Igor Bondarenko <je...@gmail.com>
Committed: Mon Feb 2 15:45:42 2015 +0000
----------------------------------------------------------------------
Allura/allura/tests/test_webhooks.py | 106 +++++++++++++++++++++++-------
Allura/allura/webhooks.py | 99 +++++++++++++++++++++-------
Allura/development.ini | 5 ++
3 files changed, 163 insertions(+), 47 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/allura/blob/aa638e80/Allura/allura/tests/test_webhooks.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/test_webhooks.py b/Allura/allura/tests/test_webhooks.py
index 791128e..ab82cb8 100644
--- a/Allura/allura/tests/test_webhooks.py
+++ b/Allura/allura/tests/test_webhooks.py
@@ -3,7 +3,7 @@ import hmac
import hashlib
import datetime as dt
-from mock import Mock, patch
+from mock import Mock, patch, call
from nose.tools import (
assert_raises,
assert_equal,
@@ -17,13 +17,13 @@ from tg import config
from allura import model as M
from allura.lib import helpers as h
-from allura.lib.utils import DateJSONEncoder
from allura.webhooks import (
MingOneOf,
WebhookValidator,
WebhookController,
send_webhook,
RepoPushWebhookSender,
+ SendWebhookHelper,
)
from allura.tests import decorators as td
from alluratest.controller import setup_basic_test, TestController
@@ -400,45 +400,103 @@ class TestWebhookController(TestController):
return [link(tds[0]), link(tds[1]), text(tds[2]), delete_btn(tds[3])]
-class TestTasks(TestWebhookBase):
+class TestSendWebhookHelper(TestWebhookBase):
- @patch('allura.webhooks.requests', autospec=True)
- @patch('allura.webhooks.log', autospec=True)
- def test_send_webhook(self, log, requests):
- requests.post.return_value = Mock(status_code=200)
- payload = {'some': ['data']}
- json_payload = json.dumps(payload, cls=DateJSONEncoder)
- send_webhook(self.wh._id, payload)
+ def setUp(self, *args, **kw):
+ super(TestSendWebhookHelper, self).setUp(*args, **kw)
+ self.payload = {'some': ['data', 23]}
+ self.h = SendWebhookHelper(self.wh, self.payload)
+
+ def test_timeout(self):
+ assert_equal(self.h.timeout, 30)
+ with h.push_config(config, **{'webhook.timeout': 10}):
+ assert_equal(self.h.timeout, 10)
+
+ def test_retries(self):
+ assert_equal(self.h.retries, [60, 120, 240])
+ with h.push_config(config, **{'webhook.retry': '1 2 3 4 5 6'}):
+ assert_equal(self.h.retries, [1, 2, 3, 4, 5, 6])
+
+ def test_sign(self):
+ json_payload = json.dumps(self.payload)
signature = hmac.new(
self.wh.secret.encode('utf-8'),
json_payload.encode('utf-8'),
hashlib.sha1)
signature = 'sha1=' + signature.hexdigest()
+ assert_equal(self.h.sign(json_payload), signature)
+
+ def test_log_msg(self):
+ assert_equal(
+ self.h.log_msg('OK'),
+ 'OK: repo-push http://httpbin.org/post /adobe/adobe-1/src/')
+ assert_equal(
+ self.h.log_msg('Error', response=Mock(status_code=500, reason='that is why')),
+ 'Error: repo-push http://httpbin.org/post /adobe/adobe-1/src/ 500 that is why')
+
+ @patch('allura.webhooks.SendWebhookHelper', autospec=True)
+ def test_send_webhook_task(self, swh):
+ send_webhook(self.wh._id, self.payload)
+ swh.assert_called_once_with(self.wh, self.payload)
+
+ @patch('allura.webhooks.requests', autospec=True)
+ @patch('allura.webhooks.log', autospec=True)
+ def test_send(self, log, requests):
+ requests.post.return_value = Mock(status_code=200)
+ self.h.sign = Mock(return_value='sha1=abc')
+ self.h.send()
headers = {'content-type': 'application/json',
'User-Agent': 'Allura Webhook (https://allura.apache.org/)',
- 'X-Allura-Signature': signature}
+ 'X-Allura-Signature': 'sha1=abc'}
requests.post.assert_called_once_with(
self.wh.hook_url,
- data=json_payload,
+ data=json.dumps(self.payload),
headers=headers,
timeout=30)
log.info.assert_called_once_with(
- 'Webhook successfully sent: %s %s %s',
- self.wh.type, self.wh.hook_url, self.wh.app_config.url())
+ 'Webhook successfully sent: %s %s %s' % (
+ self.wh.type, self.wh.hook_url, self.wh.app_config.url()))
+ @patch('allura.webhooks.time', autospec=True)
@patch('allura.webhooks.requests', autospec=True)
@patch('allura.webhooks.log', autospec=True)
- def test_send_webhook_error(self, log, requests):
+ def test_send_error_response_status(self, log, requests, time):
requests.post.return_value = Mock(status_code=500)
- send_webhook(self.wh._id, {})
- assert_equal(requests.post.call_count, 1)
- assert_equal(log.info.call_count, 0)
- log.error.assert_called_once_with(
- 'Webhook send error: %s %s %s %s %s',
- self.wh.type, self.wh.hook_url,
- self.wh.app_config.url(),
- requests.post.return_value.status_code,
- requests.post.return_value.reason)
+ self.h.send()
+ assert_equal(requests.post.call_count, 4) # initial call + 3 retries
+ assert_equal(time.sleep.call_args_list,
+ [call(60), call(120), call(240)])
+ assert_equal(log.info.call_args_list, [
+ call('Retrying webhook in: %s', [60, 120, 240]),
+ call('Retrying webhook in %s seconds', 60),
+ call('Retrying webhook in %s seconds', 120),
+ call('Retrying webhook in %s seconds', 240)])
+ assert_equal(log.error.call_count, 4)
+ log.error.assert_called_with(
+ 'Webhook send error: %s %s %s %s %s' % (
+ self.wh.type, self.wh.hook_url,
+ self.wh.app_config.url(),
+ requests.post.return_value.status_code,
+ requests.post.return_value.reason))
+
+ @patch('allura.webhooks.time', autospec=True)
+ @patch('allura.webhooks.requests', autospec=True)
+ @patch('allura.webhooks.log', autospec=True)
+ def test_send_error_no_retries(self, log, requests, time):
+ requests.post.return_value = Mock(status_code=500)
+ with h.push_config(config, **{'webhook.retry': ''}):
+ self.h.send()
+ assert_equal(requests.post.call_count, 1)
+ assert_equal(time.call_count, 0)
+ log.info.assert_called_once_with('Retrying webhook in: %s', [])
+ assert_equal(log.error.call_count, 1)
+ log.error.assert_called_with(
+ 'Webhook send error: %s %s %s %s %s' % (
+ self.wh.type, self.wh.hook_url,
+ self.wh.app_config.url(),
+ requests.post.return_value.status_code,
+ requests.post.return_value.reason))
+
class TestRepoPushWebhookSender(TestWebhookBase):
http://git-wip-us.apache.org/repos/asf/allura/blob/aa638e80/Allura/allura/webhooks.py
----------------------------------------------------------------------
diff --git a/Allura/allura/webhooks.py b/Allura/allura/webhooks.py
index 7df8a42..ebfb3d0 100644
--- a/Allura/allura/webhooks.py
+++ b/Allura/allura/webhooks.py
@@ -19,6 +19,9 @@ import logging
import json
import hmac
import hashlib
+import time
+import socket
+import ssl
import requests
from bson import ObjectId
@@ -29,6 +32,7 @@ from formencode import validators as fev, schema, Invalid
from ming.odm import session
from webob import exc
from pymongo.errors import DuplicateKeyError
+from paste.deploy.converters import asint, aslist
from allura.controllers import BaseController
from allura.lib import helpers as h
@@ -224,32 +228,81 @@ class WebhookController(BaseController):
'form': form}
+class SendWebhookHelper(object):
+
+ def __init__(self, webhook, payload):
+ self.webhook = webhook
+ self.payload = payload
+
+ @property
+ def timeout(self):
+ return asint(config.get('webhook.timeout', 30))
+
+ @property
+ def retries(self):
+ t = aslist(config.get('webhook.retry', [60, 120, 240]))
+ return map(int, t)
+
+ def sign(self, json_payload):
+ signature = hmac.new(
+ self.webhook.secret.encode('utf-8'),
+ json_payload.encode('utf-8'),
+ hashlib.sha1)
+ return 'sha1=' + signature.hexdigest()
+
+ def log_msg(self, msg, response=None):
+ message = '{}: {} {} {}'.format(
+ msg,
+ self.webhook.type,
+ self.webhook.hook_url,
+ self.webhook.app_config.url())
+ if response:
+ message = '{} {} {}'.format(
+ message,
+ response.status_code,
+ response.reason)
+ return message
+
+ def send(self):
+ json_payload = json.dumps(self.payload, cls=DateJSONEncoder)
+ signature = self.sign(json_payload)
+ headers = {'content-type': 'application/json',
+ 'User-Agent': 'Allura Webhook (https://allura.apache.org/)',
+ 'X-Allura-Signature': signature}
+ ok = self._send(self.webhook.hook_url, json_payload, headers)
+ if not ok:
+ log.info('Retrying webhook in: %s', self.retries)
+ for t in self.retries:
+ log.info('Retrying webhook in %s seconds', t)
+ time.sleep(t)
+ ok = self._send(self.webhook.hook_url, json_payload, headers)
+ if ok:
+ return
+
+ def _send(self, url, data, headers):
+ try:
+ r = requests.post(
+ url,
+ data=data,
+ headers=headers,
+ timeout=self.timeout)
+ except (requests.exceptions.RequestException,
+ socket.timeout,
+ ssl.SSLError):
+ log.exception(self.log_msg('Webhook send error'))
+ return False
+ if r.status_code >= 200 and r.status_code < 300:
+ log.info(self.log_msg('Webhook successfully sent'))
+ return True
+ else:
+ log.error(self.log_msg('Webhook send error', response=r))
+ return False
+
+
@task()
def send_webhook(webhook_id, payload):
webhook = M.Webhook.query.get(_id=webhook_id)
- url = webhook.hook_url
- json_payload = json.dumps(payload, cls=DateJSONEncoder)
- signature = hmac.new(
- webhook.secret.encode('utf-8'),
- json_payload.encode('utf-8'),
- hashlib.sha1)
- signature = 'sha1=' + signature.hexdigest()
- headers = {'content-type': 'application/json',
- 'User-Agent': 'Allura Webhook (https://allura.apache.org/)',
- 'X-Allura-Signature': signature}
- # TODO: catch
- # TODO: configurable timeout
- r = requests.post(url, data=json_payload, headers=headers, timeout=30)
- if r.status_code >= 200 and r.status_code < 300:
- log.info('Webhook successfully sent: %s %s %s',
- webhook.type, webhook.hook_url, webhook.app_config.url())
- else:
- # TODO: retry
- # TODO: configurable retries
- log.error('Webhook send error: %s %s %s %s %s',
- webhook.type, webhook.hook_url,
- webhook.app_config.url(),
- r.status_code, r.reason)
+ SendWebhookHelper(webhook, payload).send()
class WebhookSender(object):
http://git-wip-us.apache.org/repos/asf/allura/blob/aa638e80/Allura/development.ini
----------------------------------------------------------------------
diff --git a/Allura/development.ini b/Allura/development.ini
index 8258c7c..a00377b 100644
--- a/Allura/development.ini
+++ b/Allura/development.ini
@@ -125,6 +125,11 @@ user_prefs_storage.ldap.fields.display_name = cn
# Limit the number of emails a user can claim.
user_prefs.maximum_claimed_emails = 20
+# webhook.timeout = 30 # seconds, default = 30
+
+# List of pauses between retries, if hook fails (in seconds)
+# webhook.retry = 60 120 240
+
# Limit rate of webhook firing (in seconds, default = 30)
# Option format: webhook.<hook type>.limit,
# all '-' in hook type must be changed to '_'