Carl:

Actually, the "subclass the operation" method fails for obvious reasons
here:

https://github.com/django/django/blob/df0523debcc2d0984f1bc11d323f04227d4b388b/django/apps/registry.py#L202

I'm increasingly thinking that using MIGRATION_MODULES and just
maintaining a local copy of Mezzanine's migration is the simplest
solution.  I sense that I'm going to face some resistance if I suggest
changing their docs to reflect this.  I'll start with a blog post.


On 11/04/2014 04:30 PM, Justin Myles Holmes wrote:
> Well, as I say, I don't particularly like this field injection notion,
> and I'm happy to individually subclass where needed in my own projects.
>
> I'm less surefooted about issuing a pull request to mezzanine to alter
> their docs to suggest doing this, although the doc is currently
> ambiguous enough to be of little use anyway.
>
> I think that you and Carl have changed my mind about the
> reasonableness of including migrations in one app for a model in
> another.  I'm no longer able to think of a scenario that justifies it.
>
>
> On 11/04/2014 04:12 PM, Andrew Godwin wrote:
>> Carl is right that this isn't a supported thing; Django models are
>> designed to be static, and not mutate based on settings.
>>
>> Migrations are designed to only support one linear change history of
>> models; swappable models are just about supported but not perfect and
>> leads to issues. Adding in the ability to change or remove other
>> fields based on installed models is kind of against the way
>> migrations is designed and you'll have to do a lot of work to get
>> around it.
>>
>> In this particular case, it's possible to make it work, but you'll
>> have to subclass every one of the actions you want to use and provide
>> a way to override the app_label it operates on, and manually
>> construct the migrations to get the dependencies correct and
>> non-circular (as I said, you'll have to do a lot of work to get
>> around it). As long as you have anything resembling magically
>> changing models in your project, you're not going to be able to use
>> anything makemigrations outputs and expect it to Just Work™.
>>
>> Andrew
>>
>> On Tuesday, November 4, 2014 3:59:23 PM UTC-8, Carl Meyer wrote:
>>
>>     Hi Justin,
>>
>>     On 11/04/2014 04:42 PM, Justin Myles Holmes wrote:
>>     >> I don't understand this use case. A ForeignKey impacts only
>>     one database
>>     > table's schema; the table for the model on which the ForeignKey
>>     field is
>>     > located. The schema of the model the ForeignKey points to
>>     doesn't need
>>     > to change at all based on the presence or absence of some other
>>     > ForeignKey pointing to it.
>>     >
>>     > Example:
>>     >
>>     > Two apps: apps.coconuts and apps.swallows, each with a
>>     canonical model
>>     > (Coconut and Swallow, respectively).
>>     >
>>     > The organization runs two version of the project: one with
>>     apps.swallows
>>     > and one without.
>>     >
>>     > The one with apps.swallows needs a field on Coconut, "carried,"
>>     which is
>>     > a FK -=> Swallow.
>>
>>     This is basically the same use case as the Mezzanine one - it's
>>     attempting to make a model more "reusable" by monkeypatching it with
>>     additional fields from outside. I think this should be avoided and
>>     discouraged, not better supported.
>>
>>     The specifics of how to avoid would depend on details of the
>>     case, but I
>>     would look towards providing the reusable portions of the model
>>     as an
>>     abstract base, and maybe additionally using swappable models if
>>     other
>>     models need to have an FK to the "extendable" model.
>>
>>     (Swappable models aren't yet formally supported for use outside
>>     contrib.auth, and there are still rough edges in migration's
>>     support for
>>     them, but I think moving them towards formal support is a much
>>     better
>>     answer to this general use case than monkeypatching fields onto
>>     models.)
>>
>>     Another possibility (which I still don't like, but is better than
>>     monkeypatching) might be to conditionally define the Coconut.carried
>>     field, depending on the value of a setting. In that case, the right
>>     solution for migrations is to still define the migration that
>>     adds the
>>     Coconut.carried field in the coconuts app, but make it
>>     conditionally a
>>     no-op migration based on the value of the same setting.
>>
>>     > To me, the proper way to do this is to ship a migration in the
>>     swallows
>>     > app which adds this field to apps.coconuts.models.Coconut.
>>
>>     Sorry, I don't agree that there's anything "proper" about that
>>     :-) The
>>     migration is a secondary issue here - the basic problem is that
>>     you're
>>     monkeypatching the Coconut model, and I don't think migrations
>>     should
>>     support/encourage one app monkeypatching another app's models.
>>
>>     > This is certainly a rare use case, but not a special case.  On
>>     the other
>>     > hand, I don't see the advantage of restricting the models which
>>     may be
>>     > migrated to those in the app in which the migration appears,
>>     > particularly now that migration dependencies are more
>>     straightforward.
>>
>>     It's a straightforward issue of encapsulation. The database
>>     schema for
>>     the models in app X is the responsibility of app X, not app Y. So
>>     far
>>     the only reasons I've seen why app Y might want to run a
>>     migration on an
>>     app-X model is because app Y is monkeypatching that model, to
>>     which I
>>     think the correct response is "don't do that," not "Django should
>>     better
>>     support that."
>>
>>     >> I also wonder if there might be a way to do (2) without the
>>     > monkeypatching. That is, could Mezzanine provide an importable
>>     Operation
>>     > subclass for this purpose and document how to import and use it
>>     in a
>>     > migration?
>>     >
>>     > Well, the problem there is that Operation isn't used directly, but
>>     > subclassed for each individual type of migration operation.
>>
>>     But for the specific Mezzanine use case, there are really only a few
>>     relevant migration operations, right? Seems like add-column,
>>     alter-column, and drop-column would cover it.
>>
>>     Carl
>>
>> -- 
>> You received this message because you are subscribed to the Google
>> Groups "Django users" group.
>> To unsubscribe from this group and stop receiving emails from it,
>> send an email to django-users+unsubscr...@googlegroups.com
>> <mailto:django-users+unsubscr...@googlegroups.com>.
>> To post to this group, send email to django-users@googlegroups.com
>> <mailto:django-users@googlegroups.com>.
>> Visit this group at http://groups.google.com/group/django-users.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/django-users/da3398bc-d175-432e-86f6-bbefad4ce9f8%40googlegroups.com
>> <https://groups.google.com/d/msgid/django-users/da3398bc-d175-432e-86f6-bbefad4ce9f8%40googlegroups.com?utm_medium=email&utm_source=footer>.
>> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-users+unsubscr...@googlegroups.com.
To post to this group, send email to django-users@googlegroups.com.
Visit this group at http://groups.google.com/group/django-users.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-users/54597531.2040007%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to