-
Notifications
You must be signed in to change notification settings - Fork 148
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
Backoff code inside function #18
Comments
I like this idea. In particular, the context manager variant of it. That said, I've tried to keep the API surface of this library as small as possible, and I'd definitely want to spend some time and make sure we're getting it right if we were going to make a non-trivial expansion of it. I do think it's possible to have a function act as both a decorator and a context manager and so I think the API you suggested is possible. But the implementation may vary or be limited depending on the python version in use. Right now we support all the way back to 2.6! (If think you probably know, but the way to do with with the current API is to create a thin wrapping function around the 3rd party function which you then decorate. If it's really just a 1 for 1, param for param wrapper it does end up seeming a bit awkward.) |
Yeah, workaround is @backoff.whatever(...)
def retry(*args, **kwargs):
return lib.func(*args, **kwargs) |
I spent a little time exploring the context manager variant of this. Unfortunately, it's not possible to retry blocks of code this way. See https://www.python.org/dev/peps/pep-0343/ - specifically the comments about Anonymous Block Statements under Motivation and Summary. Having thought about this more, I'd recommend one of two best practices for backoff:
|
It the problem of a for retry in backoff.on_exception(backoff.expo, TimeoutError, max_tries=8):
with retry:
library.some_function(yada, blah, foo, bar)
# end with
# end for The |
@bgreen-litl Could you consider adding this in the documentation? I believe context managers would be more natural, too bad they don't work for retrying (except if @luckydonald suggestion works). Anyway, I think many of us would appreciate seeing this explanation in the documentation. Thank you for this great library! 👍 |
@pquentin Thank you for the suggestion. I think adding these best practices to the documentation is a fine idea. That documentation is getting a bit crowded as a simple README, but I'll see if we can fit one more thing in before we need to blow it up into something bigger. Also, I haven't completely given up on the original context manager idea. I still think it's a good one. It would need to be structured a little differently than @luckydonald originally suggested, but I think there is a path to a context manager version of backoff that might work. I've had a few false starts on an implementation for it, but the idea I've been working with is something like this:
The important difference from the original suggestion is that the value of the context manager needs to be a function rather than an anonymous block of code, which won't work unfortunately. I have this issue under the 2.0 milestone, and I am still working on it. Any insights or ideas are appreciated. I do worry that my version of the context manager idea above might not be much better or more natural or succinct than the local scope decorated function I documented. I want to make sure that if there's a trade off in terms of complexity that it's actually worth it... |
(It looks like this specific issue is no longer under the 2.0 milestone, at least according to GitHub.) You're right that a local function sounds less magical and simpler even if it's slightly more verbose. And it's more well, TOOWTDI. :) |
@pquentin, to which of my suggestion did you refer? @bgreen-litl, What about the So 2 lines would be still less than writing it yourself every time: for retry in backoff.retry_on_exception(backoff.expo, TimeoutError, max_tries=5):
with retry:
# >>> do stuff here <<<
# end with
# end for Now manually: timeout = backoff.expo()
for try_count in range(5):
try:
# >>> do stuff here <<<
break # because executed successfully
expect TimeoutError as e:
if try_count == 5:
raise e
# end if
sleep(timeout.next())
# end try
# end for |
I just realized: from somewhere.library import some_function as _some_function
import backoff
from requests.exceptions import TimeoutError
some_function = backoff.on_exception(backoff.expo, TimeoutError, max_tries=8)(_some_function)
some_function(yada, blah, foo, bar) |
I just adopted the code example from #18 (comment) into IBM-Cloud/sql-query-clients#65 @luckydonald Thank you! |
Somewhat related: #210 proposes supporting wrapping a |
I am calling some functions of a lib i wanna backoff.
Problem is, I cannot modify the lib.
So I am looking for something like:
or
Not sure what is possible to do.
Edit (2017-04-27): Calling just one function can be solved like seen below
The text was updated successfully, but these errors were encountered: