Opened 8 years ago

Closed 8 years ago

#27337 closed Bug (invalid)

Convoluted MTI with abstract model mixin fails on PY2, works on PY3

Reported by: Tim Heap Owned by: nobody
Component: Database layer (models, ORM) Version: 1.10
Severity: Normal Keywords: python2, mti py2
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This is a rather convoluted situation, but with the set up below, PY2 fails but PY3 works. For context, this is roughly the same model set up used in Wagtail:

  • A Page class with custom Metaclass,
  • Developers extend Page using MTI to make custom page types
  • Mixins can be used for adding common functionality from e.g. plugins.
from django.db import models
from django.utils import six
from django.test import TestCase

class ConvolutedMTIWithAbtractMixinTests(TestCase):
    def test_convoluted_mti(self):

        class PageBase(type(models.Model)):
            """
            Wagtail's metaclass does some things, but nothing related.
            This empty metaclass still spoils things for some reason?
            """

        class Page(six.with_metaclass(PageBase, models.Model)):
            """The base page model. Every page should inherit from this."""
            url = models.CharField(max_length=255)

            class Meta:
                app_label = 'tests'

        class FancyMixin(models.Model):
            """A model mixin with extra functionality, including extra model fields."""
            fancy_field = models.CharField(max_length=10)

            class Meta:
                abstract = True

        class MyPage(FancyMixin, Page):
            """My custom page type. Extends Page using MTI, and mixes in FancyMixin"""
            content = models.CharField(max_length=255)

            class Meta:
                app_label = 'tests'

        # The page_ptr from MyPage to Page is created.
        self.assertTrue(hasattr(MyPage, 'page_ptr'))

        # The other fields are all there
        self.assertTrue(MyPage._meta.get_field('url'))
        self.assertTrue(MyPage._meta.get_field('fancy_field'))
        self.assertTrue(MyPage._meta.get_field('content'))

        # The page_ptr field exists as a parent pointer to page.
        self.assertIn(Page, MyPage._meta.parents)
        self.assertIsInstance(MyPage._meta.parents[Page], models.OneToOneField)

        # But Django on py27 gets confused, and 'page_ptr' is not 'registered'
        # (There is no nice _meta.has_field, so try: catch will do.)
        try:
            MyPage._meta.get_field('page_ptr')
        except FieldDoesNotExist:
            self.fail("MyPage.page_ptr field is missing!")

I stuck this test in tests/model_inheritance/tests.py when testing this out, but the tests can probably go anywhere. For some tests failing in the wild, this Travis CI build (https://travis-ci.org/takeflight/wagtail/builds/167013485) of some patches to Wagtail shows this happening on Django 1.8, 1.9 and 1.10 at least, but only on Python 2.7.

I've poked around a little bit to see if I could work out what is going on, but I am not familiar enough with the model internals to make much headway.

Change History (5)

comment:1 by Tim Graham, 8 years ago

Keywords: py2 added
Triage Stage: UnreviewedAccepted

I'm not going to spend any time investigating the issue, but I'll assume the report is valid and accept the ticket. We're accepting Python 2-specific bug fixes until mid-January 2017 (1.11 alpha), at which point we'll close any unresolved issues that only affect Python 2 as wontfix as master will become Python 3 only.

in reply to:  1 comment:2 by Tim Heap, 8 years ago

I will eagerly look forward to the day we can all drop Python 2 support.

comment:3 by Aymeric Augustin, 8 years ago

Thanks for the bug report anyway! :-)

comment:4 by Andrew Nester, 8 years ago

Hi guys!

I was able to reproduce this behavior just in plain Python 2.7
You can check my question on Stack Overflow for more details.
http://stackoverflow.com/questions/40134852/metaclass-inheritance-in-python-2-and-python-3

comment:5 by Aymeric Augustin, 8 years ago

Resolution: invalid
Status: newclosed

Closing as Django doesn't seem to be at fault here.

Note: See TracTickets for help on using tickets.
Back to Top