Django 1.8

[kaedroho]

This PR adds Django 1.8 support into Wagtail

[kaedroho]

I think it's worth trying to get this into 0.9. Django 1.8 is scheduled for the end of this month and 1.0 might be a long wait.

[tomdyson]

I agree. How much work do you think it is? Could someone else help?

On Tuesday, 17 March 2015, Karl Hobley notifications@github.com wrote:

I think it's worth trying to get this into 0.9. Django 1.8 is scheduled
for the end of this month and 1.0 might be a long wait.

Reply to this email directly or view it on GitHub
#1060 (comment).

sent from my phone

[kaedroho]

The work on getting Wagtail working on Django 1.8 is mostly done. Mainly waiting on the following PRs to be merged:

• Fixed #24495 -- Allow unsaved instance in ForeignKey check to be bypassed django/django#4334
• Django 1.8 support jazzband/django-taggit#297
• Django 1.8 support django-modelcluster#20
• Django 1.8 #1060

Reviews much appreciated!

We also need to think about how we're going to get RoutablePageMixin working with the changes made to Djangos url configuration. Possibly an API breakage may be required here...

[kaedroho]

Would probably be best to redefine this in wagtail so an older version of modelcluster can still be used under Django 1.7

[mx-moth]

I tried to combine Django 1.8 (rc1), Wagtail, and Jinja templates, and came across a problem: Jinja templates fail when the context contains a self key! I made a Django bug for it: https://code.djangoproject.com/ticket/24538#ticket

[kaedroho]

@timheap thanks for testing!

[huxley]

Just noticed that the Django ticket @timheap references is now marked wontfix because Tim has found that self is a special variable in Jinja2. Wondering how difficult it would be to switch Wagtail to use a different variable than self?

[chrxr]

Hi @timheap, please could you review this. Django 1.8 is due to be released tomorrow, and we're keen to get support into Wagtail 1.0. Thanks!

[mx-moth]

This looks good! I am keen to see Wagtail in Django 1.8. Wagtail 1.0 will be huge at this rate! Some thoughts on this pull request:

---

tox.ini tests py27-dj18-postgres, but not -mysql or -sqlite, as these are commented out. Is this deliberate?

tox.ini contains some git URLs for dependencies still. I understand that these changes are not yet merged so this is required, just dont forget them before release!

---

Feature detection seems to be used in many places, as opposed to version detection. In my opinion, this makes the code much harder to read and reason about, as well as hiding the true intention of the code behind comments. For example get_related_model:

def get_related_model(rel):
    # In Django 1.7 and under, the related model is accessed by doing: rel.model
    # This was renamed in Django 1.8 to rel.related_model. rel.model now returns
    # the base model.
    return getattr(rel, 'related_model', rel.model)

and TestRouting.test_request_serving:

# Django 1.8+
if hasattr(used_template, 'template'):
    used_template = used_template.template

In my opinion, checking the version is much clearer, straight forward, less error prone, and more explicit:

def get_related_model(rel):
    # In Django 1.7 and under, the related model is accessed by doing: rel.model
    # This was renamed in Django 1.8 to rel.related_model.
    if DJANGO_VERSION < (1, 8):
        return rel.model
    else:
        return rel.related_model

if DJANGO_VERSION >= (1, 8):
    used_template = used_template.template

This has the added benefit of being self documenting, as it is clear that the functionality differs from version to version. It also works in all cases, and is already in use where feature detection does not work (e.g. migrations involving content types: defaults={'name': 'document'} if DJANGO_VERSION < (1, 8) else {}). Finally, when Django 1.7 support is dropped in the future, searching the codebase for the tuple (1, 8) should return all instances of hacks done to support the 1.7/1.8 differences.

---

I did not review the pull requests for django-taggit and django-modelcluster.

[kaedroho]

tox.ini tests py27-dj18-postgres, but not -mysql or -sqlite, as these are commented out. Is this deliberate?

Yeah.. Unfortunately, the more we make Travis do the slower it gets. We're testing all the database backends on Python 3.4, but just Postgres on Python 2.7.

tox.ini contains some git URLs for dependencies still. I understand that these changes are not yet merged so this is required, just dont forget them before release!

Maybe I'll just include them for the Django 1.8 tests for now and test Django 1.7 with released versions.

Feature detection seems to be used in many places, as opposed to version detection. In my opinion, this makes the code much harder to read and reason about, as well as hiding the true intention of the code behind comments.

That looks better, will change

[kaedroho]

This may require some tweaks now that the API is merged

[kaedroho]

Ready for merge. There's still a load of deprecation warnings, I'll solve those in another PR.

[kaedroho]

Django taggit 0.13.0 released with my PR merged. Still waiting on django-modelcluster. This PR can be merged at any time though.

[gasman]

Have cherry-picked af5cbaa, 229a666, 775da10, c2456b6, fc374bc - please rebase.

[gasman]

5f198fc, b43aff6, 55cd422, 4d8e31b all cherry-picked. 819b8cc rejected, because we're not yet ready to add 1.8 to the 'include' list (the RoutablePage failures are still outstanding as of this PR). Remaining commits to be addressed in a separate PR which I'll open shortly.