Some of my functions use a "fail_silently" flag. It is used in the following way:
def test(a, b, c=1, fail_silently = False)
try:
return int(a) + c
except:
if fail_silently:
return None
raise
Therefore, if there is an error, we catch it and fail gracefully. The key here is that it can be toggled dynamically by whoever is calling it.
I am using this for a many different functions and class methods and thought to make it a decorator.
There are a few problems:
- I want to be able name the flag "raise_exception" or "fail_silently" or "whatever"...
- The flag may or may not have a default value
- The flag may or may not be passed in (usually not)
- It needs to work with class methods too
Thus my decorator needs to look for (assuming the flag is called "fail_silently") the flag in the following locations in this order
- **kwargs (the passed in function arguments), simple dictionary get on flag
- *args - get the positional argument of the flag, then scan for index in args (which might not be there)
- Get the default value of the flag from the function
The problem is the code is now getting really messy and has many points of failure.
def try_except_response(parameter = 'fail_silently'):
def real_decorator(func):
@functools.wraps(func)
def wrapper(*args, **kwargs):
try:
return func(*args, **kwargs) # the function itself,
except: # if it raises an Exception!
# check to see if the Flag is in passed in kwargs!
if kwargs.get(parameter)== True:
return None
elif kwargs.get(parameter) == False:
raise
else:
# Flag is not in kwargs, check to see if it is in args
function_args, vargs, kewords, defaults = inspect.getargspec(func) # get the index of argument of interest
try:
argument_index = function_args.index(parameter) # get the index of argument of interest
except ValueError:
raise ValueError('The inputted decorator for "fail_silently" was not found! The behavior will not work.')
if len(args) > argument_index: # check to see if it is inputted
keyword = args[argument_index] # get the value!!
if keyword == True:
return None
elif kwargs == False:
raise
else:
raise ValueError('The "fail_silently" flag did not return a value that we understand. Must be True or False')
else:
# not in args either! let's go get the default value.
# TODO MORE STUFF!!!
raise
raise
raise
return wrapper
What is the best way this can be cleaned up? Or alternatives? I am thinking about implementing a version of locals() that will create a dictionary of all parameters passed into the function and then check from there...
There are simplifying assumptions we can make, but that would require the person writing the function to know something ('flag is always last argument, or default value is always False...'), but I am hoping to get the decorator to be as transparent as possible so it just "works" and lets the function be a function (or class method)
4 Answers 4
I'm not a fan of this design. @TheBlackCat is right. Don't do this.
If you are to do this, rethink your design. A more sensible one might look like
import functools
from contextlib import suppress
def try_except_response(func):
@functools.wraps(func)
def wrapper(*args, fail_silently, **kwargs):
if fail_silently:
with suppress(Exception):
return func(*args, **kwargs)
else:
return func(*args, **kwargs)
return wrapper
The key principle here is KISS.
Note that I'm suppressing Exception
, not BaseException
, since catching SystemExit
and KeyboardInterrupt
is almost always a bad idea.
If you want to make fail_silently
a default argument, just use another wrapper:
def default_fail_silently(default):
def decorator(func):
@functools.wraps(func)
def wrapper(*args, fail_silently=default, **kwargs):
return func(*args, fail_silently=fail_silently, **kwargs)
return wrapper
return decorator
Then you can do
@default_fail_silently(False)
@try_except_response
def test(a, b, c=1)
return int(a) + c
This does mean there's more boilerplate, since it exists for each piece of functionality you add, but the logic itself is an order of magnitude simpler.
-
\$\begingroup\$ i totally agree here. trying to simply the logic. is there a way it can be simplified where the default_fail_silently flag can be checked at function evaluation? that way each caller can set their own whether or not to supress \$\endgroup\$user1639926– user16399262015年06月29日 21:50:42 +00:00Commented Jun 29, 2015 at 21:50
-
1\$\begingroup\$ @user1639926 "that way each caller can set their own whether or not to supress" → I'm not sure what you mean by this. Why is what I gave not sufficient? You do, say,
test(a, b, fail_silently=False)
. \$\endgroup\$Veedrac– Veedrac2015年06月29日 21:58:15 +00:00Commented Jun 29, 2015 at 21:58 -
\$\begingroup\$ Note that
contextlib.suppress
is new in python 3.4 \$\endgroup\$Eric– Eric2015年07月27日 21:54:07 +00:00Commented Jul 27, 2015 at 21:54
Your code looks really nice, I saw a few parts that might be improved:
if kwargs.get(parameter)== True:
: binary operators are suggested, by PEP8, to have a space pre&proceeding the operator=
function_args, vargs, kewords,
: is that last one a misspell?if keyword == True:
: If you're testing whether the variable is empty, as PEP8 says, you can useif keyword
Other than that, your code looks really nice, good work!
I'm trying to get this code to work (Python 2.7) and it doesn't work copied from your question. try_except_response
needs to return real_decorator
, I think, as well as include the imports.
Now I've tried it - it seems unhelpful that a function can have 'fail_silently' as an argument which this looks for. If you are editing the function to put that in the argument list, why not edit it to put the silencer in the function while you are there?
Anyway, saying nothing of whether this is a good idea or a good approach, the code you have posted has some unnecessary comments which repeats what the line of code says - this is no help to someone who can read Python - and no help to someone who can't:
except: # if it raises an Exception!
keyword = args[argument_index] # get the value!!
argument_index = function_args.index(parameter) # get the index of argument of interest
And some which don't explain clearly, or are misleading:
if len(args) > argument_index: # check to see if it is inputted
#Check ... what?
function_args, vargs, kewords, defaults = inspect.getargspec(func) # get the index of argument of interest
#This doesn't get an index.
Regarding the level of indentation; you can take this structure:
if kwargs.get(parameter)== True:
return None
elif kwargs.get(parameter) == False:
raise
else:
# All this code is indented
# ...
and make it:
# Was the parameter passed in as a kwarg?
if parameter in kwargs:
if kwargs.get(parameter):
return None
else:
raise
# All this code is not indented
You can take:
if len(args) > argument_index:
# big indented block
else:
raise
and reverse the logic:
# Was the parameter provided to the function call?
if len(args) <= argument_index: # no
raise
# not-indented code
After searching for the parameter value in the keyword args and not finding it, if you find the value in args, why do you call it ... keyword?
And you can probably remove the "except ValueError" code if you finish writing the TODO behaviour for if searching fails.
# Is the parameter a named parameter in the function definition?
function_args, vargs, kewords, defaults = inspect.getargspec(func)
if not parameter in argument_index:
raise
Then the inner wrapper
becomes a try / except, and four clearly separate sections.
def wrapper(*args, **kwargs):
try:
return func(*args, **kwargs)
except:
# Was the parameter passed in as a kwarg?
if parameter in kwargs:
if kwargs.get(parameter):
return None
else:
raise
# Is the parameter one of the normal function parameters?
function_args, vargs, kewords, defaults = inspect.getargspec(func)
if not parameter in function_args:
raise
# Was the parameter provided to the function call?
argument_index = function_args.index(parameter)
if len(args) <= argument_index: # no
raise
# Can we use the provided value?
keyword = args[argument_index]
if keyword is True:
return None
elif kwargs is False:
raise
else:
raise ValueError('The "fail_silently" flag did not return a value that we understand. Must be True or False')
# If we could not do anything else, re-raise by default.
raise
Following on from a previous answer, we can simplify even further to:
from contextlib import *
def try_except_response(func):
@functools.wraps(func)
def wrapper(*args, fail_silently, **kwargs):
with suppress(Exception) if fail_silently else ExitStack():
return func(*args, **kwargs)
return wrapper
Explore related questions
See similar questions with these tags.
int()
raising an Exception, and an ignore_exception decorator which allows you to specify particular exceptions to be ignored: stackoverflow.com/questions/2262333/… \$\endgroup\$