Handle retries using urllib3 Retry

[danmichaelo]

Per #43, this is my take at replacing the custom retry code with the tried and tested urllib3 Retry class. Benefits include that it also handles Timeout and ConnectionError, and that the sleep time increases for each retry. Tests pass, and I'm also testing the branch in a real world scenario now, where I need to harvest from a server which is quite unstable, and happens to produce both timeouts, connection errors and 500 errors during updates. It seems to work fine so far!

• A slight inconvenience of retrying upon ConnectionError, is that it will also retry if you misspell the hostname. Personally, I think the benefits of handling ConnectionError outweighs that incovenience, but it could be something to discuss. If the rest of the URL contains a typo, so that the server returns a 404, it will of course still raise an error immediately.

• Perhaps the major issue here is that the default_retry_after argument can no longer be supported. It has been replaced by a new argument retry_backoff_factor, to control the exponential backoff factor, but the replacement will be a breaking change since the old argument cannot be converted into the new one.

• Also note that I extended the default retry_status_codes with more codes. Since this is "OAI for humans", and humans do not necessarily know which errors to expect up front, I think it makes sense to include some more common error codes by default. But I can leave this extension out of this PR if it's controversial.

• Logging: If it's desirable to have logging, one possibility is to enable debug logging for the retry module:

logging.getLogger("urllib3.util.retry").setLevel(logging.DEBUG)

to get log messages like these:

2020-07-21 16:01:45,408 DEBUG retry Incremented Retry for (url='/'): Retry(total=9, connect=5, read=2, redirect=None, status=9)
2020-07-21 16:01:45,410 DEBUG retry Incremented Retry for (url='/'): Retry(total=8, connect=5, read=2, redirect=None, status=8)
2020-07-21 16:01:48,020 DEBUG retry Incremented Retry for (url='/'): Retry(total=7, connect=5, read=2, redirect=None, status=7)
2020-07-21 16:01:53,227 DEBUG retry Incremented Retry for (url='/'): Retry(total=6, connect=5, read=2, redirect=None, status=6)
2020-07-21 16:02:03,635 DEBUG retry Incremented Retry for (url='/'): Retry(total=5, connect=5, read=2, redirect=None, status=5)
2020-07-21 16:02:24,445 DEBUG retry Incremented Retry for (url='/'): Retry(total=4, connect=5, read=2, redirect=None, status=4)
2020-07-21 16:03:06,052 DEBUG retry Incremented Retry for (url='/'): Retry(total=3, connect=5, read=2, redirect=None, status=3)

I'm not sure if this should be the responsibility of sickle or the user, though.

[bertsky]

Note: method_whitelist has been deprecated in urllib3 v1.26 and removed in its v2. And since requests pins urllib3 to a range which includes the latter, IMHO the newer idiom must now be used here:

Suggested change

	method_whitelist=frozenset(['GET', 'POST'])
	allowed_methods=frozenset(['GET', 'POST'])

Otherwise, you'll get a

    retry_adapter = requests.adapters.HTTPAdapter(max_retries=Retry(
TypeError: __init__() got an unexpected keyword argument 'method_whitelist'

[bertsky]

Suggested change

	assert retries.method_whitelist == frozenset(['POST', 'GET'])
	assert retries.allowed_methods == frozenset(['POST', 'GET'])