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.8 support #356

Closed
chhantyal opened this issue Jan 19, 2015 · 17 comments
Closed

Django 1.8 support #356

chhantyal opened this issue Jan 19, 2015 · 17 comments
Milestone

Comments

@chhantyal
Copy link

I am alpha testing Django 1.8

Meta API is formalised in 1.8 and as a result, method get_fields_with_model is deprecated.

That method is used here

for fld in model._meta.get_fields_with_model()

Because of this, project which is on 1.7 is not able to to run on 1.8. Please refer to migration docs https://docs.djangoproject.com/en/1.8/ref/models/meta/#migrating-old-meta-api

@craigds
Copy link
Member

craigds commented Jan 19, 2015

I don't see this result when running the tests (tox -e py27-master). Could you supply a test model that shows the problem?

@chhantyal
Copy link
Author

Hey thanks for quick response.

I created a test app here https://github.com/chhantyal/mptt-test

With model https://github.com/chhantyal/mptt-test/blob/master/mptt_test/app/models.py

Just clone, install requirements and run 'makemigrations' or any other commands. It will give error.

Here is traceback http://dpaste.com/246NP6F

This is issue with PyPi version as well as master branch

@craigds
Copy link
Member

craigds commented Jan 19, 2015

Interesting. Seems this happens because django 1.8 no longer lets you call ._meta.get_fields before the models have been fully loaded.

It's possible this code is all obsolete - from my brief reading of the ModelBase code it looks like all fields should be added to the model class before TreeManager.init_from_model() is called, so we shouldn't need to call get_fields() at all. Hopefully I'll have time in the next few days to investigate this.

If that doesn't pan out, we could probably work around this by doing TreeManager.init_from_model() in a AppConfig.ready() hook, but that might have it's own problems (and it necessitates two very separate code paths for django 1.6 and 1.7+)

@carltongibson
Copy link

I bumped into this playing with 1.8a1.

A quick work-around:

  1. Change model superclass to django.models.Model.
  2. Implement AppConfig.ready and call mptt.register there.

This seems to work OK, for the now.

@chhantyal
Copy link
Author

@carlospalol wow, that looks very clever. Thanks. At least I can work with 1.8 until official support comes.

@claudep
Copy link
Contributor

claudep commented Jan 28, 2015

@mazing
Copy link

mazing commented Jan 29, 2015

Does anyone have a work-around?

@craigds
Copy link
Member

craigds commented Feb 2, 2015

Current status: waiting to see what the outcome of that Django ticket is before implementing a fix.

@craigds craigds added this to the 0.7 milestone Feb 8, 2015
@craigds
Copy link
Member

craigds commented Feb 11, 2015

The upstream ticket has been closed (wontfix). Seems a possible solution is to call TreeManager.init_from_model() from the AppCache.ready signal on django 1.8.

craigds added a commit that referenced this issue Feb 11, 2015
Still not quite working, but it's a step in the right direction.

re #356
@craigds
Copy link
Member

craigds commented Feb 11, 2015

a note re this fix: AppConfig.ready() is actually not a suitable place for this, because we need to do this init before that's called so we can determine what model to add the tree fields on.

But it turns out we don't need to inspect the fields at all to do what we need - just need to find the topmost concrete model base in the mro() which is a MPTTModel subclass. This simplifies things quite a bit so I went with that.

Thanks for the bug reports. I don't intend to backport django 1.8 support to 0.6.x, but a 0.7 release probably isn't too far off.

@craigds craigds changed the title Django 1.8 : get_fields_with_model is deprecated Django 1.8 support Feb 11, 2015
@chhantyal
Copy link
Author

Thank you. That's great - we will be fine using master branch for now as Django 1.8 is not yet released 👍

@claudep
Copy link
Contributor

claudep commented Mar 28, 2015

As Django 1.8 is around the corner, it would be great if an 1.8-compatible release could be published on PyPI.

@chhantyal
Copy link
Author

Hi @craigds
Now 1.8 is released, I think it's time to update PyPi?

@ttanner
Copy link

ttanner commented Apr 2, 2015

HEAD works fine for me with Django 1.8. please release it

@hassek
Copy link

hassek commented Apr 2, 2015

Yep, waiting for this too :)

@craigds
Copy link
Member

craigds commented Apr 3, 2015

Done :)

@claudep
Copy link
Contributor

claudep commented Apr 4, 2015

Thanks Craig, awesome!

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

No branches or pull requests

7 participants