Python code to retry function

Question

I am trying to write a python function which retries a given function until given time or function returns True with given delay.

I have written the following function, but was thinking if there is a better way to do it?

Example function I want to retry for

def is_user_exist(username):
 try:
 pwd.getpwnam(username)
 log.info("User %s exist" % username)
 return True
 except KeyError:
 log.exception("User %s does not exist." % username)
 return False

My retry function

def retry(func, *func_args, **kwargs):
 retry_count = kwargs.get("retry_count", 5)
 delay = kwargs.get("delay", 5)
 while retry_count > 1:
 if func(*func_args):
 return
 log.debug("waiting for %s seconds before retyring again")
 sleep(delay)
 retry_count = retry_count - 1
 return func(*func_args)

• Mast: Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers .
• Dan Oberlam: @PatrickMevzek unless I misread that post, it doesn't seem possible to do with context managers
• jpmc26: In my experience, retry functions are a code smell to begin with.
• Dan Oberlam: @jpmc26 why is that? There are plenty of situations where retrying automatically is valid (networking being the most obvious example)
• jpmc26: @Dannnno It's usually an indication you're doing asynch wrong, like calling sleep is. The OP's specific example is local: if the user isn't already there, why will it be there in the near future? In the best case, they've got some process running asynchronously that might create it. But the more reliable way to do this is to wait for an entry in a queue after the user is created. If you're talking about something low level like implementing TCP, you have a point. But most code today is written at a higher level, and failure at these levels usually indicates a retry isn't going to work.

Answer 1

I like all of Ev. Kounis' answer, so I'll add some higher level details.

Let it be truth-y

Right now you aren't strictly requiring func to return True or False, just something truth-y or false-y. This is fine, and Pythonic. I think you would benefit from pointing that out in a docstring (which you should also write). Additionally, it may be worthwhile to return the actual value of func(*func_args) instead of True or False; this way you can actually get a value from the function if you wanted to take advantage of the truth-yness of the value.

Better kwargs support

You also don't allow any keyword arguments to be passed to the function - the function might support them, so you should to. I would promote the two keyword arguments you pull out of kwargs to explicit keyword arguments, with appropriate defaults (5 in your case).

Exceptions

It's weird to me that this function does have any concept of retrying after exceptions. I wouldn't want you to do something like this, for what I hope are obvious reasons

for _ in range(retry_count):
 try:
 if func(*func_args):
 return True
 except:
 pass
 log.debug("wating for %s seconds before retrying again")
 sleep delay)

However, in many cases I suspect you would know what exceptions you might expect (for example, a network timeout or connectivity blip) and that you might want to be handled by the retrying framework. To this end, I think something like this could be nice:

def retry(func, *args, retry_count=5, delay=5, allowed_exceptions=(), **kwargs):
 for _ in range(retry_count):
 try:
 result = func(*args, **kwargs)
 if result: return result
 except allowed_exceptions as e:
 pass

This obviously isn't a complete implementation; I left out some of the other pieces you have, and it behaves oddly if it fails on the last iteration, but it should be enough to start.

Fancy stuff

I think we could also get more value from this if it was a decorator. Then consumers of a function don't even need to know if they want it to retry or not; they just call their function and see that it works, and whether or not it was retried becomes irrelevant. Don't forget to use functools.wraps to preserve metadata.

import functools
def retry(retry_count=5, delay=5, allowed_exceptions=()):
 def decorator(f):
 @functools.wraps(f)
 def wrapper(*args, **kwargs):
 for _ in range(retry_count):
 # everything in here would be the same
 return wrapper
 return decorator

Then you can enable retrying for everyone, like so:

@retry(retry_count=5, delay=5)
def is_user_exist(username):
 try:
 pwd.getpwnam(username)
 log.info("User %s exist" % username)
 return True
 except KeyError:
 log.exception("User %s does not exist." % username)
 return False

Really fancy stuff

Why block when you're waiting? You could be doing so much more (this is for Python 3.5 and above) using asyncio. There isn't built-in support for this before that, but I know there are asynchronous frameworks that should be able to accomplish the same task.

