Serialize model managers in migrations

[Markus Holtermann]

Right now it's impossible to use e.g. the UserManager in a RunPython operation, thus User.objects.create_superuser() isn't available. We should serialize the model managers into migrations too. I suggest to do it the same way as with callable argument values on fields like upload_to: the manager need to stay around as long as there is a migration referencing it.

[Tim Graham]

My understanding is that this has been discussed offline with Andrew and accepted.

[Shai Berger]

I don't understand how this is supposed to work. As a trivial example -- suppose your (custom) user model changes over time, and grows a new mandatory field. This requires you to change your UserManager.create_superuser() accordingly. For the migration to still work, you need to serialize the actual manager code -- but you clearly do not intend this to be the case, and rightly so.

No, I think that if you need to call some code in a migration, then this code needs to be made available to the migration, but requiring the manager to be compatible with all past migrations still around makes very little sense to me.

[Markus Holtermann]

Hi Shai,

the managers will be serialized in the same way as e.g. functions for field arguments are serialized: imported and referenced. A migration for a custom user model will thus look like

# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import models, migrations
import myapp.models


class Migration(migrations.Migration):

    operations = [
        migrations.CreateModel(
            name='MyUser',
            fields=[
                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
                ('password', models.CharField(max_length=128, verbose_name='password')),
                ('email', models.EmailField(max_length=255, verbose_name='email address', blank=True, unique=True)),
            ],
            options={
                'verbose_name': 'user',
                'verbose_name_plural': 'users',
            },
            managers=[
                ('objects', myapp.models.MyUserManager()),
            ],
        ),
    ]

The managers argument keeps track of all managers of a model unless only the default manager (objects = models.Manager()) is available. The argument will be empty / not given in those situations.

A current work-in-progress implementation is available in my ​migration-managers branch.

[Carl Meyer]

I had the same concern as Shai with this ticket. Anything that is imported in migration files adopts some new, rather stringent, backwards-compatibility requirements. It can never move, go away, or change in backwards-incompatible ways, as long as the migration(s) referencing it are still around. And getting rid of old migrations is not easy, especially for reusable third-party projects (see the lengthy discussion over on #23861).

But I can also see some cases where a simple custom manager is fundamental to a model's behavior, and less likely to change. (For one example, I think of custom managers I've implemented in the past for soft-delete). I can certainly see the attractiveness of automatically having this manager in migrations as well.

One option would be to make this behavior available, but opt-in via something like a use_in_migrations = True attribute on the manager, roughly parallel to the existing use_for_related_fields. That way we wouldn't be forcing anyone into these new back-compat restrictions on custom managers, but for those who understand the tradeoff (the downsides should be well documented) and want to make it, they have that choice.

I like this idea, although I still wonder whether even having that option is a bad temptation to have lying around -- something that'll be awfully tempting in the moment, but you may regret it in the morning :-)

[Shai Berger]

Just FTR, the use_in_migration flag indeed removes my objections.

[Simon Charette]

Hi Markus,

Since we discussed this topic a bit in Amsterdam I also wanted to chime in order to let you know the use_in_migration flag also removes the objections I raised.

[Markus Holtermann]

I finally had some time to write some docs and tests. Here's the pull request: ​https://github.com/django/django/pull/3687

[Tim Graham <timograham@…>]

In e37ab311fc52ff5c9ea0951ce3a8d8b38f285900:

Changed internal storing of abstract and concrete managers to be in a single list.

This commit prepares the internal manager layout to be serialized by
migrations; refs #23822.

[Tim Graham <timograham@…>]

In aa5ef0d4fc67a95ac2a5103810d0c87d8c547bac:

Fixed #23822 -- Added support for serializing model managers in migration

Thanks to Shai Berger, Loïc Bistuer, Simon Charette, Andrew Godwin,
Tim Graham, Carl Meyer, and others for their review and input.

[Tim Graham <timograham@…>]

In 3ef50a772b6419c0c41635f80995df25fbc5116e:

Fixed reverse test execution for migration manager tests

Thanks to Tim Graham for reporting the issue; refs #23822

[Tim Graham <timograham@…>]

In 1f03d2d924ba90e83dc9d8c93438bf5ee2f17ccd:

Refs #23822 -- Made MigrationOptimizer aware of model managers

[Simon Charette <charette.s@…>]

In 3938b3cc:

Fixed #26286 -- Prevented content type managers from sharing their cache.

This should prevent managers methods from returning content type instances
registered to foreign apps now that these managers are also attached to models
created during migration phases.

Thanks Tim for the review.

Refs #23822.

[Simon Charette <charette.s@…>]

In ba6f83ec:

[1.9.x] Fixed #26286 -- Prevented content type managers from sharing their cache.

This should prevent managers methods from returning content type instances
registered to foreign apps now that these managers are also attached to models
created during migration phases.

Thanks Tim for the review.

Refs #23822.

Backport of 3938b3ccaa85f1c366909a4839696007726a09da from master

[Simon Charette <charette.s@…>]

In 4701c81d:

[1.8.x] Fixed #26286 -- Prevented content type managers from sharing their cache.

This should prevent managers methods from returning content type instances
registered to foreign apps now that these managers are also attached to models
created during migration phases.

Thanks Tim for the review.

Refs #23822.

Backport of 3938b3ccaa85f1c366909a4839696007726a09da from master