Opened 10 years ago

Last modified 3 weeks ago

#24686 assigned New feature

Support for Moving a model between two Django apps

Reported by: Alex Rothberg Owned by: Bhuvnesh
Component: Migrations Version: dev
Severity: Normal Keywords:
Cc: jedie, Simon Charette, Shai Berger, David Wobrock Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

An issue that I run into is that at some point one of my apps becomes so large that I decide to split the application into a number of separate logical units. Right now the Django migration system leaves me on my own to write a correct migration to support updating the db to reflect the new multi-app layout.

There is an SO post that attempts to outline a solution but I would suggest trying to detect these model moves and then auto-generating these migrations.

Change History (42)

comment:1 by Tim Graham, 10 years ago

Component: UncategorizedMigrations
Needs documentation: set
Triage Stage: UnreviewedAccepted
Version: 1.8master

Not sure about feasibility of detecting/auto-generating, but if not, we could certainly document the SeparateDatabaseAndState solution in docs/howto/writing-migrations.txt.

comment:2 by Alex Rothberg, 10 years ago

I'm not sure how the plumbing works for migrations, but I would think the auto-detect logic would work like renaming of fields where if a model with the same name is deleted in one app and created in another the user is prompted with "did you move this model...".

comment:3 by Marten Kenbeek, 10 years ago

An actual implementation would supersede #24016.

I don't think the operation to move a model to another app has to be significantly more complex that the current RenameModel. The autodetection, on first sight, is just a matter of removing this check: https://github.com/django/django/blob/master/django/db/migrations/autodetector.py#L421

I do have some concerns about dependencies. If the migration lives in the old app, the autodetector won't be able to detect that the next migration in the new app depends on the RenameModel migration in the old app. Likewise, if the migration lives in the new app, the next migration in the old app won't have a dependency on the migration that moves the model. The last case might not be a problem, but I can't say that off the top of my head.

comment:4 by Alex Rothberg, 10 years ago

Crazy question, but what about project level migrations?

in reply to:  4 comment:5 by Markus Holtermann, 10 years ago

Referring back to the example solution from SO:

As soon as you remove the old_app from installed apps and apply the migrations on an empty database the SeparateDatabaseAndState operation in new_app will only create the model in memory but no database table. Therefore it's not an option to migrate data between apps to get rid of one app it this simple form.

Replying to cancan101:

Crazy question, but what about project level migrations?

What about them? You can define settings.MIGRATION_MODULES and put all app migrations in your project. But I advice you to not do that unless there is a 3rd party app that doesn't have Django migration support. Then this is the way to go to add migrations for that particular app.

Replying to knbk:

I do have some concerns about dependencies. If the migration lives in the old app, the autodetector won't be able to detect that the next migration in the new app depends on the RenameModel migration in the old app. Likewise, if the migration lives in the new app, the next migration in the old app won't have a dependency on the migration that moves the model. The last case might not be a problem, but I can't say that off the top of my head.

Sounds about right to me. You will eventually run into a hellhole of dependencies.

comment:6 by Alex Rothberg, 10 years ago

@MarkusH Are you saying that the solution posted on SO won't work?
I was suggesting the project level migrations to address the issues raised by @knbk so that the migrator could see both apps.

comment:7 by Markus Holtermann, 10 years ago

@cancan101, I'm saying that you will have trouble to get the suggested migrations work on an empty database once old_app is removed from the list of installed apps. To be specific, since you already need to fix the migration dependencies it's probably not that big of a deal to also drop the SeparateDatabaseAndState and replace it with a CreateModel operation.

comment:8 by Florian Klink, 8 years ago

I also ran into this today, when pulling some models into a separate app.

The stackoverflow post does not work anymore as soon as there are cross-app foreign keys, but this example here does.
In the solution outlined there, a migration inside the new app depends on a migration of the old app (see here), so loading all migrations should at least break as soon if you remove the old app from INSTALLED_APPS ;-)

Given the currently needed amount of manual work, we should really try to detect these movements and produce migrations like above.

