Add return-value/exception based backoff time

[sherifnada]

Closes #102

Take 2 of #122 this time using @bgreen-litl 's awesome suggestion to use .send.

The code is much cleaner and easier to use. The approach is to:

1. Pass the thrown exception (if using on_exception) or return value (if using on_predicate) to the wait time generator
2. The generator yields a value just like before, but now it also has the option of inspecting the exception/return value

To make this easy to use I added a new generator from_value which takes a callable whose input is the exception or decoratee-return-value, and whose return value is length of time to wait.

The consumer can use the new generator by doing:

def is_rate_limited(response):
  return response.status == 429

def wait_time_from_response(response):
  return int(response.headers['retry-after'])

@backoff.on_predicate(backoff.from_value, predicate=is_rate_limited, parser=wait_time_from_response)
def foo():
  return make_http_request()

Questions:

• I added one test in test_wait_gen.py for the new generator. Are there any additional test cases you'd recommend? Maybe something to verify that if the return value from the input callable is not a valid number (e.g: non-negative) that the appropriate exception is thrown?

If this approach looks good I'll add docs and any more tests you recommend.

[bgreen-litl]

I like this! I like how you set up backoff.from_value and the parser param.

I think this is on the right track. My one concern is:

Does this change the contract for the wait_gen parameter such that the generator function now must start with that naked yield? You updated expo, fibo, constant which is fine and good, but will this break existing custom wait generator functions? e.g.

def my_wait_generator():
    # do i need an empty `yield` here?

    yield from range(3)

@backoff.on_exception(my_wait_generator, MyException):
...

If so, is there a way to make this style of wait generator function work as well?

[sherifnada]

@bgreen-litl we could check if the parser argument was called and if so, assume that generator accepts the values. i.e: in common._init_wait_gen we can do:

def _init_wait_gen(wait_gen, wait_gen_kwargs):
    kwargs = {k: _maybe_call(v) for k, v in wait_gen_kwargs.items()}
    initialized = wait_gen(**kwargs)
    if 'parser' in wait_gen_kwargs:
        initialized.send(None)
        
    return initialized

Such a generator necessarily has to have a dangling yield at the beginning to accept the first value passed into it. It's not ideal, but unfortunately I don't think there is a way around the ugliness of having that first empty yield statement in an input-accepting generator since send can only be called with a non-None argument if the generator is waiting at a yield statement.

This way this is fully backwards compatible with any custom generators that don't live in this repo.

What I don't like about this is that if we ever change the word parser then things could go south, but we can write tests around this behavior to make sure it's regression-proof. What do you think?

[sherifnada]

Alternatively we could include the first exception/return in the wait_gen_kwargs but it will break any current generators that don't have an escape-hatch **kwargs in their arguments.

[sherifnada]

@bgreen-litl sorry to keep nagging :) any thoughts on this?

[bgreen-litl]

I apologize for the very delayed response. I've been swamped with other things.

On reflection, I think the benefit of this (a clean way to handle Retry-After) outweighs the potential backward compatibility issue. I don't think we should jump through any hoops testing for parser etc. It was always intended that you could supply a custom wait generator if desired, but it wasn't explicitly documented and I suspect it is not taken advantage of very much. Regardless, it will be very easy to update any custom wait generator for the new contract. We should just try to find a spot to mention it in the README.

Thanks again for this work.

[sherifnada]

Thanks @bgreen-litl, very exciting! I'll work on adding these docs soon.

[bgreen-litl]

I merged these commits to master. Thanks for this!

[sherifnada]

@bgreen-litl thank you and apologies for never coming back around to this! been a super hectic 3 months

[PLPeeters]

@bgreen-litl Any idea of when we can expect a new release with this change? I'd like to use this feature in multiple projects and would like to avoid specifying a Git URL in my requirements.txt if possible :)

[bgreen-litl]

This is available in backoff 2.0.0 via the backoff.runtime wait generator