Retry being called despite successful execution of function

[chetnaB]

On using retry_unless_exception_type , retry is always called (despite a successful run) until the exception(s) is(are) encountered. However, correct behavior with this condition should be to retry only if a function has failed due to some exception other than the ones mentioned in the retry condition.
e.g. This is the function definition, wherein I want to retry the function only if the exception is not among psycopg2.ProgrammingError and NameError and either the time since last retrial is less than 1 min, or the attempt count is less than 3:

@retry(stop=(stop_after_delay(60) | stop_after_attempt(2)),
        reraise=True,
        retry=retry_unless_exception_type((psycopg2.ProgrammingError, NameError)))
def run_query(query_variable):
    cursor.execute(query_variable)
    print cursor.rowcount
    print "Done"

and here's the execution log, which shows that function was retried twice despite successful runs:

53059
Done
53059
Done
Traceback (most recent call last):
  File "update_offer_base_item_mapping.py", line 22, in <module>
    run_query(a)
  File "/Users/grofers/Documents/cb/py2venv/lib/python2.7/site-packages/tenacity/__init__.py", line 241, in wrapped_f
    return self.call(f, *args, **kw)
  File "/Users/grofers/Documents/cb/py2venv/lib/python2.7/site-packages/tenacity/__init__.py", line 330, in call
    start_time=start_time)
  File "/Users/grofers/Documents/cb/py2venv/lib/python2.7/site-packages/tenacity/__init__.py", line 297, in iter
    raise retry_exc.reraise()
  File "/Users/grofers/Documents/cb/py2venv/lib/python2.7/site-packages/tenacity/__init__.py", line 137, in reraise
    raise self
tenacity.RetryError: RetryError[<Future at 0x109cf4190 state=finished returned NoneType>]

[jd]

You want retry=retry_unless_exception_type((psycopg2.ProgrammingError, NameError)) | retry_if_result(lambda r: True)) AFAIU.

[yuvadm]

@jd this is a serious pitfall, I had the same expectation about retry_unless_exception_type and got bit by this awkward behavior.

Furthermore, your suggested fix doesn't fix the problem, since the second part of the | condition will never be evaluated as the first part is already too eager to retry.

A proper workaround that does work is something like this retry=retry_if_exception(lambda e: not isinstance(e, FooError))

[asqui]

Hmmm, so it seems the problem here is that there is an ambiguity in the interpretation of:
retry=retry_unless_exception_type(T1, T2)

When reading this code, there are two possible interpretations:

1. Retry until an exception of type T1 or T2 is raised.
2. If an exception is raised, retry unless it is of type T1 or T2.

1 is what actually happens (as is clearly described by the docstring of retry_unless_exception_type: """Retries until an exception is raised of one or more types.""")

2 is perhaps a more intuitive interpretation of the method name, when seen in context, such as the example given by the OP:

@retry(stop=(stop_after_delay(60) | stop_after_attempt(2)),
        reraise=True,
        retry=retry_unless_exception_type((psycopg2.ProgrammingError, NameError)))

The subtlety is that when we talk about retrying "unless exception type [is raised]", this statement does not specify what happens if no exception is raised. The difference we're talking about here is the difference between an exception was raised (but was not of the specified type(s)) vs. no exception was raised. In both these cases it is true that an exception of the specified type(s) has not been raised.

Looking at all the retry_* helpers we currently have:

• retry_if_exception
• retry_if_exception_type
• retry_unless_exception_type -- "Number 1" behaviour, as detailed above
• retry_if_result
• retry_if_not_result
• retry_if_exception_message
• retry_if_not_exception_message

retry_if_not_exception_message has the same behaviour -- if no exception is raised, it will still retry.

@jd I'm guessing that changes to the naming or behaviour of retry_unless_exception_type is undesirable at this stage?

How about a new method that gives the "Number 2" behaviour:

• retry_if_exception_not_of_type

@yuvadm Would this name adequately communicate the behaviour that you are after, and do you think you would intuitively pick up this method rather than retry_unless_exception_type for your use-case? (If not, can you suggest a better name?)

[yuvadm]

@asqui your reasoning makes sense to me, I understand why changing the name at this point would be undesirable. retry_if_exception_not_of_type sounds very reasonable, especially if it's properly documented, so that future users will know exactly which decorator, and thus which behavior, to choose from.

[william-silversmith]

I concur. A new method with that name would resolve the issue.

[jd]

Works for me. :)

[william-silversmith]

Thanks @jd! Would you like one of us to create a PR? Thanks for all your hard work on this repo. We were able to process over a petabyte of neuroscience data with flaky network connections because of it.

[jd]

@william-silversmith sure go ahead