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
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 22 additions & 9 deletions riprova/retrier.py
Expand Up @@ -148,7 +148,9 @@ def istimeout(self, start):
Returns:
bool: `True` if timeout exceeded, otherwise `False`.
"""
return self.timeout > 0 and (now() - start) > self.timeout
now_time = now()
print now_time - start
return now_time - start + 1 > self.timeout > 0

def _handle_error(self, err):
"""
Expand All @@ -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.


def _notify_subscriber(self, delay):
# Notify retry subscriber, if needed
if self.on_retry:
self.on_retry(self.error, delay)

def _get_delay(self):
# Get delay before next retry
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.

delay = self.backoff.next()

# If backoff is ready
if delay == Backoff.STOP:
return raise_from(MaxRetriesExceeded('max retries exceeded'), err)
return raise_from(MaxRetriesExceeded('max retries exceeded'),
self.error)

# Notify retry subscriber, if needed
if self.on_retry:
self.on_retry(err, delay)

# Sleep before next try
self.sleep(0.5)
return delay

def run(self, fn, *args, **kw):
"""
Expand Down Expand Up @@ -201,7 +209,7 @@ def run(self, fn, *args, **kw):
start = now()

# Run operation in a infinitive loop until the task succeeded or
# and max retry attemts are reached.
# and max retry attempts are reached.
while True:
# Ensure we do not exceeded the max timeout
if self.istimeout(start):
Expand All @@ -212,6 +220,11 @@ def run(self, fn, *args, **kw):
return self._call(fn, *args, **kw)
except Exception as err:
self._handle_error(err)
delay = self._get_delay()
self._notify_subscriber(delay)

# Increment retry attempts
self.attempts += 1

# Sleep before next try
self.sleep(float(delay) / 1000) # Millisecs converted to secs
8 changes: 5 additions & 3 deletions tests/retrier_test.py
Expand Up @@ -18,7 +18,9 @@ def test_retrier_defaults():

def test_retrier_custom():
def sleep(): pass

def on_retry(): pass # noqa

def evaluator(): pass # noqa

backoff = ConstantBackoff()
Expand Down Expand Up @@ -121,7 +123,7 @@ def test_retrier_run_max_timeout(MagicMock):
iterable = (ValueError, NotImplementedError, RuntimeError, Exception)
task = MagicMock(side_effect=iterable)

retrier = Retrier(timeout=200, backoff=ConstantBackoff(interval=100))
retrier = Retrier(timeout=200, backoff=ConstantBackoff(interval=120))

with pytest.raises(RetryTimeoutError):
retrier.run(task, 2, 4, foo='bar')
Expand All @@ -130,8 +132,8 @@ def test_retrier_run_max_timeout(MagicMock):
assert task.call_count >= 1
task.assert_called_with(2, 4, foo='bar')

assert retrier.attempts >= 1
assert isinstance(retrier.error, ValueError)
assert retrier.attempts == 2
assert isinstance(retrier.error, NotImplementedError)


def test_retrier_istimeout():
Expand Down