Skip to content
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

Django 1.10 support for FieldTracker #233

Merged
merged 3 commits into from
Aug 18, 2016
Merged

Conversation

jarekwg
Copy link
Contributor

@jarekwg jarekwg commented Aug 15, 2016

Fixes #232

So two things going on here.

First, as of Django 1.8, we should be using get_deferred_fields rather than manually probing for isinstance(field_obj, DeferredAttribute).

Second, as of Django 1.10, it seems inherited models don't pull through the deferred fields in __class__.__dict__. Have to consult the base class to get them. I have a feeling this might be a regression and will probs raise a ticket with django to make sure, but this bandaid should work for the time being. There's a slight niggle in that dj110 now supports overriding fields from the base class. I think I'm building the dict in the correct order so that stuff is overridden correctly, but there might need to be additional tests added to check for this (can open a new issue). Also, perhaps there's a way to get a hold of the deferred field that avoids the use of __class__.__dict__ altogether?

Tests all pass now so removed dj110 from allowed failures.

@jarekwg
Copy link
Contributor Author

jarekwg commented Aug 17, 2016

After some django digging, a handle to DeferredAttribute is now obtainable by accessing the field through the instance's class. I've updated the PR to use this implementation, as it's considerably neater.

- env: TOXENV=py27-django110
- env: TOXENV=py34-django110
- env: TOXENV=py35-django110
- env: TOXENV=py27-django_trunk
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd rather leave the trunk builds in allow_failures even if they pass currently. Once we get into the heat of 1.11 feature development, they will probably break again, and I don't want the build breaking due to changes outside model-utils' control. Django trunk build is just advance warning, it's not a blocker.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh True, will do!

@carljm carljm merged commit e3c53e1 into jazzband:master Aug 18, 2016
@jarekwg jarekwg deleted the upgrade/dj110 branch September 20, 2017 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants