Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed normal retry time.sleep to use interval #8

Merged
merged 3 commits into from Dec 29, 2016
Merged

Conversation

tsarpaul
Copy link
Contributor

Also fixed inconsistent test: #7
Also added ImportError and KeyboardInterrupt to be raised despite retry: #6

@coveralls
Copy link

coveralls commented Dec 29, 2016

Coverage Status

Coverage decreased (-0.2%) to 93.525% when pulling 7048532 on tsarpaul:master into 6246161 on h2non:master.

@@ -157,19 +159,25 @@ def _handle_error(self, err):
# Update latest cached error
self.error = err

# Did user interrupt function?
if err is KeyboardInterrupt or err is ImportError:
raise
Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for this, but I'm going to support this in a more versatile way where the developer can add/mutate the error exceptions whitelist.

@h2non
Copy link
Owner

h2non commented Dec 29, 2016

LGTM, however I'm going to introduce some changes there in order to support some features.

@h2non h2non closed this Dec 29, 2016
@h2non h2non reopened this Dec 29, 2016
@coveralls
Copy link

coveralls commented Dec 29, 2016

Coverage Status

Coverage decreased (-0.2%) to 93.525% when pulling 7048532 on tsarpaul:master into 6246161 on h2non:master.

self.on_retry(err, delay)

# Sleep before next try
self.sleep(0.5)
Copy link
Owner

Choose a reason for hiding this comment

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

Good catch! I don't know how I forgot about this.

@h2non h2non merged commit 31f28b5 into h2non:master Dec 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants