Consider replacing hasattr in data adapters #11

Open
BradMclain opened this Issue Feb 25, 2013 · 2 comments

Projects

None yet

2 participants

@BradMclain
Member

Currently data adapters use hasattr() to check for the existence of attributes on the persistent object, however hasattr fails if any Exception is thrown in the property/attribute.

This makes debugging difficult as it can hide errors in properties by treating them as non-existent attributes instead of throwing the correct exception.

An alternative approach would be to use something like the following, explicitly checking for AttributeError:

try:
getattr(someObject, 'someProperty')
except AttributeError:
print "Doesn't exist"
else
print "Exists"

This error is also present in the SQLAlchemy data adapter where it hides any DetachedInstanceError exceptions that are thrown when trying to access out of session objects.

@devraj devraj was assigned Aug 31, 2013
@devraj devraj closed this May 16, 2014
@devraj devraj reopened this May 16, 2014
@devraj
Member
devraj commented Jun 7, 2014
try:
   getattr(persistent_object, attribute_key)
Exception, exp:
   continue

does not work for platforms like AppEngine because NDB raises this exception if the property wasn't set for a particular object. This does not allow us to distinguish between a non existent properties vs not set properties.

SQLAlchemy might deal with it differently and it might be a per adapter solution

@BradMclain BradMclain modified the milestone: 2.0.x, 2.0.0 Nov 29, 2014
@devraj devraj modified the milestone: 2.0.1 Jan 18, 2015
@devraj devraj modified the milestone: 2.0.1, 2.0.2 Feb 17, 2015
@devraj devraj modified the milestone: 2.0.2, 2.0.3 Feb 25, 2015
@BradMclain BradMclain modified the milestone: 2.0.3, 2.0.4, 2.0.5 Feb 26, 2015
@BradMclain BradMclain modified the milestone: 2.0.5, 2.0.6, 2.0.7 Mar 5, 2015
@BradMclain BradMclain added a commit that referenced this issue Nov 9, 2016
@BradMclain BradMclain potential fix for #11 f254e9a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment