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 await
ing 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