There has been a problem with zope.interface's verifyObject function that occurs in conjunction with Python properties: when verifyObject checks for the presence of an object's attribute, it does so by using hasattr() which in turn tries a getattr() call. If the attribute is implemented as a property, this may raise an exception which will be masked by hasattr() due to a bare except: clause. One scenario where this is a problem is a property that performs some component lookup which will succeed at application runtime but not in unit tests where verifyObject is commonly used. While it has also been argued that behaviour so complex that it may raise an Exception should not be implemented as a property in the first place, we believe that verifyObject should handle this case better than it currently does. A possible fix would be to add an additional check for a data descriptor on the object's class if the hasattr() test returns False. Are there any objections against modifying verifyObject in this way? Thomas -- Thomas Lotze · tl@gocept.com gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany http://gocept.com · tel +49 345 1229889 0 · fax +49 345 1229889 1 Zope and Plone consulting and development
On Oct 15, 2008, at 3:27 AM, Thomas Lotze wrote:
There has been a problem with zope.interface's verifyObject function that occurs in conjunction with Python properties: when verifyObject checks for the presence of an object's attribute, it does so by using hasattr() which in turn tries a getattr() call. If the attribute is implemented as a property, this may raise an exception which will be masked by hasattr() due to a bare except: clause. One scenario where this is a problem is a property that performs some component lookup which will succeed at application runtime but not in unit tests where verifyObject is commonly used.
While it has also been argued that behaviour so complex that it may raise an Exception should not be implemented as a property in the first place, we believe that verifyObject should handle this case better than it currently does. A possible fix would be to add an additional check for a data descriptor on the object's class if the hasattr() test returns False.
Are there any objections against modifying verifyObject in this way?
I would change it to just use getattr rather than hasattr. try: getattr(ob, name) except AttributeError: return False ... Jim -- Jim Fulton Zope Corporation
Jim Fulton <jim@zope.com> wrote:
I would change it to just use getattr rather than hasattr.
try: getattr(ob, name) except AttributeError: return False ...
This doesn't handle the case that the attribute exists as a property but raises an AttributeError when trying to produce its value. Thomas -- Thomas Lotze · tl@gocept.com gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany http://gocept.com · tel +49 345 1229889 0 · fax +49 345 1229889 1 Zope and Plone consulting and development
Thomas Lotze wrote:
Jim Fulton <jim@zope.com> wrote:
I would change it to just use getattr rather than hasattr.
try: getattr(ob, name) except AttributeError: return False ...
This doesn't handle the case that the attribute exists as a property but raises an AttributeError when trying to produce its value.
Yes, but this is still better than hasattr() which eats all exceptions. I think eating AttributeErrors is fine because to the user of 'ob', it would seem that ob.attr wouldn't exist anyway if it threw an AttributeError (even if that exception was about something else, it's still an AttributeError). I suggest doing marker = object() return getattr(ob, name, marker) is marker rather than return hasattr(ob, name)
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Thomas Lotze wrote:
Jim Fulton <jim@zope.com> wrote:
I would change it to just use getattr rather than hasattr.
try: getattr(ob, name) except AttributeError: return False ...
This doesn't handle the case that the attribute exists as a property but raises an AttributeError when trying to produce its value.
What is the difference? Any property which raises an attribute error is broken, and does not satisfy the contract of the interface: it might as well not be there at all. On a de gustibus note, I would have spelled it without the try: return getattr(ob, name, ob) is not ob Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFI9hhp+gerLs4ltQ4RAvYYAJkBpvcecqFmmSf9c9KMtPITnocOgQCghD71 irczDNf47QMt2qXB6LuBPKw= =Atrg -----END PGP SIGNATURE-----
Jim Fulton wrote:
I would change it to just use getattr rather than hasattr.
try: getattr(ob, name) except AttributeError: return False ...
Given the controversy about our original proposal, I think I'll just implement Jim's suggestion. I'll do so as soon as possible. -- Thomas
On Thu, Oct 23, 2008 at 08:57:30AM +0200, Thomas Lotze wrote:
Jim Fulton wrote:
I would change it to just use getattr rather than hasattr.
try: getattr(ob, name) except AttributeError: return False ...
Given the controversy about our original proposal, I think I'll just implement Jim's suggestion. I'll do so as soon as possible.
Thank you. Marius Gedminas -- Shift happens. -- Doppler
Thomas Lotze wrote at 2008-10-15 09:27 +0200:
There has been a problem with zope.interface's verifyObject function that occurs in conjunction with Python properties: when verifyObject checks for the presence of an object's attribute, it does so by using hasattr() which in turn tries a getattr() call. If the attribute is implemented as a property, this may raise an exception which will be masked by hasattr() due to a bare except: clause. One scenario where this is a problem is a property that performs some component lookup which will succeed at application runtime but not in unit tests where verifyObject is commonly used.
While it has also been argued that behaviour so complex that it may raise an Exception should not be implemented as a property in the first place, we believe that verifyObject should handle this case better than it currently does. A possible fix would be to add an additional check for a data descriptor on the object's class if the hasattr() test returns False.
Are there any objections against modifying verifyObject in this way?
I fear your must describe your proposed change more precisely: When your problem is the stated use case: "verifyObject" fails because something necessary for the interface to be properly implemented is missing in the test but available in a real environment, I would say: that is very fine: Do not change "verifyObject" but give the test instead the resources expected to be available in the real environment. When you want to provide a better (more informative) exception (not "fails to implement" but "ComponentLookupError"), then I am with you. -- Dieter
"Dieter Maurer" <dieter@handshake.de> wrote:
I fear your must describe your proposed change more precisely:
Nothing to be afraid of here ;o)
When your problem is the stated use case: "verifyObject" fails because something necessary for the interface to be properly implemented is missing in the test but available in a real environment,
Yes, this is what I'm talking about.
I would say: that is very fine: Do not change "verifyObject" but give the test instead the resources expected to be available in the real environment.
I think this line of reasoning intermingles two different things to be tested: the existence of an implementation of an attribute (as a data descriptor, whatever its getter method tries to do), and whether the attribute's value is sensible at any given point of time. You can compare this to a plain attribute promised by an interface: if it exists on an object that claims to provide the interface, verifyObject will be happy. The attribute's particular value at the time verifyObject looks at it may still be something very stupid from the point of view of the application. The latter is the subject of a different test, not one of interface implementation. To put it differently: If I implement a class and give its instances all attributes defined by a given interface then I expect verifyObject to damn well see that those attributes are there, without having to do all sorts of stuff in a completely different part of the code just in order to satisfy verifyObject's implementation. I'm aware that this is purely from the point of view of the user and does not take into account any design discussions that may have taken place when verifyObject was created.
When you want to provide a better (more informative) exception (not "fails to implement" but "ComponentLookupError"), then I am with you.
This would be better than the current behaviour, but it would still lie because of the difference between the existence of an implementation of the attribute in question and its successful execution in a given set of circumstances. Thomas -- Thomas Lotze · tl@gocept.com gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany http://gocept.com · tel +49 345 1229889 0 · fax +49 345 1229889 1 Zope and Plone consulting and development
Thomas Lotze wrote at 2008-10-15 20:55 +0200:
"Dieter Maurer" <dieter@handshake.de> wrote:
I fear your must describe your proposed change more precisely:
Nothing to be afraid of here ;o)
When your problem is the stated use case: "verifyObject" fails because something necessary for the interface to be properly implemented is missing in the test but available in a real environment,
Yes, this is what I'm talking about.
I would say: that is very fine: Do not change "verifyObject" but give the test instead the resources expected to be available in the real environment.
I think this line of reasoning intermingles two different things to be tested: the existence of an implementation of an attribute (as a data descriptor, whatever its getter method tries to do), and whether the attribute's value is sensible at any given point of time.
You can compare this to a plain attribute promised by an interface: if it exists on an object that claims to provide the interface, verifyObject will be happy. The attribute's particular value at the time verifyObject looks at it may still be something very stupid from the point of view of the application. The latter is the subject of a different test, not one of interface implementation.
I do not follow your argumentation: An attribute it not there because there is a descriptor, it is only there when the descriptor provides a value: I have seen descriptors in Zope3 that are only there to remove an attribute that might otherwise be inherited -- by unconditionally raising "AttributeError". In this case, the existance of the particular descriptor precisely means that the attribute is missing. Moreover, when an attribut is implemented by a descriptor, I am very much interested that its "__get__" does not raise an exception. If this is not the case, I would like to be warned early; if possible even by a unit test. This means that I am against your proposed change.
To put it differently: If I implement a class and give its instances all attributes defined by a given interface then I expect verifyObject to damn well see that those attributes are there,
Computed attributes introduce a new quality: while a standard attribute is either present or absent, a computed attribute may also raise an exception (and behave exactly like an absent attribute) on access. You want to treat those attributes as present, I do not.
I'm aware that this is purely from the point of view of the user and does not take into account any design discussions that may have taken place when verifyObject was created.
I, too, am a user (and was not involved in the design of "verifyObject"). Nevertheless, I do not share your point of view.
.... This would be better than the current behaviour, but it would still lie because of the difference between the existence of an implementation of the attribute in question and its successful execution in a given set of circumstances.
Use a different method to verify your notion of adherence with an interface. Do not change "verifyObject" for this. -- Dieter
Hi, On Thu, 2008-10-16 at 18:45 +0200, Dieter Maurer wrote:
I do not follow your argumentation: An attribute it not there because there is a descriptor, it is only there when the descriptor provides a value: I have seen descriptors in Zope3 that are only there to remove an attribute that might otherwise be inherited -- by unconditionally raising "AttributeError". In this case, the existance of the particular descriptor precisely means that the attribute is missing.
Moreover, when an attribut is implemented by a descriptor, I am very much interested that its "__get__" does not raise an exception. If this is not the case, I would like to be warned early; if possible even by a unit test.
Then again, verifyObject is a *very* light way to spot simple errors. IMHO attributes and methods aren't that different in Python, as "using" both may result in exceptions. However, implementing a method of an interface doesn't check whether the method actually executes correctly -- only whether an implementation is present. Arguably, the check for an attribute would be sufficient if it checked whether an attribute implementation is around -- either by a simple attribute value or a descriptor providing that. There's another method: verifyClass. This definitely only checks the presence of an implementation. Thomas: There is an issue that we regularly see with verifyClass that makes us instanciated the objects and then use verifyObject. I don't remember what it was right now. Do you? Christian -- Christian Theune · ct@gocept.com gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany http://gocept.com · tel +49 345 1229889 7 · fax +49 345 1229889 1 Zope and Plone consulting and development
Christian Theune <ct@gocept.com> wrote:
Arguably, the check for an attribute would be sufficient if it checked whether an attribute implementation is around -- either by a simple attribute value or a descriptor providing that.
At this point, I guess the outcome of the discussion depends on whether it is considered legal or abuse to implement a data descriptor in such a way that it hides an attribute defined on a base class by raising an AttributeError.
There's another method: verifyClass. This definitely only checks the presence of an implementation.
To be more precise: the declaration of implementation of an interface as opposed to actual implementation of its attributes.
Thomas: There is an issue that we regularly see with verifyClass that makes us instanciated the objects and then use verifyObject. I don't remember what it was right now. Do you?
Not really, other than to avoid the case of a happy verifyClass() call with the application dying of a forgotten attribute implementation. Could there be classes we subclass that claim to implement an interface but don't fully do so until after instantiation? Just a guess... Thomas -- Thomas Lotze · tl@gocept.com gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany http://gocept.com · tel +49 345 1229889 0 · fax +49 345 1229889 1 Zope and Plone consulting and development
Thomas Lotze wrote at 2008-10-16 20:57 +0200:
Christian Theune <ct@gocept.com> wrote:
Arguably, the check for an attribute would be sufficient if it checked whether an attribute implementation is around -- either by a simple attribute value or a descriptor providing that.
At this point, I guess the outcome of the discussion depends on whether it is considered legal or abuse to implement a data descriptor in such a way that it hides an attribute defined on a base class by raising an AttributeError.
That is only one extreme case. Instead, the discussion outcome depends on whether an attribute is considered to be there when the descriptor raises an exception. As the attribute is not accessible in this case, I argue that it is not there: in this case, it does not have the wrong value, it has no value at all...
There's another method: verifyClass. This definitely only checks the presence of an implementation.
To be more precise: the declaration of implementation of an interface as opposed to actual implementation of its attributes.
Thomas: There is an issue that we regularly see with verifyClass that makes us instanciated the objects and then use verifyObject. I don't remember what it was right now. Do you?
Not really, other than to avoid the case of a happy verifyClass() call with the application dying of a forgotten attribute implementation. Could there be classes we subclass that claim to implement an interface but don't fully do so until after instantiation? Just a guess...
Indeed: as Python lacks a means to define instance attributes on the class, the presence of attributes can (reliably) only be checked on instances. -- Dieter
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Dieter Maurer wrote:
Thomas Lotze wrote at 2008-10-16 20:57 +0200:
Christian Theune <ct@gocept.com> wrote:
Arguably, the check for an attribute would be sufficient if it checked whether an attribute implementation is around -- either by a simple attribute value or a descriptor providing that. At this point, I guess the outcome of the discussion depends on whether it is considered legal or abuse to implement a data descriptor in such a way that it hides an attribute defined on a base class by raising an AttributeError.
That is only one extreme case.
Instead, the discussion outcome depends on whether an attribute is considered to be there when the descriptor raises an exception. As the attribute is not accessible in this case, I argue that it is not there: in this case, it does not have the wrong value, it has no value at all...
+1. A descriptor which raises AttributeError *denies* the existence of the attribute, period: if the contract / interface requires that the attribute exist, then such a descriptor violates the contract (under whatever circumstances it raises the error). The choice then becomes: - Satisfy the contract, e.g. by updating the descriptor to return some appropriate default in the cases where it now raises AttributeError. - Relax the contract by removing the attribute from the interface. In either case, 'verifyClass' and 'verifyObject' will behave as expected, without changes. Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFI+N5v+gerLs4ltQ4RAqqEAJ440ALN/9seugX+jiayKhv6htLFRQCgvw06 nyzQaM/HA/m8j7Mxd0FmXTM= =rgCs -----END PGP SIGNATURE-----
Christian Theune wrote at 2008-10-16 20:27 +0200:
... Then again, verifyObject is a *very* light way to spot simple errors. IMHO attributes and methods aren't that different in Python, as "using" both may result in exceptions.
Using attributes (not computed one) does not result in exceptions.
However, implementing a method of an interface doesn't check whether the method actually executes correctly -- only whether an implementation is present.
Of course, it does not -- because (in general) there is not a single call which would need to be checked but usually an unlimited number of calls. One may argue that a parameterless method is very similar to an attribute (Eiffel does this), but this is not the case in Python.
Arguably, the check for an attribute would be sufficient if it checked whether an attribute implementation is around -- either by a simple attribute value or a descriptor providing that.
Of course one can argue: Gocept argues for it and I argue against it.
There's another method: verifyClass. This definitely only checks the presence of an implementation.
It only checks methods and ignores everything else. For methods, it uses the same "getattr" as "verifyObject" does (in fact, both are implemented by the same function). And if the attribute is implemented by a property (of the class, i.e. the decriptor is on the metaclass), then the descriptors "__get__" is called (in the same way as for "verifyObject"). Instance properties (descriptor on the class) may not define methods (probably a bug). -- Dieter
"Dieter Maurer" <dieter@handshake.de> schrieb:
And if the attribute is implemented by a property (of the class, i.e. the decriptor is on the metaclass), then the descriptors "__get__" is called (in the same way as for "verifyObject"). Instance properties (descriptor on the class) may not define methods (probably a bug).
I don't understand what you're saying in that last sentence; can you elaborate? Viele Grüße, Thomas -- Thomas Lotze · tl@gocept.com gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany http://gocept.com · tel +49 345 1229889 0 · fax +49 345 1229889 1 Zope and Plone consulting and development
Thomas Lotze wrote at 2008-10-17 19:42 +0200:
"Dieter Maurer" <dieter@handshake.de> schrieb:
... Instance properties (descriptor on the class) may not define methods (probably a bug).
I don't understand what you're saying in that last sentence; can you elaborate?
"verifyObject/verifyClass" is likely not to handle the following case correctly: class I(Interface): def m(...): ... class C(object): implements(I) m = property(lambda self: lambda ...: ...) i.e. when a method (declared by the interface) is implemented by a property. -- Dieter
"Dieter Maurer" <dieter@handshake.de> schrieb:
"verifyObject/verifyClass" is likely not to handle the following case correctly:
class I(Interface): def m(...): ...
class C(object): implements(I) m = property(lambda self: lambda ...: ...)
i.e. when a method (declared by the interface) is implemented by a property.
Ah, I see. Thank you. Thomas -- Thomas Lotze · tl@gocept.com gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany http://gocept.com · tel +49 345 1229889 0 · fax +49 345 1229889 1 Zope and Plone consulting and development
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Dieter Maurer wrote:
Thomas Lotze wrote at 2008-10-17 19:42 +0200:
"Dieter Maurer" <dieter@handshake.de> schrieb:
... Instance properties (descriptor on the class) may not define methods (probably a bug). I don't understand what you're saying in that last sentence; can you elaborate?
"verifyObject/verifyClass" is likely not to handle the following case correctly:
class I(Interface): def m(...): ...
class C(object): implements(I) m = property(lambda self: lambda ...: ...)
i.e. when a method (declared by the interface) is implemented by a property.
Why would I want to do that, rather than using 'def m(self):'? Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFI+OQK+gerLs4ltQ4RAjhgAJ9hS2kbePps6Ka2yqK8gOF94XOmlgCg1HWc y/uPVTUWU3e7p6PZROToHLY= =UCfg -----END PGP SIGNATURE-----
Tres Seaver <tseaver@palladion.com> schrieb:
Dieter Maurer wrote:
class C(object): implements(I) m = property(lambda self: lambda ...: ...)
i.e. when a method (declared by the interface) is implemented by a property.
Why would I want to do that, rather than using 'def m(self):'?
- to win an obfucated-code contest - to get an additional closure for the method that is created each time the method is accessed Viele Grüße, Thomas Lotze -- Thomas Lotze · tl@gocept.com gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany http://gocept.com · tel +49 345 1229889 0 · fax +49 345 1229889 1 Zope and Plone consulting and development
Tres Seaver wrote at 2008-10-17 15:14 -0400:
...
"verifyObject/verifyClass" is likely not to handle the following case correctly:
class I(Interface): def m(...): ...
class C(object): implements(I) m = property(lambda self: lambda ...: ...)
i.e. when a method (declared by the interface) is implemented by a property.
Why would I want to do that, rather than using 'def m(self):'?
Why do you want to compute (non-method) attributes rather than define them directly on the class or set them on the instance? Indeed, defining methods by means of descriptors is not uncommon. "staticmethod" and "classmethod" are prominent examples. Fortunately for "verifyClass", they return "function" and "method" when resolved on a class. Would they not, "verifyClass" would fail (as in the "property" case). I have been wrong with respect to "verifyObject": "verifyObject" handles the case correctly. I hit a (minor) error when I made some tests to answer your question: from zope.interface import Interface, implements from zope.interface.verify import verifyObject, verifyClass
class I(Interface): ... def m(): pass ... class C(object): ... implements(I) ... @classmethod ... def m(): pass ... c=C() verifyObject(I, c) True verifyClass(I, C) True c.m() Traceback (most recent call last): File "<stdin>", line 1, in ? TypeError: m() takes no arguments (1 given)
I.e. the Zope3 "verify" does not catch that a classmethod needs an argument where the class can be passed in. -- Dieter
participants (8)
-
Christian Theune -
Dieter Maurer -
Jim Fulton -
Marius Gedminas -
Philipp von Weitershausen -
Thomas Lotze -
Thomas Lotze -
Tres Seaver