Re: A migration in one app that adds a field to another
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 >> conditiona
Re: A migration in one app that adds a field to another
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 &g
Re: A migration in one app that adds a field to another
> 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. 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. 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. > 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. On 11/04/2014 03:33 PM, Carl Meyer wrote: > Hi Justin, > > On 11/04/2014 04:17 PM, Justin Myles Holmes wrote: >> Hey Carl. Missed you so very much at DjangoCon. :-) > Thanks :-) > >>> I'm not sure why the mezzanine docs don't mention using the >> SOUTH_MIGRATION_MODULES setting (or Django 1.7 equivalent >> MIGRATION_MODULES) to override the location of the mezzanine app's >> migrations to a package in your project, where you can freely add custom >> migrations without modifying mezzanine code. >> >> While I don't want to speak for Mezzanine (and, frankly, am not crazy >> about their approach to field injection in the first place), I surmise >> that they want to be able to both ship their own migrations (when their >> models change) and allow their users to add their own migrations (for >> injecting fields). >> >> As I say, I don't love the solution, but I do see its advantages over >> merely providing a mixin, as they have a relationship structure of their >> own between their models that they want to maintain. >> >> You say that it's a reasonable tradeoff; I don't think I agree. For >> Django's part, I'm not convinced that, without exception, migrations for >> a particular model belong in the same app as that model. I think there >> are reasonable times to place them elsewhere, and this is one of them. >> Another is an occasion in which an organization has its own internal >> apps and runs versions of its project without and without a particular >> app. If that app requires an incoming ForeignKey from a model in a >> different app, that migration is ideally only implied in the presence of >> the target app. > 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. > > It still seems to me that there isn't a good reason for Django to > support migrations that modify the schema of a model in some other app. > That's the migrations equivalent of monkeypatching, and I think it will > usually end in tears :-) > >>> I don't see any monkeypatch there, just subclassing, which is not the same >>> thing. I'd say that >> looks like a very nice solution to the problem. The level of "fiddling >> with internals" required seems roughly proportional to the peculiarity >> of the use case; I doubt you'll find anything better (other than perhaps >> avoiding the problem entirely by using MIGRATION_MODULES). >> >> I was unclear: What I meant was that a potential solution within the >> Mezzanine codebase is a monkeypatch which changes the state_forwards >> method at runtime to comport with the subclass example that I've provided. > Ah, I see. I didn't realize that you were looking for a generic solution > in Mezzanine, rather than a solution for your specific Mezzanine-using > project. > >> My question (which at this point probably belongs more on dev than here) >> is: Which is the most Djangoic approach: >> >> 1) Modify the Mezzanine docs to encourage users to take their own >> action, such as A) using MIGRATION_MODULES or B) manually editing >> migrations and subclassing
Re: A migration in one app that adds a field to another
> 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. 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. 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. > 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? On 11/04/2014 03:33 PM, Carl Meyer wrote: > Hi Justin, > > On 11/04/2014 04:17 PM, Justin Myles Holmes wrote: >> Hey Carl. Missed you so very much at DjangoCon. :-) > Thanks :-) > >>> I'm not sure why the mezzanine docs don't mention using the >> SOUTH_MIGRATION_MODULES setting (or Django 1.7 equivalent >> MIGRATION_MODULES) to override the location of the mezzanine app's >> migrations to a package in your project, where you can freely add custom >> migrations without modifying mezzanine code. >> >> While I don't want to speak for Mezzanine (and, frankly, am not crazy >> about their approach to field injection in the first place), I surmise >> that they want to be able to both ship their own migrations (when their >> models change) and allow their users to add their own migrations (for >> injecting fields). >> >> As I say, I don't love the solution, but I do see its advantages over >> merely providing a mixin, as they have a relationship structure of their >> own between their models that they want to maintain. >> >> You say that it's a reasonable tradeoff; I don't think I agree. For >> Django's part, I'm not convinced that, without exception, migrations for >> a particular model belong in the same app as that model. I think there >> are reasonable times to place them elsewhere, and this is one of them. >> Another is an occasion in which an organization has its own internal >> apps and runs versions of its project without and without a particular >> app. If that app requires an incoming ForeignKey from a model in a >> different app, that migration is ideally only implied in the presence of >> the target app. > 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. > > It still seems to me that there isn't a good reason for Django to > support migrations that modify the schema of a model in some other app. > That's the migrations equivalent of monkeypatching, and I think it will > usually end in tears :-) > >>> I don't see any monkeypatch there, just subclassing, which is not the same >>> thing. I'd say that >> looks like a very nice solution to the problem. The level of "fiddling >> with internals" required seems roughly proportional to the peculiarity >> of the use case; I doubt you'll find anything better (other than perhaps >> avoiding the problem entirely by using MIGRATION_MODULES). >> >> I was unclear: What I meant was that a potential solution within the >> Mezzanine codebase is a monkeypatch which changes the state_forwards >> method at runtime to comport with the subclass example that I've provided. > Ah, I see. I didn't realize that you were looking for a generic solution > in Mezzanine, rather than a solution for your specific Mezzanine-using > project. > >> My question (which at this point probably belongs more on dev than here) >> is: Which is the most Djangoic approach: >> >> 1) Modify the Mezzanine docs to encourage users to take their own >> action, such as A) using MIGRATION_MODULES or B) manually editing >> migrations and subclassing the operation in question as I've done >> 2) Adding a monkeypatch to Mezzanine which automates solution 1B above >> (ie,
Re: A migration in one app that adds a field to another
Hey Carl. Missed you so very much at DjangoCon. :-) > I'm not sure why the mezzanine docs don't mention using the SOUTH_MIGRATION_MODULES setting (or Django 1.7 equivalent MIGRATION_MODULES) to override the location of the mezzanine app's migrations to a package in your project, where you can freely add custom migrations without modifying mezzanine code. While I don't want to speak for Mezzanine (and, frankly, am not crazy about their approach to field injection in the first place), I surmise that they want to be able to both ship their own migrations (when their models change) and allow their users to add their own migrations (for injecting fields). As I say, I don't love the solution, but I do see its advantages over merely providing a mixin, as they have a relationship structure of their own between their models that they want to maintain. You say that it's a reasonable tradeoff; I don't think I agree. For Django's part, I'm not convinced that, without exception, migrations for a particular model belong in the same app as that model. I think there are reasonable times to place them elsewhere, and this is one of them. Another is an occasion in which an organization has its own internal apps and runs versions of its project without and without a particular app. If that app requires an incoming ForeignKey from a model in a different app, that migration is ideally only implied in the presence of the target app. > I don't see any monkeypatch there, just subclassing, which is not the same > thing. I'd say that looks like a very nice solution to the problem. The level of "fiddling with internals" required seems roughly proportional to the peculiarity of the use case; I doubt you'll find anything better (other than perhaps avoiding the problem entirely by using MIGRATION_MODULES). I was unclear: What I meant was that a potential solution within the Mezzanine codebase is a monkeypatch which changes the state_forwards method at runtime to comport with the subclass example that I've provided. My question (which at this point probably belongs more on dev than here) is: Which is the most Djangoic approach: 1) Modify the Mezzanine docs to encourage users to take their own action, such as A) using MIGRATION_MODULES or B) manually editing migrations and subclassing the operation in question as I've done 2) Adding a monkeypatch to Mezzanine which automates solution 1B above (ie, a function that takes an Operation object and returns a OperationOnAnotherApp object) 3) Providing a solution in Django for migrating a model from an app other than that model's home On 11/01/2014 09:16 PM, Carl Meyer wrote: > Hi jMyles, > > On 11/01/2014 04:35 PM, jMyles Holmes wrote: >> Using Django 1.7, how can the user perform a migration operation (say, >> AddField) on an app other than the app in which the migration lives? >> >> Here's the use case: >> >> Mezzanine lets you inject fields on its own stock models using the >> EXTRA_MODEL_FIELDS setting. >> >> However, as the Mezzanine docs point out >> (http://mezzanine.jupo.org/docs/model-customization.html#field-injection-caveats), >> there are some "caveats" regarding migrations. >> >> Specifically, the migrations need to be placed in a local app even >> though they're applied to the Mezzanine models. >> >> What's the proper way to do this in Django 1.7? > > Perhaps someone with more experience with the new migrations system can > correct me, but I doubt there is a "proper way" - this is an unusual and > probably unsupported use case that I would guess was never envisioned in > the design. In general, changes to a model belong in the migrations for > that model's app. > > I'm not sure why the mezzanine docs don't mention using the > SOUTH_MIGRATION_MODULES setting (or Django 1.7 equivalent > MIGRATION_MODULES) to override the location of the mezzanine app's > migrations to a package in your project, where you can freely add custom > migrations without modifying mezzanine code. > > The downside of this approach is that if a new mezzanine version in the > future ships migrations for changes to their model, you'll have to copy > or regenerate those migrations yourself in your custom migrations > directory. That seems like a reasonable tradeoff in the situation where > you're effectively monkeypatching another app's models. > >> It's easy to do it with a monkeypatch. Here's an example: >> >> https://gist.github.com/jMyles/130050563ba96a7d7cc8 > > I don't see any monkeypatch there, just subclassing, which is not the > same thing. I'd say that looks like a very nice solution to the problem. > The level of "fiddling with internals" required seems roughly > proportional to the peculiarity of the use case; I doubt you'll find > anything better (other than perhaps avoiding the problem entirely by > using MIGRATION_MODULES). > > Carl > -- You received this message because you are subscribed to the Google Groups "Django users" group. To unsubscribe from this group and stop
