rest request retry upon failure #63
Conversation
| @@ -21,6 +24,20 @@ | |||
| class Requester: | |||
| "Basic Requester using requests and blocking calls" | |||
|
|
|||
| retries = attr.ib(default=attr.Factory( | |||
| config_item_getter('REQUESTER', 'retries')), | |||
There was a problem hiding this comment.
Nice! For the other classes I tried to have the ini file section have the same title as the class name (i.e. the same case). Not sure if that runs foul of INI file conventions of being all upper case. The aim was that with the ConfigMixin it would automatically find the correct section and be open for extension by subclasses. It was a bit tricky and I see that for the api keys in Luno and BraveNewCoin we didn't follow that. However it does seem to work for assets and currencies (e.g. for Luno) but I think you have to define a custom validator.
There was a problem hiding this comment.
Donald Knuth's famous quote (and what I tell my team often) may apply here: Premature optimization is the root of all evil. My take is that we may be overthinking things a bit here. Lets just try make things simple for now. In that regard, I propose making things all capitals (like the ConfigSetter examples online) and have the spelling identical to the class name.
Also, those custom validators look like code bloat to me...
When a rest request fails, it should be retried again before giving up. This is best practice (and in fact, I implemented this very feature for Microsoft OneNote).
The code below needs to be unit tested to ensure no regressions. However, we have not agreed upon the framework, so I will do it once it is up.
Lastly, I needed to implement this feature because crypto-compare has been giving lots of 503 responses lately. 503 response is temp unavailable, and one should try again in that case.