From 251c008372a3a92f6b1f2a7ce164d064cf21a440 Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Apr 30 2018 15:00:18 +0000 Subject: Add a retry decorator to wrap frail functions. We're having some DNS issues internally that throw NewConnectionErrors at us regularly. This is a workaround. --- diff --git a/greenwave/config.py b/greenwave/config.py index 9801d86..b872100 100644 --- a/greenwave/config.py +++ b/greenwave/config.py @@ -13,11 +13,20 @@ class Config(object): PORT = 5005 PRODUCTION = False SECRET_KEY = 'replace-me-with-something-random' + RESULTSDB_API_URL = 'https://taskotron.fedoraproject.org/resultsdb_api/api/v2.0' WAIVERDB_API_URL = 'https://waiverdb.fedoraproject.org/api/v1.0' + + # Options for outbound HTTP requests made by python-requests REQUESTS_TIMEOUT = (6.1, 15) REQUESTS_VERIFY = True + + # General options for retrying failed operations (querying external services) + RETRY_TIMEOUT = 6 + RETRY_INTERVAL = 2 + POLICIES_DIR = '/etc/greenwave/policies' + # By default, don't cache anything. CACHE = {'backend': 'dogpile.cache.null'} diff --git a/greenwave/resources.py b/greenwave/resources.py index 4cd44db..5a4d57e 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -6,16 +6,21 @@ waiverdb, etc..). """ -import requests import json + +import requests +import urllib3.exceptions + from flask import current_app from greenwave.cache import cached +from greenwave.utils import retry requests_session = requests.Session() @cached +@retry(wait_on=urllib3.exceptions.NewConnectionError) def retrieve_results(item): """ Retrieve cached results from resultsdb for a given item. """ # XXX make this more efficient than just fetching everything @@ -31,6 +36,7 @@ def retrieve_results(item): # NOTE - not cached, for now. +@retry(wait_on=urllib3.exceptions.NewConnectionError) def retrieve_waivers(product_version, item): timeout = current_app.config['REQUESTS_TIMEOUT'] verify = current_app.config['REQUESTS_VERIFY'] diff --git a/greenwave/tests/test_utils.py b/greenwave/tests/test_utils.py new file mode 100644 index 0000000..57fd8e7 --- /dev/null +++ b/greenwave/tests/test_utils.py @@ -0,0 +1,37 @@ + +# SPDX-License-Identifier: GPL-2.0+ + +import pytest + +from greenwave.utils import retry + + +def test_retry_passthrough(): + """ Ensure that retry doesn't gobble exceptions. """ + expected = "This is the exception." + + @retry(timeout=0.1, interval=0.1, wait_on=Exception) + def f(): + raise Exception(expected) + + with pytest.raises(Exception) as actual: + f() + + assert expected in str(actual) + + +def test_retry_count(): + """ Ensure that retry doesn't gobble exceptions. """ + expected = "This is the exception." + + calls = [] + + @retry(timeout=0.3, interval=0.1, wait_on=Exception) + def f(): + calls.append(1) + raise Exception(expected) + + with pytest.raises(Exception): + f() + + assert sum(calls) == 3 diff --git a/greenwave/utils.py b/greenwave/utils.py index c36522e..ef75ab1 100644 --- a/greenwave/utils.py +++ b/greenwave/utils.py @@ -4,6 +4,7 @@ import functools import glob import logging import os +import time import yaml from flask import jsonify, current_app, request @@ -125,3 +126,30 @@ def insert_headers(response): response.headers['Access-Control-Allow-Headers'] = 'Content-Type' response.headers['Access-Control-Allow-Method'] = 'POST, OPTIONS' return response + + +def retry(timeout=None, interval=None, wait_on=Exception): + """ A decorator that allows to retry a section of code... + ...until success or timeout. + + If omitted, the values for `timeout` and `interval` are + taken from the global configuration. + """ + def wrapper(function): + @functools.wraps(function) + def inner(*args, **kwargs): + _timeout = timeout or current_app.config['RETRY_TIMEOUT'] + _interval = interval or current_app.config['RETRY_INTERVAL'] + # These can be configured per-function, or globally if omitted. + start = time.time() + while True: + try: + return function(*args, **kwargs) + except wait_on as e: + log.warn("Exception %r raised from %r. Retry in %rs" % ( + e, function, _interval)) + time.sleep(_interval) + if (time.time() - start) >= _timeout: + raise # This re-raises the last exception. + return inner + return wrapper