Skip to content
This repository was archived by the owner on Oct 29, 2024. It is now read-only.

do not sleep after last retry before raising exception #790

Merged
merged 5 commits into from
Apr 11, 2020

Conversation

krzysbaranski
Copy link
Contributor

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

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
@sebito91 sebito91 self-assigned this Apr 8, 2020
Copy link
Contributor

@sebito91 sebito91 left a 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?

@@ -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":
Copy link
Contributor

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":
    ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

defaults to 3. 0 indicates try until success
For example:
1 - try only once (without retry),
2 - maximum two tries (including one retry)
Copy link
Contributor

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@krzysbaranski krzysbaranski requested a review from sebito91 April 11, 2020 16:46
Copy link
Contributor

@sebito91 sebito91 left a 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!

@sebito91 sebito91 merged commit e7ef045 into influxdata:master Apr 11, 2020
ocworld pushed a commit to AhnLab-OSS/influxdb-python that referenced this pull request Apr 13, 2020
* 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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants