Migrations have bases=(NewBase,) for models with custom metaclass

[Craig de Stigter]

With this model:

from django.db import models
from django.db.models.base import ModelBase
from django.utils import six


class MetaZork(ModelBase):
    pass


class BlibbityBlorg(six.with_metaclass(MetaZork, models.Model)):
    ZerbyBlugg = models.CharField(max_length=1)

django-admin makemigrations produces the following:

    operations = [
        migrations.CreateModel(
            name=b'BlibbityBlorg',
            fields=[
                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
                (b'ZerbyBlugg', models.CharField(max_length=1)),
            ],
            options={
            },
            bases=(django.db.models.base.NewBase,),
        ),
    ]

NewBase is a six artifact; the correct base would be models.Model.

Originally reported by petkostas on django-mptt: ​https://github.com/django-mptt/django-mptt/issues/302

[Craig de Stigter]

Test project that shows the problem

[Simon Charette]

Can you make sure the following patch works for you ​https://github.com/django/django/pull/2567.

[Craig de Stigter]

This works for the testproject I submitted, but doesn't work for mptt because __module__ is different:

(Pdb) base
<class 'mptt.models.NewBase'>
(Pdb) base.__module__
'mptt.models'

I'm not sure why six is setting this module differently; it's nothing special that mptt is doing AFAIK.

Doing some quick research into the differences, I'll post back shortly.

[Craig de Stigter]

Well, I still don't understand why this happens, but it seems that the NewBase doesn't always have the __module__ you expect. So maybe the code shouldn't check for that.

ipdb> MuppettZarkity.mro()
[<class 'testproject.testapp.models.MuppettZarkity'>, <class 'mptt.models.MPTTModel'>, <class 'mptt.models.NewBase'>, <class 'django.db.models.base.Model'>, <class 'django.db.models.base.NewBase'>, <type 'object'>]
ipdb> BlibbityBlorg.mro()
[<class 'testproject.testapp.models.BlibbityBlorg'>, <class 'testproject.testapp.models.ZerbyJarb'>, <class 'django.db.models.base.NewBase'>, <class 'django.db.models.base.Model'>, <class 'django.db.models.base.NewBase'>, <type 'object'>]

However they do consistenly have nothing much in their .__dict__ so perhaps that's a better check than .__module__:

                if (base.__name__ == 'NewBase' and
                    any((not attr.startswith('__') for attr in base.__dict__))):

[Simon Charette]

It doesn't work in the mttp case because they skip ModelBase.__new__ ​here, it would work if they called super(MPTTModelBase, cls).__new__ instead.

Unsure how to deal correctly with this. ​I've proposed six to replace their with_metaclass implementation with ​the one Flask uses in order to prevent those NewBase artifacts. I ran the full Django test suite with this modified version and the NewBase hacks removed and it fully passed.

Fixing the issue at this level makes more sense me than trying to work around it by __mro__ and __dict__ checks.

[Ben Davis]

I went ahead and created a ​pull request on six for this issue. Hopefully if it's merged in we can quickly merge a fix for Django as well.

[Simon Charette]

Your pull request seems to have been merged into six so I went forward and updated ​my PR to break changes into two commits: one that backport the new version of with_metaclass to our vendored version of six and a second one that adds a regression test case for the issue reported here.

[Aymeric Augustin]

Ship it!

[Simon Charette <charette.s@…>]

In 390f888745803a2f6c75c5a22402daf1221f8e29:

Fixed #22447 -- Make sure custom model bases can be migrated.

Thanks to cdestigter for the report.

[Simon Charette <charette.s@…>]

In cda5745df0e9301c65e13f552ee19a4bf0490997:

[1.7.x] Fixed #22447 -- Make sure custom model bases can be migrated.

Thanks to cdestigter for the report.

Backport of 390f888745 from master