Cp 7117/add retries to api calls #124
Conversation
…hich deal with error responses
quandl/api_config.py
Outdated
| use_retries = True | ||
| number_of_retries = 8 | ||
| retry_backoff_factor = 0.1 | ||
| max_wait_between_retries = 15 |
There was a problem hiding this comment.
i wonder if they agree with this 15 rule? it's not in the ticket 😹
There was a problem hiding this comment.
I went over these defaults with Pegah who said they were reasonable. Its open to discussion though
There was a problem hiding this comment.
I wouldn't be overly worried about the max_wait_between_retries. If it gets to that point something is terribly wrong. However we may be able to tweak the retry_backoff_factor, 4 retries within the first second may be too aggressive if its a Quandl latency issue that is causing the failure and could result in eating up rate limits. It is hard to tell though until we see what is actually causing these retries.
There was a problem hiding this comment.
@ericvautour-quandl Is there a value you'd suggest? If we bump the retry_backoff_factor I'd be in favour of reducing either the max_wait_between_retries or the number of retries
|
I think for the default values, it's better to check with the person who created the ticket before merging it to prod 😅 |
README.md
Outdated
| @@ -28,7 +28,11 @@ pip3 install quandl | |||
| |---|---|---| | |||
| | api_key | Your access key | `tEsTkEy123456789` | Used to identify who you are and provide full access. | | |||
| | api_version | The API version you wish to use | 2015-04-09 | Can be used to test your code against the latest version without committing to it. | | |||
|
|
|||
| | use_retries | Whether API calls which return statuses in `RETRY_STATUS_CODES` should be automatically retried | True | |||
There was a problem hiding this comment.
How many columns does this table have. Looks like yours are 3 and the ones above are 4.
There was a problem hiding this comment.
Theres 3 columns rendered. The top two rows specifying contents for a fourth column aren't being shown to the user - looks to be more of documentation for the dev. The rows I added already have a sufficient explanation so I wont be adding any comments to them
Overview
Automatically retry API calls if they return with any status codes within
RETRY_STATUS_CODES. Allows user to configure all retry settings viaApiConfig. Enables retry functionality to True by default.A note on the default settings with regards to sleep time
I've set the defaults ofmax_wait_between_retriesto 15 seconds,number_of_retriesto 8 andretry_backoff_factorto 0.1. These settings will provide the user max 8 retries, with wait times of [0.1, 0.2, 0.4, 0.8, 1.6. 3.2, 6.4, 12.8, 15] for each retry.Cumulatively this would be 40 seconds wait time assuming each of there calls error out. With any settings higher than this I suppose we run the risk of having the user wondering what is going on with their call. What does everyone think of these defaults?After discussing with Eric we agreed the current defaults were a bit to aggressive. I've reduced
number_of_retriesto 5,retry_backoff_factorto 0.5 andmax_wait_between_retriesto 8 seconds.