#479 Handle retry failures
Closed: Fixed by vmaljulin. Opened by lholecek.

This is same as #460 but exception urllib3.exceptions.MaxRetryError needs to be handled on other places as well -- ideally wrap it around custom exception in greenwave/request_session.py and handle it where needed.

Also refactor current solution for RemoteRule so it doesn't checks if URL ends with 'gating.yaml' in some general exception handler (it shouldn't know details about some other module).


To be clearer, there are couple of places where HTTPError but MaxRetryError is not handled. This doesn't to be correct since both exceptions are tightly related. Additionally, I think the former exception is raised from response.raise_for_status() and the latter when making the request.

Possible fix in greenwave/request_session.py:

class MaxRetryReachedResponse:
    ok = False
    status_code = 502
    def __init__(self, text):
        self.text = text
    def raise_for_status(self):
        raise BadGateway(self.text)
def request(*args, **kwargs):
    """
    Make a request.
    Same as with requests library but will retry of failure and returns
    MaxRetryReachedResponse if maximum number of retries reached.
    :return:
        MaxRetryReachedResponse: if maximum number of retries reached
        requests.Response: otherwise
    """
    try:
        return _requests_session.request(*args, **kwargs)
    except MaxRetryError as e:
        log.exception('Request failed')
        return MaxRetryReachedResponse(str(e))
def get(*args, **kwargs):
    return request('GET', *args, **kwargs)
def post(*args, **kwargs):
    return request('POST', *args, **kwargs)

...or subclass requests.Session and override request() method.

Another idea is to have dedicated submodule that wraps requests so that other submodules don't have to handle it.

Also, this code needs to be changed. Function in utils submodule shouldn't know anything about details in higher level submodules (gating.yaml is related to policies.RemoteRule).

Metadata Update from @vmaljulin:
- Issue assigned to vmaljulin

Fix note: MaxRetryFailure is handled in requests.adapters lib (line 500). It's being reraised by some particular error. In my fix I gonna handle all these errors.

Commit 647680fe fixes this issue

Metadata
Related Pull Requests