As I've said before, I like this idea, and the opt-in thing is even better and gets rid of a lot of the compatibility headaches I was worried about.
The review looks good overall - have left a couple of notes, only really the one change I want to see (the operational dependency stuff). Andrew On Fri, Dec 5, 2014 at 10:28 AM, Carl Meyer <[email protected]> wrote: > Hi Markus, > > On 12/05/2014 11:22 AM, Markus Holtermann wrote: > > Ticket: https://code.djangoproject.com/ticket/23822 > > Pull Request: https://github.com/django/django/pull/3687 > > > > Hey folks, > > > > As of now migrations don't expose the model managers to models. Thus it > is > > "impossible" to use e.g. `create_superuser` in a `RunPython` operation. I > > opened ticket #23822 to keep track of that issue. I propose a solution as > > implemented in pull request 3687: managers get an optional class > attribute > > `use_in_migrations` that, if True, will mark that manager to be added to > > migrations by the migration framework. Managers that are part of > migrations > > need to stay around for as long as a migration references the managers. > > This is the same as with custom model fields or functions passed to field > > attributes (e.g. `upload_to`). > > > > To keep track of the ordering of the managers (and know which one is > really > > the default), I merged the `Model._meta.abstract_managers` and > > `.concrete_managers` into a single list and added properties to return > the > > respective subsets instead. > > > > The `managers` attribute on `CreateModel` will be empty without enabling > at > > lease one manager. The normal manager (`models.Manager`) will (almost) > > never be serialized. Except for the situation where a model's default > > manager is a manager without migration support, but a custom manager is. > In > > that case a default manager `objects = models.Manager()` will be added > > explicitly to prevent the non-default manager to become a default > manager. > > > > I added migration support for all contrib managers (where appropriate), > e.g > > `UserManager` instead of `BaseUserManager`. > > > > I'd like to get a thorough review before merging, especially regarding > the > > `ModelState.render()` and `from_model()` functions. I think there are > some > > improvements possible I haven't though about. > > I think the feature makes sense, as an opt-in. > > I've added it to my review list, I'll try to get to it soon. > > Thanks! > > Carl > > -- > You received this message because you are subscribed to the Google Groups > "Django developers (Contributions to Django itself)" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > Visit this group at http://groups.google.com/group/django-developers. > To view this discussion on the web visit > https://groups.google.com/d/msgid/django-developers/5481F965.3010600%40oddbird.net > . > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAFwN1urbKFNTYtT8rJSxEFE9qAjQ-1DD3dLqdTOtr2JYCz3eVw%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