By awaiting an asynchronous function that just runs for n seconds, you achieve the same goal but allow other asynchronous functions to do work while you're just waiting. Note that depending on the event loop you might end up waiting for slightly more or less time.

I also cleaned up the issues I mentioned about handling exceptions; it now always returns the result of the function if it has one, and it'll re-raise the exception without losing any traceback if there was one. That also uses a Python 3 only feature; I've left a comment for how to do it in Python 2.

Note, I'm not as familiar with asyncio as I never got to do any serious dev there, so I might not have this piece of code exactly correct; the theory should be sound though.

import functools
import asyncio 
def retry(retry_count=5, delay=5, allowed_exceptions=()):
 def decorator(f):
 @functools.wraps(f)
 async def wrapper(*args, **kwargs):
 result = None
 last_exception = None
 for _ in range(retry_count):
 try:
 result = func(*func_args, **kwargs)
 if result: return result
 except allowed_exceptions as e:
 last_exception = e
 log.debug("Waiting for %s seconds before retrying again")
 await asyncio.sleep(delay)
 if last_exception is not None:
 raise type(last_exception) from last_exception
 # Python 2
 # import sys
 # raise type(last_exception), type(last_exception)(last_exception), sys.exc_info()[2]
 return result
 return wrapper
 return decorator

• salvob: Awesome post! What if I want to get a value through the retries? For example If I have a last_page to check for every iteration inside the function. And when I retry I don't want to do it all over again. But starting from that last_page. Is it possible?
• Dan Oberlam: @salvob no clue! If you get it to work feel free to post your code for review :)

Answer 2

The only thing I noticed is that retry has a potentially inconsistent behavior.

Let me explain:

def retry(func, *func_args, **kwargs):
 retry_count = kwargs.get("retry_count", 5)
 delay = kwargs.get("delay", 5)
 while retry_count > 1:
 if func(*func_args):
 return
 log.debug("waiting for %s seconds before retyring again")
 sleep(delay)
 retry_count = retry_count - 1
 return func(*func_args)

If func is successful while checking it inside the while, None will be returned. On the other hand, if it is successful outside the while, it will return whatever func returns (in your example True). You do not want to have that..

So I would propose a slight re-coding:

def retry(func, *func_args, **kwargs):
 retry_count = kwargs.get("retry_count", 5)
 delay = kwargs.get("delay", 5)
 for _ in range(retry_count): # all tries happen in the loop
 if func(*func_args):
 return True # if we succeed we return True
 log.debug("waiting for %s seconds before retyring again")
 sleep(delay) 
 return False # if we did not, we return False

---

You can get a bit fancier if you want to by subsituting the above for loop with this:

for _ in range(retry_count):
 res = func(*func_args) or log.debug("waiting for %s seconds before retyring again")
 if res is None:
 sleep(delay)
 else:
 return True

Note that I am assuiming here that log.debug returns None but it does not really matter as long as it does not return True.

• Dan Oberlam: Strictly speaking, it might not return True outside of the loop; it'll return whatever func does, which is probably something that can look truth-y or false-y.
• Ma0: @Dannnno Correct, I am just referring to OP's example func.
• Eric Duminil: You don't pass kwargs to func, do you?

Answer 3

Theory

Your retry function is very similar to the structure of any.

Keeping only the essential, you could write retry as :

any(func(*func_args) for _ in range(count))

Code

If you want kwargs, log and sleep, you can write:

def retry(func, *func_args, **kwargs):
 count = kwargs.pop("count", 5)
 delay = kwargs.pop("delay", 5)
 return any(func(*func_args, **kwargs)
 or log.debug("waiting for %s seconds before retyring again" % delay)
 or time.sleep(delay)
 for _ in range(count))

Notes

• log.debug and time.sleep are both falsy, so func or log or time is truthy if and only if func is truthy.
• dict.pop is needed to extract count and delay from kwargs. They would get passed to func otherwise.

Complete code

import time
import pwd
import logging as log
def is_user(username):
 try:
 pwd.getpwnam(username)
 log.info("User %s exist" % username)
 return True
 except KeyError:
 log.error("User %s does not exist." % username)
 return False
