Skip to content

Commit

Permalink
ParseResource's need to know they've been loaded
Browse files Browse the repository at this point in the history
In __getattr__ we check to see if the object has been lazy loaded or not
before we try to do attribute lookups. However, once the attributes have
been loaded, we never record that.

So this changes _init_attrs, which actually does the loading, to record
that it's done so.

This change took a loop I had that was taking 8-10s to finish down to
.5-1s.
  • Loading branch information
dgadling committed Oct 6, 2014
1 parent a48cadb commit 2fde657
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions parse_rest/datatypes.py
Expand Up @@ -237,6 +237,7 @@ def __getattr__(self, attr):
def _init_attrs(self, args):
for key, value in six.iteritems(args):
setattr(self, key, ParseType.convert_from_parse(value))
self._is_loaded = True

def _to_native(self):
return ParseType.convert_to_parse(self)
Expand Down

4 comments on commit 2fde657

@dankrause
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems incorrect. The _init_attrs method only does the loading in the case where it was not lazy loaded. If a ParseResource is created with _is_loaded set to False, the purpose is to signal the __getattr__ method to actually do the loading by calling self.GET. This is useful when you only pass the objectId to __init__. If you never access the object, it's never loaded. If you do access an attribute of the object, it's first loaded, then _is_loaded is removed (not set to True), so that it doesn't fetch again.

With this commit, lazy loading fails entirely - if you create a ParseResource is with _is_loaded set to False, it's immediately set to True, and accessing any attributes fails, because they were never loaded.

If this commit fixed a problem you were experiencing, it could only be if you were passing _is_loaded = False, even though you actually also supplied all the attributes to __init__.

I think this commit should be reverted.

@dgadling
Copy link
Contributor Author

@dgadling dgadling commented on 2fde657 Nov 8, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dankrause
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can see where it actually does the lazy loading here, and it's clear that the it should not load more than once, and it should definitely never load on __getattr__ if _is_loaded was never set. Something else was going on in your case. When reverting this commit locally, I'm unable to reproduce any scenario where lazy loading kicks in without setting _is_loaded to false myself, or using a Pointer. I'm also unable to reproduce any scenarios where lazy loading happens more than once.

It looks like this isn't thread-safe, so accessing the same unloaded Pointer from multiple threads might cause it to load more than once, but it doesn't sound like you were running into that.

@dgadling
Copy link
Contributor Author

@dgadling dgadling commented on 2fde657 Nov 8, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.