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

Measure elapsed time after function call #130

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lbenezriravin
Copy link

This patch fixes what I consider to be unexpected behavior:

Steps to reproduce

import time

import backoff


@backoff.on_exception(backoff.constant, RuntimeError, jitter=None, max_time=1)
def foo():
    print("Running")
    time.sleep(2)
    raise


def real_time():
    start = time.time()
    try:
        foo()
    except:
        pass
    print(time.time() - start)

real_time()

Observed behavior

$ poetry run python foo.py 
Running
Running
5.004697322845459

Expected behavior
When I specify the max_time argument, I expect the worst-case upper bound for execution time to be the specified timeout + the execution time of a single function call. Moreover, I would not expect retries to trigger if the give up condition is already fulfilled after the first function execution.

This behavior appears to be caused by collected elapsed for the details before the target function is executed. This patch should fix both of the above bugs by swapping that ordering.

Lihu Ben-Ezri-Ravin added 2 commits July 21, 2021 15:12
When I specify the `max_time` argument, I expect the worst-case upper
bound for execution time to be the specified timeout + the execution
time of a single function call. Moreover, I would not expect retries to
trigger if the give up condition is already fulfilled after the first
function execution.

The existing behavior appears to be caused by collected `elapsed` for
the `details` before the target function is executed. Both of the above
bugs should be fixed by swapping that ordering.
@fiendish
Copy link

This would have addressed #186

@Kirusi
Copy link

Kirusi commented Jan 31, 2023

I forked the library as https://github.com/Kirusi/improved_backoff. If you depend on properly working max_time feel free to use it.

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.

3 participants