-
Notifications
You must be signed in to change notification settings - Fork 155
Support predefined intervals wait generator #49
Support predefined intervals wait generator #49
Conversation
This allows
|
After the list item is consumed, the last item is yielded forever. |
1 similar comment
bgreen-litl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of this wait generator. Some comments inline.
| yield interval | ||
|
|
||
|
|
||
| def predefined(intervals=[1]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's usually not a good idea to put a mutable object (in this case a list) as a default in a function definition. If you haven't been bitten by this before (I have!) the behavior can be really surprising.
That list is going to get instantiated once at import time - you won't get a new instantiation for each initialization of the generator.
In [34]: def foo(vals=[1,2,3]):
...: while vals:
...: yield vals.pop()
In [35]: list(foo())
Out[35]: [3, 2, 1]
In [36]: list(foo())
Out[36]: []
| intervals: A list with the intervals to yield. | ||
| """ | ||
| while True: | ||
| if len(intervals): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather have the intervals be defined by an arbitrary sequence rather than requiring a mutable list. All the other places in the API which take a sequence are type-agnostic.
This way any iterable would work.
@backoff.on_exception(backoff.predefined, intervals=(1, 2, 3))
Including generators. And it would play nice with itertools. The equivalent of backoff.constant with interval=1:
@backoff.on_exception(backoff.predefined, intervals=itertools.repeat(1))
And I don't know why you'd want to do this, but you could:
@backoff.on_exception(backoff.predefined, intervals=itertools.cycle([1,2,3]))
We might still want to support the behavior you defined where the last item is repeated once the iterator is exhausted, so something like:
itr = iter(intervals)
while True:
try:
last = next(itr)
except StopIteration:
pass
yield last
But this has me thinking a little bit... what if instead, a StopIteration raised by the wait generator was actually a signal to trigger giving up?
@backoff.on_exception(backoff.predefined, intervals=[1,3,6])
In other words, the above definition would result in a maximum of 4 tries with those predefined intervals between them. I feel like this might be a convenient way to fully specify retry behavior with a built in give up condition.
litl#49 suggested the same thing by way of a new built in wait generator, but I think it makes sense to extend the constant wait generator to do the same thing, taking advantage of give up on StopIteration.
|
@ignaciovazquez I know it's been a while, but checkout out #63 if this is still of interest |
|
Looks great!
… On 27 Nov 2018, at 18:53, Bob Green ***@***.***> wrote:
@ignaciovazquez <https://github.com/ignaciovazquez> I know it's been a while, but checkout out #63 <#63> if this is still of interest
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#49 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AApcaJpboegFHyVpA-xuxD9G-3Ntl8aBks5uzbRIgaJpZM4SiHgo>.
|
litl#49 suggested the same thing by way of a new built in wait generator, but I think it makes sense to extend the constant wait generator to do the same thing, taking advantage of give up on StopIteration.
|
@ignaciovazquez I merged #63 Thank you for this PR and idea. I ended up implementing it slightly differently but I will credit you in the release notes for the feature. Sorry it took a while! |
This allows the user to specify custom internals via a list.
For example