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
signature.asc
Description: OpenPGP digital signature
