AlterModelManagers migration is generated for all Models with custom Managers pointing to the default Django Manager

[Anthony King]

Working on a project with Models written with migrations generated by Django 1.8 (upgraded to 1.9), when running makemigrations, some unexpected migrations were generated.

...
migrations.AlterModelManagers(
    name='some_model',
    managers=[
        ('objects', django.db.models.manager.Manager()),
    ],
),

We've never run AlterModelManagers in the past. Instead of the Manager we define being added, the default Manager is being set.

I can add code examples + instructions later

[Anthony King]

Further investigation shows that if you have a manager without use_in_migrations set, it will explicitly set the default manager in AlterModelManagers instead of the previous behaviour of not creating a migration at all (ie, the migration treats the model as if no manager has been set at all).

As such, I'm going to remove the bug label, however it would be good to mention this in the release notes to avoid questions down the road.

[Loic Bistuer]

Hi cybojenix, could you still provide code examples? I'd like to ensure it's the ideal behavior.

[Markus Holtermann]

I briefly talked to cybojenix on IRC, this sounds like a bug to me.

Seen behavior:

class CustomManager(models.Manager):
    pass

class Model(models.Model):
    objects = CustomManager()

This creates a migration with a default manager though no manager is marked with use_in_migrations = True.

Expected behavior:

Unless there's >=1 manager with use_in_migrations = True don't actually add the manager to the state but only consider it was added. This prevents unnecessary migrations that don't add anything useful. Essentially, a ModelState with an empty managers dict is the same as defining a model without setting an explicit manager (neither default nor a custom one).

---

Marked as release blocker due to unexpected creation of migrations. I'd have wanted this fixed when I'd have noticed it while reviewing ​https://github.com/django/django/pull/6175

[Anthony King]

​https://github.com/django/django/pull/6635

[Matthew Schinckel]

I believe reinstating the code at ​https://github.com/django/django/commit/ed0ff913c648b16c4471fc9a9441d1ee48cb5420#diff-c8a3e1d248f8f5b10af7ac5b1d8479f4L516 is sufficient to fix this bug.

[Matthew Schinckel]

Replying to schinckel:

I believe reinstating the code at ​https://github.com/django/django/commit/ed0ff913c648b16c4471fc9a9441d1ee48cb5420#diff-c8a3e1d248f8f5b10af7ac5b1d8479f4L516 is sufficient to fix this bug.

Ah, not quite. This causes a deprecation warning in the manager inheritance stuff.

Perhaps the check could be included in the autodetector, instead of the ModelState.

[Matthew Schinckel]

Okay, I have a patch, but no tests: ​https://github.com/schinckel/django/commit/38a0cb2a9bfb4752b661905a2de6278fed9771dd

Should I be creating a PR against 1.10.x, or master?

[Tim Graham]

Test is added to the PR.

[Loic Bistuer]

Latest ​PR.

[Tim Graham <timograham@…>]

In 2eb7cb2f:

Fixed #26643 -- Prevented unnecessary AlterModelManagers operations caused by the manager inheritance refactor.

This also makes migrations respect the base_manager_name and
default_manager_name model options.

Thanks Anthony King and Matthew Schinckel for the initial patches.

[Tim Graham <timograham@…>]

In 0f23bceb:

[1.10.x] Fixed #26643 -- Prevented unnecessary AlterModelManagers operations caused by the manager inheritance refactor.

This also makes migrations respect the base_manager_name and
default_manager_name model options.

Thanks Anthony King and Matthew Schinckel for the initial patches.

Backport of 2eb7cb2fffcc382979d0731370de26b051d04659 from master