-
Notifications
You must be signed in to change notification settings - Fork 524
do not sleep after last retry before raising exception #790
Conversation
retry=0 - retry forever retry=1 - try once, on error don't do any retry retry=2 - 2 tries, one original and one retry on error retry=3 - 3 tries, one original and maximum two retries on errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @krzysbaranski, appreciate you taking the time to help out! Could you please look to address the feedback and resubmit when you get a chance?
influxdb/client.py
Outdated
@@ -292,7 +295,7 @@ def request(self, url, method='GET', params=None, data=None, | |||
_try += 1 | |||
if self._retries != 0: | |||
retry = _try < self._retries | |||
if method == "POST": | |||
if retry and method == "POST": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would just invert the logic here:
if self._retries != 0:
retry = _try < self._retries
if not retry:
raise
if method == "POST":
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
influxdb/client.py
Outdated
defaults to 3. 0 indicates try until success | ||
For example: | ||
1 - try only once (without retry), | ||
2 - maximum two tries (including one retry) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please also add a line here for the default option?
:param retries: number of attempts your client will make before aborting, defaults to 3
0 - try until success
1 - attempt only once (without retry)
2 - maximum two attempts (including one retry)
3 - maximum three attempts (default option)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you very much for contributing!
* do not sleep after last retry before raising exception * documentation: clarification of retry parameter retry=0 - retry forever retry=1 - try once, on error don't do any retry retry=2 - 2 tries, one original and one retry on error retry=3 - 3 tries, one original and maximum two retries on errors * retries - move raise before sleep * retries - documentation * fix line length
do not sleep after last retry before raising exception
documentation: clarification of how retry parameter works
retry=0 - retry forever
retry=1 - try once, on error don't do any retry
retry=2 - 2 tries, one original and one retry on error
retry=3 - 3 tries, one original and maximum two retries on errors