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 test suite with this modified version 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