comment:9 by Petr Dlouhý, 8 years ago

Maybe this could be separated into two separate tasks.
We could first create database operation like django.db.models.MoveToApp. This alone could save a lot of manual work and confusion for developers.

Second stage would be creating the autodetection mechanism, which would make migrations with this operation automatically.

comment:10 by Alexandru Mărășteanu, 7 years ago

In the meantime, I've built a management command to do just that. You can find it at alexei/django-move-model

If someone cared to take a look and submit some feedback, I'd really appreciate it.

comment:11 by jedie, 5 years ago

Cc: jedie added

comment:12 by wtayyeb, 5 years ago

Hi, I have add a method to MigrationAutodetector to let it find moved models, originally as a fork of .generate_renamed_models(). It will create at least 2 migrations with SeparateDatabaseAndState to do the job.
i have create a PR for it. I try not to alter anywhere else in MigrationAutodetector, maybe others could do it better without this constrain.

I have only one problem, I need to mention the migration of rem_app_label ( commented in PR ) just before new created one ( which I create ).
without it forward plan may works, but backward plan may be incorrect leading to broken backward migration or even data loss.
Adding it manually is not an acceptable answer for Django project.

I dig in code but couldn't find it. anyone can help me?

comment:13 by wtayyeb, 5 years ago

Hi again.
Unlike my previous comment and approach, I found that RenameModel operation is well written even for moving models, so I update it and MigrationAutodetector to handle moving models.
The move mechanism is completely without SeparateDatabaseAndState and it will produce at least 2 migrations one for old_app and one for new_app like below.

00zz_new_created_migration_at_old_app.py

class Migration(migrations.Migration):
    dependencies = [
        ('new_app', '00xx_some_migration'),
        ('old_app', '00yy_pre_migration'),
        # ...
    ]
    operations = [
        operations.RenameModel(
            old_name='MyModel',
            new_name='MyModel',
            new_app_label='new_app',
        ),
    ]

00vv_new_created_migration_at_new_app.py

class Migration(migrations.Migration):
    initial = True
    dependencies = [
        ('new_app', '00xx_some_migration'),
        ('old_app', '00zz_new_created_migration_at_old_app'),
    ]
    operations = [
        operations.MoveModelPlaceholder(
            old_name='MyModel',
            new_name='MyModel',
            new_app_label='new_app',
            comment='this is a required noop operation to construct dependencies for moving model',
        ),
    ]

note that MoveModelPlaceholder is a noop operation to construct the dependencies and let forward and backward migrations without problem.

the code is in PR

comment:14 by wtayyeb, 5 years ago

Has patch: set
Last edited 5 years ago by wtayyeb (previous) (diff)

comment:15 by Abhishek Bera, 4 years ago

Owner: changed from nobody to Abhishek Bera
Status: newassigned

comment:16 by Abhishek Bera, 4 years ago

Hi, I am new to django development so I started with this documentation ticket. I am struggling with the level of detail I should mention in the document. So far I just updated the param and mentioned what it does. Here is the diff :

ddiff --git a/docs/ref/migration-operations.txt b/docs/ref/migration-operations.txt
index d1620cce8e..f4a444e8e9 100644
--- a/docs/ref/migration-operations.txt
+++ b/docs/ref/migration-operations.txt
 ``RenameField``
 ---------------
 
-.. class:: RenameField(model_name, old_name, new_name)
+.. class:: RenameField(model_name, old_name, new_name, new_app_label=None)
 
 Changes a field's name (and, unless :attr:`~django.db.models.Field.db_column`
-is set, its column name).
+is set, its column name) or moves the Model to `new_app_label`.

Also, what should be the branch name for this PR?

Last edited 22 months ago by Tim Graham (previous) (diff)

comment:18 by Abhishek Bera, 4 years ago

Owner: Abhishek Bera removed
Status: assignednew

comment:19 by Jacob Walls, 4 years ago

Needs tests: set

comment:20 by Durval Carvalho, 22 months ago

Owner: set to Durval Carvalho
Status: newassigned