def retry(func, *func_args, **kwargs):
 count = kwargs.pop("count", 5)
 delay = kwargs.pop("delay", 5)
 return any(func(*func_args, **kwargs)
 or log.debug("waiting for %s seconds before retyring again" % delay)
 or time.sleep(delay)
 for _ in range(count))
retry(is_user, 'username', count=10, delay=0.5)

• Dan Oberlam: I'm not a huge fan of using any here; if you have a very limited set of functionality you want then it works fine and is very simple, but understanding why you're doing it that way is somewhat unintuitive. I also think that oring together everything is somewhat ugly - that should be a separate function imo.
• Eric Duminil: @Dannnno: Thanks for the comment. To each his own, I guess. OP's is basically interested in knowing if funcwill work at least once in count times, and that's pretty much what any is for. All the rest is optional decoration IMHO.

Answer 4

Here are some problems with your current setup:

1. The function being retried can't take keyword arguments. This can be fixed pretty easily for the most part, but allowing the function to take arguments like delay will be more complicated.
2. To use retries, you have to call retry(is_user_exist, username, ...). This makes it harder to avoid repetition.
3. You may end up with the same traceback appearing in the logs 5 times in a row.
4. retry requires that the function returns things in a certain way. This will be annoying for some functions when you have to add extra lines, and terrible for other functions where a falsy return value is valid.

I suggest a decorator such as the one below. I wrote this decorator a while ago and have happily used it in several places. The idea is similar to Dannnno's answer, but I only retry after exceptions and don't pay attention to return values.

def retry(num_attempts=3, exception_class=Exception, log=None, sleeptime=1):
 def decorator(func):
 @functools.wraps(func)
 def wrapper(*args, **kwargs):
 for i in range(num_attempts):
 try:
 return func(*args, **kwargs)
 except exception_class as e:
 if i == num_attempts - 1:
 raise
 else:
 if log:
 log.error('Failed with error %r, trying again', e)
 sleep(sleeptime)
 return wrapper
 return decorator

Here is some usage in real code:

from requests.exceptions import ConnectionError
@retry(5, ConnectionError, log)
def request(self, method, url='', **kwargs):
 ...

Here's another example where I only retry at the call site, rather than changing the definition of the function:

retry(5, Exception, log)(ftp.get)(path, destination)

Your case is a bit unusual because an exception is involved but you ultimately don't want to raise it. You could perhaps rewrite your code as follows:

if is_user_exist(username):
 process_existing_user()
else:
 process_nonexistent_user()

becomes:

try:
 retry(5, KeyError, log)(pwd.getpwnam)(username)
except KeyError:
 process_nonexistent_user()
else:
 process_existing_user()

If you have other functions which you want to retry when a condition is false that don't involve exceptions, you could explicitly raise an exception:

class StuffNotFound:
 pass
@retry(exception_class=StuffNotFound)
def process_stuff():
 stuff = get_stuff():
 if not stuff:
 raise StuffNotFound()
 process(stuff)

Ultimately the problem with this question is that we're talking about how to write a very generic and widely applicable function, but we only have one use case to apply it to. If you have other examples of code you want to retry, this discussion can be more informed.

Answer 5

I'm surprised no one mentioned tenacity library.

It does exactly what you want + there is an already builtin implementation for asyncio. You can also use parameters(waiting then retrying, waiting x number of time, etc.) that are quite permissive.

• Gloweye: Probably because it's not stdlib and including a module just for something you can easily code yourself like this is to high a price for that.

Answer 6

You can replace your while loops and retry_count countdown with a simple for loop via range()

def retry(func, *func_args, **kwargs):
 retry_count = kwargs.get("retry_count", 5)
 delay = kwargs.get("delay", 5)
 for _ in range(retry_count):
 if func(*func_args):
 return
 log.debug("waiting for %s seconds before retyring again")
 sleep(delay)
 return func(*func_args)

• janos: The point your making was already made in another, earlier answer. I suggest to either edit this answer so that it adds new value, or else delete it.
• user171782: @janos I saw that after posting it but I cannot figure out how to delete this post on the mobile app. I will when I get on the desktop site