Issue 2550952
Created on 2017-09-14 20:35 by ezio.melotti, last changed 2017-09-20 02:25 by rouilj.
Messages | |||
---|---|---|---|
msg6015 | Author: [hidden] (ezio.melotti) | Date: 2017-09-14 20:35 | |
https://sourceforge.net/p/roundup/code/ci/36630a062fb5 introduced a check that uses inspect.getargspec(). In our instance we were using a callable instance for a check ( https://hg.python.org/tracker/python- dev/file/545a7d7ef9e4/schema.py#l251 ) and while updating roundup we got the failure "TypeError: <__builtin__.may_view_spam instance at 0x7f2ac7629bd8> is not a Python function" since getargspec() only works with Python functions and not arbitrary callables. We fixed this on our end by replacing the callable class with a function that uses a closure to store the klassname, but the fact that arbitrary callables are no longer supported is technically a backward incompatible change that you might want to fix in Roundup. FWIW getargspec(inst.__call__) works, but there might be better way to fix it. |
|||
msg6016 | Author: [hidden] (rouilj) | Date: 2017-09-14 23:20 | |
Hi Ezio: In message <1505421313.77.0.0767788137936.issue2550952@psf.upfronthosting.co.za>, Ezio Melotti writes: > >https://sourceforge.net/p/roundup/code/ci/36630a062fb5 introduced a check >that uses inspect.getargspec(). In our instance we were using a callable >instance for a check ( https://hg.python.org/tracker/python- >dev/file/545a7d7ef9e4/schema.py#l251 ) and while updating roundup we got >the failure "TypeError: <__builtin__.may_view_spam instance at >0x7f2ac7629bd8> is not a Python function" since getargspec() only works >with Python functions and not arbitrary callables. Hmm, from https://bugs.python.org/issue20828 we get something like: _WrapperDescriptor = type(type.__call__) _MethodWrapper = type(all.__call__) _ClassMethodWrapper = type(int.__dict__['from_bytes']) def get_callable_argspec(fn): if inspect.isfunction(fn) or inspect.ismethod(fn): inspectable = fn elif inspect.isclass(fn): inspectable = fn.__init__ elif hasattr(fn, '__call__'): inspectable = fn.__call__ else: inspectable = fn if isinstance(fn, (_WrapperDescriptor, _MethodWrapper, _ClassMethodWrapper)): raise ValueError('unsupported callable {!r}'.format(fn)) try: return inspect.getargspec(inspectable) except TypeError: raise This seems to be best, but could it break on python if they once again deprecate getfullargspec per https://docs.python.org/3/library/inspect.html Changed in version 3.4: This function is now based on signature(), but still ignores __wrapped__ attributes and includes the already bound first parameter in the signature output for bound methods. Changed in version 3.6: This method was previously documented as deprecated in favour of signature() in Python 3.5, but that decision has been reversed in order to restore a clearly supported standard interface for single-source Python 2/3 code migrating away from the legacy getargspec() API. From https://www.programcreek.com/python/example/87828/inspect.getfullargspec [From project xonsh-master, under directory xonsh, in source file inspectors.py.] def getargspec(obj): """Wrapper around :func:`inspect.getfullargspec` on Python 3, and :func:inspect.getargspec` on Python 2. In addition to functions and methods, this can also handle objects with a ``__call__`` attribute. """ if safe_hasattr(obj, '__call__') and not is_simple_callable(obj): obj = obj.__call__ return inspect.getfullargspec(obj) if ISPY3K else inspect.getargspec(obj) They have a fair number of local functions there safe_hasattr and is_simple_callable but using something like this should fix your immediate issue right Ezio? >We fixed this on our end by replacing the callable class with a function >that uses a closure to store the klassname, but the fact that arbitrary >callables are no longer supported is technically a backward incompatible >change that you might want to fix in Roundup. I don't remember any of the examples in the docs that use anything other than an actual function. So as you correctly state, technically it's changed incompatibly. However it was never advertised that it would work 8-). However there is probably no statement that arbitrary callables shouldn't work so let's try to fix it for your use case at least. >FWIW getargspec(inst.__call__) works, but there might be better way to >fix it. There are some code ideas above. My thoughts are: Add function findargspec to anypy/getarspec_.py that wraps getargspec/getfullargspec and checks for __call__ and uses that if present. Use this function in place of inspect.getargspec in security.py Remove the comment to remove the array subscript notation as it looks like the field name changes for the 4 fields in common between the tuple returned by getargspec and getfullargspec If I get this done would you be able to add a couple of test cases to the test suit that exercises both forms of __call__ to see if I got the code correct? Have a great day. |
|||
msg6017 | Author: [hidden] (ezio.melotti) | Date: 2017-09-15 00:07 | |
> Hmm, from https://bugs.python.org/issue20828 we get something like: > > ... > if isinstance(fn, > (_WrapperDescriptor, _MethodWrapper, _ClassMethodWrapper)): > raise ValueError('unsupported callable {!r}'.format(fn)) > ... This is probably a bit overkill if getargspec is only going to be used for "check functions" in schema.py. I don't think people are going to write C extensions or other complex solutions to check for permissions :) > [From project xonsh-master, under directory xonsh, in source file > inspectors.py.] > > ... > if safe_hasattr(obj, '__call__') and not is_simple_callable(obj): > obj = obj.__call__ > > return inspect.getfullargspec(obj) if ISPY3K else inspect.getargspec(obj) This seems a good compromise between functionality and simplicity and should work with callable instances (like the one we had). > Add function findargspec to anypy/getarspec_.py that wraps > getargspec/getfullargspec and checks for __call__ and uses that > if present. > > Use this function in place of inspect.getargspec in security.py This is an option, especially considering that you are (finally) moving on Python 3 :) However, have you considered other non-LBYL approaches that don't require using getargspec()? Unfortunately the only alternative that comes to mind is using a try/except, except that the TypeError you would get is not specific enough and you might end up masking unrelated errors -- even if you check for the error message. > If I get this done would you be able to add a couple of test cases to > the test suit that exercises both forms of __call__ to see if I got > the code correct? Right now I'm still busy trying to update our bug tracker, so probably not. Adding these test cases should be quite straightforward though, since you can convert any existing: def func(args): ... to class Foo: def __call__(self, args): ... and pass Foo() instead of func. > Have a great day. You too :) |
|||
msg6018 | Author: [hidden] (rouilj) | Date: 2017-09-20 02:25 | |
Fixed in commit rev:c94fd717e28c using modified version of first example in msg6016. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2017-09-20 02:25:11 | rouilj | set | status: new -> fixed versions: + devel messages: + msg6018 assignee: rouilj components: + API resolution: fixed |
2017-09-15 00:07:36 | ezio.melotti | set | messages: + msg6017 |
2017-09-14 23:20:35 | rouilj | set | nosy:
+ rouilj messages: + msg6016 |
2017-09-14 20:35:13 | ezio.melotti | create |