x
/
packetary
Code Issues Proposed changes

Merge "Fixed retries for HTTP Errors"

This commit is contained in:
Jenkins 2016-02-01 17:30:06 +00:00 committed by Gerrit Code Review
parent cacf6b75e3 fb3f8ffe02
commit ec8a9c8c22
6 changed files with 128 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
packetary/api.py
View File

@ -37,7 +37,7 @@ class Configuration(object):
"""The configuration holder."""
def __init__(self, http_proxy=None, https_proxy=None,
retries_num=0, threads_num=0,
retries_num=0, retry_interval=0, threads_num=0,
ignore_errors_num=0):
"""Initialises.
@ -46,6 +46,7 @@ class Configuration(object):
:param https_proxy: the url of proxy for connections over https,
no-proxy will be used if it is not specified
:param retries_num: the number of retries on errors
:param retry_interval: the minimal time between retries (in seconds)
:param threads_num: the max number of active threads
:param ignore_errors_num: the number of errors that may occurs
before stop processing
@ -55,6 +56,7 @@ class Configuration(object):
self.https_proxy = https_proxy
self.ignore_errors_num = ignore_errors_num
self.retries_num = retries_num
self.retry_interval = retry_interval
self.threads_num = threads_num
@ -69,7 +71,8 @@ class Context(object):
self._connection = ConnectionsManager(
proxy=config.http_proxy,
secure_proxy=config.https_proxy,
retries_num=config.retries_num
retries_num=config.retries_num,
retry_interval=config.retry_interval
)
self._threads_num = config.threads_num
self._ignore_errors_num = config.ignore_errors_num

7
packetary/cli/app.py
View File

@ -50,6 +50,13 @@ class Application(app.App):
metavar="NUMBER",
help="The number of retries."
)
parser.add_argument(
"--retry-interval",
type=int,
default=1,
metavar="SECONDS",
help="The minimal time between retries in seconds."
)
parser.add_argument(
"--threads-num",
default=3,

71
packetary/library/connections.py
View File

@ -40,10 +40,25 @@ class RangeError(urlerror.URLError):
class RetryableRequest(urllib.Request):
MAX_INTERVAL = 5
offset = 0
retries_left = 1
retry_interval = 0
start_time = 0
def get_retry_interval(self):
"""Calculates progressive retry interval in seconds.
:return: the time to wait before start retry
"""
# we uses progressive timeout between retries,
# the greatest number of retry will have greatest timeout
# but limited with MAX_INTERVAL
coef = max(self.MAX_INTERVAL - self.retries_left, 1)
interval = self.retry_interval * coef
return min(interval, self.MAX_INTERVAL)
class ResumableResponse(StreamWrapper):
"""The http-response wrapper to add resume ability.
@ -83,7 +98,11 @@ class RetryHandler(urllib.BaseHandler):
@staticmethod
def http_request(request):
"""Initialises http request."""
"""Initialises http request.
:param request: the instance of RetryableRequest
:return: the request
"""
logger.debug("start request: %s", request.get_full_url())
if request.offset > 0:
request.add_header('Range', 'bytes=%d-' % request.offset)
@ -94,46 +113,59 @@ class RetryHandler(urllib.BaseHandler):
"""Wraps response in a ResumableResponse.
Checks that partial request completed successfully.
:param request: the instance of RetryableRequest
:param response: the response object
:return: ResumableResponse if success otherwise same response
"""
code, msg = response.getcode(), response.msg
# the server should response partial content if range is specified
if request.offset > 0 and code != http_client.PARTIAL_CONTENT:
raise RangeError(msg)
if code >= 400:
logger.error(
"request failed: %s - %d(%s), retries left - %d.",
request.get_full_url(), code, msg, request.retries_left - 1
)
if is_retryable_http_error(code) and request.retries_left > 0:
time.sleep(request.get_retry_interval())
request.retries_left -= 1
response = self.parent.open(request)
# pass response to next handler as is.
return response
logger.debug(
"finish request: %s - %d (%s), duration - %d ms.",
"request completed: %s - %d (%s), duration - %d ms.",
request.get_full_url(), response.getcode(), response.msg,
int((time.time() - request.start_time) * 1000)
)
if request.offset > 0 and response.getcode() != 206:
raise RangeError("Server does not support ranges.")
return ResumableResponse(request, response, self.parent)
def http_error(self, req, fp, code, msg, hdrs):
"""Checks error code and retries request if it is allowed."""
logger.error(
"fail request: %s - %d(%s), retries left - %d.",
req.get_full_url(), code, msg, req.retries_left
)
if req.retries_left > 0 and is_retryable_http_error(code):
req.retries_left -= 1
return self.parent.open(req)
return ResumableResponse(request, response, self.parent)
https_request = http_request
https_response = http_response
def is_retryable_http_error(code):
"""Checks that http error can be retried."""
return code == http_client.NOT_FOUND or \
code >= http_client.INTERNAL_SERVER_ERROR
"""Checks that http error can be retried.
:param code: the HTTP_CODE
:return: True if request can be retried otherwise False
"""
return code >= http_client.INTERNAL_SERVER_ERROR
class ConnectionsManager(object):
"""The connections manager."""
def __init__(self, proxy=None, secure_proxy=None, retries_num=0):
def __init__(self, proxy=None, secure_proxy=None,
retries_num=0, retry_interval=0):
"""Initialises.
:param proxy: the url of proxy for http-connections
:param secure_proxy: the url of proxy for https-connections
:param retries_num: the number of allowed retries
:param retry_interval: the minimal time between retries (in seconds)
"""
if proxy:
proxies = {
@ -144,6 +176,7 @@ class ConnectionsManager(object):
proxies = None
self.retries_num = retries_num
self.retry_interval = retry_interval
self.opener = urllib.build_opener(
RetryHandler(),
urllib.ProxyHandler(proxies)
@ -163,6 +196,7 @@ class ConnectionsManager(object):
request = RetryableRequest(url)
request.retries_left = self.retries_num
request.retry_interval = self.retry_interval
request.offset = offset
return request
@ -188,6 +222,7 @@ class ConnectionsManager(object):
"Failed to open url - %s: %s. retries left - %d.",
url, six.text_type(e), request.retries_left
)
time.sleep(request.get_retry_interval())
def retrieve(self, url, filename, **attributes):
"""Downloads remote file.

2
packetary/tests/test_cli_commands.py
View File

@ -44,6 +44,7 @@ class TestCliCommands(base.TestCase):
"--ignore-errors-num=3",
"--threads-num=8",
"--retries-num=10",
"--retry-interval=1",
"--http-proxy=http://proxy",
"--https-proxy=https://proxy"
]
@ -85,6 +86,7 @@ class TestCliCommands(base.TestCase):
self.assertEqual(3, config.ignore_errors_num)
self.assertEqual(8, config.threads_num)
self.assertEqual(10, config.retries_num)
self.assertEqual(1, config.retry_interval)
def test_clone_cmd(self, api_mock, read_file_mock, stdout_mock):
read_file_mock.side_effect = [

62
packetary/tests/test_connections.py
View File

@ -98,6 +98,29 @@ class TestConnectionManager(base.TestCase):
"/test/file", "I/O error", 0
)
@mock.patch.multiple(
"packetary.library.connections.urllib.HTTPHandler",
http_request=mock.DEFAULT,
http_open=mock.DEFAULT
)
def test_retries_on_50x(self, logger, http_open, http_request):
request = connections.RetryableRequest("http:///localhost/file1.txt")
request.retries_left = 1
http_request.return_value = request
response_mock = mock.MagicMock(code=501, msg="not found")
response_mock.getcode.return_value = response_mock.code
http_open.return_value = response_mock
manager = connections.ConnectionsManager(retries_num=2)
with self.assertRaises(connections.urlerror.HTTPError) as trapper:
manager.open_stream("http:///localhost/file1.txt")
self.assertEqual(501, trapper.exception.code)
self.assertEqual(2, http_request.call_count)
for retry_num in six.moves.range(1):
logger.error.assert_any_call(
"request failed: %s - %d(%s), retries left - %d.",
mock.ANY, 501, mock.ANY, retry_num
)
@mock.patch("packetary.library.connections.urllib.build_opener")
def test_raise_other_errors(self, *_):
manager = connections.ConnectionsManager()
@ -109,6 +132,23 @@ class TestConnectionManager(base.TestCase):
self.assertEqual(1, manager.opener.open.call_count)
@mock.patch("packetary.library.connections.urllib.build_opener")
@mock.patch("packetary.library.connections.time")
def test_progressive_delay_between_request(self, time_mock, *_):
manager = connections.ConnectionsManager(
retries_num=6, retry_interval=1
)
manager.opener.open.side_effect = IOError("I/O Error")
with self.assertRaises(IOError):
manager.open_stream("/test/file")
self.assertEqual(7, manager.opener.open.call_count)
self.assertEqual(
[1, 1, 2, 3, 4, 5],
[x[0][0] for x in time_mock.sleep.call_args_list]
)
@mock.patch("packetary.library.connections.urllib.build_opener")
@mock.patch("packetary.library.connections.ensure_dir_exist")
@mock.patch("packetary.library.connections.os")
@ -198,7 +238,7 @@ class TestRetryHandler(base.TestCase):
r = self.handler.http_response(request, response)
self.assertIsInstance(r, connections.ResumableResponse)
logger.debug.assert_called_with(
"finish request: %s - %d (%s), duration - %d ms.",
"request completed: %s - %d (%s), duration - %d ms.",
"/file/test", 200, "test", 10
)
@ -214,20 +254,18 @@ class TestRetryHandler(base.TestCase):
response.getcode.return_value = 206
self.handler.http_response(request, response)
def test_error(self, logger):
request = mock.MagicMock()
def test_handle_error(self, logger):
request = mock.MagicMock(retries_left=1, retry_interval=0, offset=0)
request.get_retry_interval.return_value = 0
request.get_full_url.return_value = "/test"
request.retries_left = 1
self.handler.http_error(
request, mock.MagicMock(), 500, "error", mock.MagicMock()
)
response_mock = mock.MagicMock(code=500, msg="error")
response_mock.getcode.return_value = response_mock.code
self.handler.http_response(request, response_mock)
logger.error.assert_called_with(
"fail request: %s - %d(%s), retries left - %d.",
"/test", 500, "error", 1
)
self.handler.http_error(
request, mock.MagicMock(), 404, "error", mock.MagicMock()
"request failed: %s - %d(%s), retries left - %d.",
"/test", 500, "error", 0
)
self.handler.http_response(request, response_mock)
self.handler.parent.open.assert_called_once_with(request)

16
packetary/tests/test_repository_api.py
View File

@ -59,13 +59,15 @@ class TestRepositoryApi(base.TestCase):
def test_create_with_config(self, connection_mock, controller_mock):
config = Configuration(
http_proxy="http://localhost", https_proxy="https://localhost",
retries_num=10, threads_num=8, ignore_errors_num=6
retries_num=10, retry_interval=1, threads_num=8,
ignore_errors_num=6
)
RepositoryApi.create(config, "deb", "x86_64")
connection_mock.assert_called_once_with(
proxy="http://localhost",
secure_proxy="https://localhost",
retries_num=10
retries_num=10,
retry_interval=1
)
controller_mock.load.assert_called_once_with(
mock.ANY, "deb", "x86_64"
@ -76,14 +78,16 @@ class TestRepositoryApi(base.TestCase):
def test_create_with_context(self, connection_mock, controller_mock):
config = Configuration(
http_proxy="http://localhost", https_proxy="https://localhost",
retries_num=10, threads_num=8, ignore_errors_num=6
retries_num=10, retry_interval=1, threads_num=8,
ignore_errors_num=6
)
context = Context(config)
RepositoryApi.create(context, "deb", "x86_64")
connection_mock.assert_called_once_with(
proxy="http://localhost",
secure_proxy="https://localhost",
retries_num=10
retries_num=10,
retry_interval=1
)
controller_mock.load.assert_called_once_with(
context, "deb", "x86_64"
@ -212,6 +216,7 @@ class TestContext(base.TestCase):
threads_num=2,
ignore_errors_num=3,
retries_num=5,
retry_interval=10,
http_proxy="http://localhost",
https_proxy="https://localhost"
)
@ -222,7 +227,8 @@ class TestContext(base.TestCase):
conn_manager.assert_called_once_with(
proxy="http://localhost",
secure_proxy="https://localhost",
retries_num=5
retries_num=5,
retry_interval=10
)
self.assertIs(