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

Provide a generator decorator #58

Closed
proppy opened this issue Apr 14, 2017 · 8 comments
Closed

Provide a generator decorator #58

proppy opened this issue Apr 14, 2017 · 8 comments

Comments

@proppy
Copy link

proppy commented Apr 14, 2017

Would it make sense to have a @retry variant that better support generator functions?

Something like:

def retry_generator(max_retries, exc_type=Exception, exc_check=None):
    def wrap(gen):
        def wrapped(*args, **kwargs):
            retry = 0
            rpc_error = None
            while retry <= max_retries:
                try:
                    for i in gen(*args, **kwargs):
                        yield i
                    return
                except exc_type as e:
                    if exc_check and exc_check(e):
                        retry += 1
                        rpc_error = e
                        continue
                    raise
            raise rpc_error
        return wrapped
    return wrap
@jd
Copy link
Owner

jd commented Apr 15, 2017

How would you use it?

@proppy
Copy link
Author

proppy commented Apr 15, 2017

some_errors = (SomeError(code=42) for i in range(3))
@retry_generator(3, SomeError, lambda e: e.code == 42)
def unreliable_stream():
   yield 1
   yield 2
   raise some_errors.next()
   yield 3
list(unreliable_stream()) # [1, 2, 1, 2, 1, 2]

@jd
Copy link
Owner

jd commented Apr 15, 2017

I don't see how the result is useful here? If you expect [1, 2, 3], you probably don't want [1, 2, 1, 2, 1, 2] as a result.

@proppy
Copy link
Author

proppy commented Apr 15, 2017

@jd sorry for the lack of context, I think I abstracted the example too much :)

In my particular usage unreliable_stream represent a grpc.io streaming RPC call, I wanted to be able to use tenacity to retry the RPC call (by calling the generation function and restarting the iteration). But maybe it's too specific of a usecase?

See below:

def grpc_retry_unvailable(max_retries):
    """Retry decorator for gRPC streaming generator functions."""
    return retry_generator(max_retries, grpc.RpcError,
                           lambda e: e.code() == grpc.StatusCode.UNAVAILABLE)

@jd
Copy link
Owner

jd commented Apr 16, 2017

Iterators are particular, if you're ready to restart it as soon as it fails, it means you basically want it entirely. So you actually want list(generator()) to finish and be retried if it cannot be completed. So that's you want to retry, which can be summarized by the following example:

import tenacity

s = {'calls': 0}
def gen():
    s['calls'] += 1
    yield 1
    yield 2
    if s['calls'] == 1:
        raise Exception
    yield 3

r = tenacity.Retrying()
ret = r(lambda: list(gen()))
print(ret) # prints [1, 2, 3]

So I'm not sure it needs anything else here?

@proppy
Copy link
Author

proppy commented Apr 16, 2017

Maybe that's too specific to how gRPC (ab)use of generator for streaming, but the semantic is the following:

stream = stub.StreamingMethodCall(req) # initiate the streaming gRPC call
for resp in stream:
    doSomething(resp)

The stub.StreamingMethodCall() is lazy and never raise exceptions, but the iteration itself can; and in case of a failure we want to retry it from the beginning (i.e: call StreamingMethodCall).

This makes it challenging for me to use tenacity.Retrying since it doesn't really expect its callable argument to be a generator function, nor it is capable of yielding result back to the caller.

This could work if we wrap the whole logic consuming the generator with retry.

def callable():
     for i in gen():
        doSomethingWith(i)
retry(callable)

But I was hoping I could insert retry between the generator itself and the code consuming it (so that for example an intermediate library could expose both lazy evaluation; provide retry logic and be decoupled from the code consuming it).

for i in retry(gen):
   doSomethingWith(i)

Let me know if that makes sense :)

@jd
Copy link
Owner

jd commented Apr 16, 2017

Maybe the best solution would be a context manager such as suggested in #48

@proppy
Copy link
Author

proppy commented Apr 16, 2017

@jd You're right! I definitely agree that would provide a nice solution to this problem!

Feel free to close this issue ;)

@jd jd closed this as completed Apr 16, 2017
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

No branches or pull requests

2 participants