comment:21 by Durval Carvalho, 22 months ago

Hi there! I've started working on this ticket and I've opened a PR with my progress so far at https://github.com/django/django/pull/16523. I'm having a little difficulty understanding the autodetector.py code but I believe it's just a matter of spending more time with it. I discovered this ticket through the gsoc2023 blog post for Django and I'm planning to create a proposed solution once I figure out how to implement it. I'm excited to be able to collaborate with my favorite framework!

comment:22 by Durval Carvalho, 22 months ago

I would like to ask for feedback on my approach. I have found what I believe to be the best way to move a model from one app to another in Django. It involves four separate operations in at least 3 different migrations:

1. Rename table operation: renames the physical table name on the database. This operation should be a SeparateDatabaseAndState operation and only change the database, without changing any state.

2. Create table operation: moves the model code to the new app, and creates a CreateModel only in state, with a SeparateDatabaseAndState operation.

3. Update all foreign keys that point to the moved model.

4. Delete the old model table, but only in state, using a SeparateDatabaseAndState operation.

I've implemented this logic in a new method called "generate_moved_models" in MigrationAutodetector. I've discovered that the current implementation only handles simple ForeignKeys and not more complex cases like ManyToManyFields. This is because the current approach generates at least one AlterField with the fields ManyToOneRel and ManyToManyField, which cannot be serialized at the time of writing operations to the migration files.

I'm still trying to figure out the best approach to tackle this issue. I'm unsure whether extending the serializer_factory to support these types of fields or creating a new operation (probably "MoveModel") similar to RenameModel would be the better solution. I noticed that in the RenameModel the operations related to column renaming in the M2M tables are done in the database_forwards method using the _alter_many_to_many.

If anyone in the Django community has any suggestions or experience with this issue, I would greatly appreciate any input. Thank you!

comment:23 by Durval Carvalho, 21 months ago

Hi everyone, I'm working on this ticket and I want to make sure that the new functionality will meet all the necessary scenarios. So far, I've identified some scenarios that the feature should cover, such as moving a single model, moving multiple models that are independent, and moving models from an app that no longer exists. Are there any other scenarios that I should consider? Your feedback would be greatly appreciated. Thank you!

comment:24 by Durval Carvalho, 21 months ago

Recently, I’ve been thinking about how reverse migrations might work with moved model operations.

To illustrate, let’s take an example of a scenario where the following operations were performed:

1. The database operation AlterModelTable, renaming the table for the moved model
    1.1. (suppose it is ‘core_category’ → ‘categories_category’)
2. The state operation CreateModel
3. The AlterField of the foreign keys pointing to the moved field
    3.1. Supose it is core.category → categories.category
4. The state operation DeleteModel

By default, the reverse of these operations will be 4 → 3 → 2 ->1. However, it’s not possible to execute operation number 3 because the table hasn’t been renamed yet. The reverse operation will try to update the foreign key field back to core.category and this will be translated to the core_category table, however this table does not exist in the database at this point.

As far as I understand, the reverse operation needs to follow the same sequence as the initial operation (1->2->3->4), but with the fields reversed, ‘categories_category’ → ‘core_category’ and categories.category → core.category.

To resolve this and return to the previous state, one option is to move the model back to its original app and then run the “makemigrations” command again. However, keep in mind that this will generate new migrations instead of reverting the ones that have already been executed.

Does this approach seem reasonable to you?

comment:25 by Jacob Walls, 21 months ago

Needs documentation: unset
Needs tests: unset

comment:26 by Mariusz Felisiak, 21 months ago

There is also an alternative PR.

comment:27 by Mariusz Felisiak, 21 months ago

Personally, I'd rather add a new migration operation than abuse SeparateDatabaseAndState().

comment:28 by Mariusz Felisiak, 21 months ago

Needs documentation: set

comment:29 by Bhuvnesh, 19 months ago

Owner: changed from Durval Carvalho to Bhuvnesh

comment:30 by Simon Charette, 18 months ago

If that can be of any help this problem and a potential solution that doesn't involve using SeparateDatabaseAndState was discussed in a related ticket https://code.djangoproject.com/ticket/34673#comment:2

comment:31 by Mariusz Felisiak <felisiak.mariusz@…>, 17 months ago

In f05cc5e3:

Refs #24686 -- Made AlterField operation a noop when renaming related model with db_table.

comment:32 by Mariusz Felisiak, 13 months ago

Cc: Simon Charette Shai Berger added
Needs documentation: unset

comment:33 by Maarten Nieber, 12 months ago

I've been struggling with this issue for a while, and came upon an extremely simple (though not perfect) solution. Things seem to keep working fine if I move the model code to a new django app, and set the "app_label" field in the Meta class to point to the old django app.

I have two questions:

  • is this totally fine to do, and expected to work?
  • should we explain in the Docs that this is a simple and effective way to move a model?

comment:34 by Bhuvnesh, 12 months ago

Hi Maarten!
I don't think moving the model code to the new django app and setting the app_label to point to the old django app will work. The auto detector will see it as the deletion of model in the old app and creation of that model in the new app. This will also lead to losing all the data associated with that model.
There is a PR open for addressing this issue which was working pretty good when the last time I tested it. I would suggest switching to that branch and try the same you suggested above (if possible) and it should work fine without any data loss. You can also have a look at the working of this new feature here. Let me know if you face any problem.

in reply to:  33 comment:35 by Shai Berger, 11 months ago

Replying to Maarten Nieber:

I've been struggling with this issue for a while, and came upon an extremely simple (though not perfect) solution. Things seem to keep working fine if I move the model code to a new django app, and set the "app_label" field in the Meta class to point to the old django app.

I have two questions:

  • is this totally fine to do, and expected to work?

I disagree with Bhuvnesh above: I expect this to work, in the sense that your project should continue to run, and no migration will be necessary.

But it's "cheating" -- the model's code has been moved to a different file, but the model is still in the old app for all other purposes: If you make a change in it, the migration will still be created in the old app. If you try to remove the old app, things will break. Anywhere you'd want to reference it by name (e.g. in a Foreign Key definition), you'll need to refer to it with the label specified in its meta, not the label of the app in whose models.py it is defined.

  • should we explain in the Docs that this is a simple and effective way to move a model?

... and the above is why I think we shouldn't. I expect doing this -- creating a discrepancy between the model's import path and the way Django treats it -- is likely to cause problems and confusion, negating the value of the refactoring that is achieved by the move.

The ability to change the app-label in the Meta is mostly useful in cases where you want a model defined outside of its app's models.py ; there are sometimes reasons to do this (e.g. a model that is defined for tests only).

comment:36 by Bhuvnesh, 11 months ago

Sorry! I completely misunderstood what maarten was trying to say. Thanks for the clarification Shai.

The auto detector will see it as the deletion of model in the old app and creation of that model in the new app. This will also lead to losing all the data associated with that model.

This will happen if you change just the app_label of model to the app_label of app you want to move the model and not actually move the model code. For example, you have SampleModel in app_a and you want to move it to app_b, so you just added app_label=app_b in SampleModel's Meta and not actually moved the model code to app_b.

Last edited 11 months ago by Bhuvnesh (previous) (diff)

comment:37 by David Wobrock, 11 months ago

Cc: David Wobrock added

comment:38 by Sarah Boyce, 8 months ago

Patch needs improvement: set

Marking as patch needs improvement as I failed to follow the instructions in the docs successfully, so I don't think they're quite clear enough.
I plan to do a series of tests once I understand what I should be doing 😁 exciting feature!

comment:39 by Bhuvnesh, 3 months ago

Patch needs improvement: unset

comment:40 by Sarah Boyce, 3 months ago

Patch needs improvement: set

comment:41 by Bhuvnesh, 3 weeks ago

Patch needs improvement: unset

comment:42 by Sarah Boyce, 3 weeks ago

Patch needs improvement: set
Note: See TracTickets for help on using tickets.
Back to Top