feat: add retry#51
Conversation
pcorpet
left a comment
There was a problem hiding this comment.
Thanks a lot for your suggestion: I've run into 429 myself several times and your change will be useful.
Can you address the styling comments + the use of requests_mock please? Otherwise I'll do the cleanup.
Reviewable status: 0 of 4 files reviewed, 7 unresolved discussions (waiting on @thejuan)
.gitignore, line 11 at r1 (raw file):
/ChangeLog /.eggs /.vscode
Please keep this in your own .gitignore
test_airtable.py, line 5 at r1 (raw file):
import requests_mock import httpretty
Please use requests_mock lib to mock response instead of adding a new library.
test_airtable.py, line 52 at r1 (raw file):
), httpretty.Response( body='{}',
I suggest you add an actual response and that you check below that it was indeed return.
test_airtable.py, line 57 at r1 (raw file):
] )
Please clean trailing whitespaces.
test_airtable.py, line 60 at r1 (raw file):
self.get() self.assertEqual(3, len(httpretty.latest_requests()))
Let's keep only one blank line between methods please.
airtable/init.py, line 8 at r1 (raw file):
import requests from requests.packages.urllib3.util.retry import Retry
To be consistent I prefer if we only import modules (except for types). So here import retry and then below use retry.Retry
airtable/init.py, line 95 at r1 (raw file):
self._dict_class = dict_class
Please drop this line (only one blank line between methods)
Unfortunately requests_mock uses a custom adapter, which replaces the retry adapter. It can't be used to test adapter features. Will fix up the rest, do you use a formatter for these I can run? |
| /AUTHORS | ||
| /ChangeLog | ||
| /.eggs | ||
| /.eggs No newline at end of file |
Airtable rate limits to 5 requests per second across all bases. They return http code 429 when you hit this limit.
This PR introduces retries via the requests retry functionality with back off so this error can be avoided.
I think this is a better approach than local rate limiting as there maybe multiple clients accessing the API, and is more aligned with the official node library
This change is