CODEWITHSUNDEEP
python-2.7
idanshmu
idanshmu

Reputation: 5251

Wrap every function of every class of a module

Objective

Wrap every function of every class of gspread module.

I know there are countless of posts on the subject and most unanimously instruct to use decorators. I'm not too familiar with decorators and felt like that approach is not as seamless as I hoped for. perhaps I didn't understand correctly.

But, I found this answer which "felt" like what I'm looking for.

(poor) Attempt

import gspread
from oauth2client.service_account import ServiceAccountCredentials
import os
import inspect

class GoogleSheetAPI:
    def __init__(self):
        f = os.path.join(os.path.dirname(__file__), 'credentials.json')
        os.environ.setdefault('GOOGLE_APPLICATION_CREDENTIALS', f)
        scope = ['https://spreadsheets.google.com/feeds',
                 'https://www.googleapis.com/auth/drive']
        credentials = ServiceAccountCredentials.from_json_keyfile_name(f, scope)
        self.client = gspread.authorize(credentials)
        self.client.login()

def SafeCall(f):
    try:
        print 'before call'
        f()
        print 'after call'
    except:
        print 'exception caught'
        return None

for class_name, c in inspect.getmembers(gspread, inspect.isclass):
    for method_name, f in inspect.getmembers(c, inspect.ismethod):            
        setattr(c, f, SafeCall(f)) # TypeError: attribute name must be string, not 'instancemethod'

g = GoogleSheetAPI()
spreadsheet = g.client.open_by_key('<ID>') # calls a function in gspread.Client
worksheet = spreadsheet.get_worksheet(0)  # calls a function in gspread.Spreadsheet
worksheet.add_rows(['key','value'])  # calls a function in gspread.Worksheet

Notes

  1. When I use the word "seamless" I mean that considering my code has many calls to many gspread functions, I want to change as little as possible. Using inspect/setattr seems like the perfect/seamless trick.

Upvotes: 0

Views: 149

Answers (1)

bruno desthuilliers
bruno desthuilliers

Reputation: 77902

There are three obvious issues with your code actually.

The first one is the TypeError - which is easy to solve FWIW: as the error message (raised by setattr() states, "attribute name must be string, not 'instancemethod'". And you're indeed trying to use f (the method itself) instead of method_name. What you want here is of course:

setattr(c, method_name, SafeCall(f))

The second issue is that your SafeCall "decorator" is NOT a decorator. A decorator (well, the kind of decorator you want here at least) returns a function that wraps the original one, your current implementation just calls the original function. Actually, it is almost what SafeCall should actually return. An example of a proper decorator would be:

def decorator(func):
    def wrapper(*args, **kw):
        print("before calling {}".format(func))
        result = func(*args, **kw)
        print("after calling {}".format(func))
        return result
    return wrapper

And finally, the third obvious issue is here:

except:
    print 'exception caught'
    return None

You certainly don't want this. This

1/ will catch absolutely everything (incuding SysExit, which is what Python raises on sys.exit() calls and StopIteration which is how iterators signals they are exhausted),

2/ discard all the very useful debugging infos - making it impossible to diagnose what actuall went wrong

3/ return something that can be plain unusable so you'll have to test the return value of each method call, and since you won't know what went wrong, you won't be able to handle the issue otherwise than printing "oops, something went wrong but don't ask me what nor where nor why" and exiting the program, which is definitly not better than letting the exception propagate - the program will crash in both cases, but at least if you leave the exception alone you'll have some hints on what caused the issue.

4/ or, much worse, return a valid return value for the method (yes, quite a few method are designed to change state and return None) so you won't even know something went wrong and happily continue execution - which is a sure way to have incorrect result and corrupted data.

5/ not to mention that the methods you're decorating that way are very probably calling each others and using (expected) exceptions internally (with proper exception handling), so you are actually introducing bugs in an otherwise working (or mostly working) library.

IOW, this is probably the worse antipattern you can ever think of...

Upvotes: 1

Related Questions