New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update helpers.py #351

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@hagai26

hagai26 commented Aug 21, 2014

Fixes the error

UnboundLocalError: local variable 'attr' referenced before assignment

which happens when using SQLAlchemy's hybrid_property, for example.

Update helpers.py
Fixing a BUG of:
UnboundLocalError: local variable 'attr' referenced before assignment

happens when using sqlalchemy hybrid_property for example.

@jfinkels jfinkels added the bug label Aug 22, 2014

@jfinkels

This comment has been minimized.

Owner

jfinkels commented Aug 22, 2014

Can you please add a test case for this?

@hagai26

This comment has been minimized.

hagai26 commented Aug 22, 2014

Good idea, working on it.

hagai26 added some commits Aug 22, 2014

Adding hybrid_property to make some tests to fail
The previous commit fixed this fail.
@hagai26

This comment has been minimized.

hagai26 commented Aug 22, 2014

This is strange.
I am checking the tests on python2.6 and all tests passes on my machine (ubuntu).

@jfinkels

This comment has been minimized.

Owner

jfinkels commented Aug 22, 2014

Make sure you have the most recent versions of the necessary dependencies; compare with the versions that Travis CI installs as well.

The error is an exception caught here: https://github.com/hagai26/flask-restless/blob/master/flask_restless/views.py#L994

The exception is

Neither 'InstrumentedAttribute' object nor 'Comparator' object associated with Pet.owner has an attribute 'id'

hagai26 added some commits Aug 25, 2014

@hagai26

This comment has been minimized.

hagai26 commented Aug 29, 2014

Fixed.
The change in the tests will make them fail without the fix in the code.

@jfinkels

This comment has been minimized.

Owner

jfinkels commented Aug 29, 2014

This still needs a unit test (in a separate test function). Also, please squash all your commits into a single commit (using git rebase -i master).

I don't understand your comment:

The change in the tests will make them fail without the fix in the code.

Can you clarify this? Are you saying you can't add a test case for some reason?

return self.owner.id
else:
return None
@hybrid_owner_id.expression

This comment has been minimized.

@jfinkels

jfinkels Aug 29, 2014

Owner

Needs an empty line before this line.

def hybrid_owner_id(self):
if self.owner:
return self.owner.id
else:

This comment has been minimized.

@jfinkels

jfinkels Aug 29, 2014

Owner

The else: is redundant, remove it.

@hagai26

This comment has been minimized.

hagai26 commented Sep 2, 2014

I see that andir fixed it already. So it's better merging from there.

@hagai26 hagai26 closed this Sep 2, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment