Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#27044 closed Bug (fixed)

`apps` passed to post_migrate_signal should contain migrated appconfigs even when no migration has been applied to them

Reported by: tkhyn Owned by: Simon Charette
Component: Migrations Version: 1.10
Severity: Release blocker Keywords:
Cc: Simon Charette Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Tim Graham)

Hi,

I noticed that when one runs migrate without arguments (implicitly migrating all apps), the apps instance passed to emit_post_migrate_signal here) contains the migrated apps' configurations only in the first run.

On subsequent runs, the plan list (here) is empty and post_migrate_apps is set to pre_migrate_apps (here), i.e. the unmigrated apps.

For example, when calling migrate with the contenttypes app enabled, this means that after the first run update_contenttypes (here) is passed an apps instance with contenttypes missing. The consequence is that update_contenttypes can not go further than this line.

To temporarily fix the issue and correctly trigger the post_migrate_signal so that the content types are properly updated each time migrate is called (and not only the first time), I need to run flush just after migrate. I don't think it should not be needed to run flush simply to correctly trigger the post_migrate_signal that will update the content types.

It worked fine in Django 1.9.9 as no project state apps instance is passed to emit_post_migrate_signal, and update_contenttypes uses the global apps instance.

I don't really know enough about the recent developments regarding apps management in Django, so I'm not sure what should be done to solve this issue. Thanks in advance if somebody has the time to look at it.

Context: I'm testing a reusable django app (django-gm2m) which test suite includes migration tests for a number of mini test apps with different models (see here). Whenever I switch to another mini-app, the database needs not only to be flushed but also re-migrated and the contenttypes need to be updated ''

Change History (14)

comment:1 Changed 3 years ago by tkhyn

Description: modified (diff)

comment:2 Changed 3 years ago by tkhyn

Summary: `apps` passed to post_migrate_signal should have access to migrated apps even when no migration has been applied to them`apps` passed to post_migrate_signal should contain migrated appconfigs even when no migration has been applied to them

comment:3 Changed 3 years ago by Tim Graham

Description: modified (diff)

I'm not sure I understand entirely. I think the idea is that if no migrations are applied, then content types shouldn't need to be updated. How can we reproduce the issue to look into it? Run the test suite for your app?

comment:4 Changed 3 years ago by Simon Charette

Cc: Simon Charette added

comment:5 Changed 3 years ago by tkhyn

Thanks for the reply,

You could try and run the test suite for the app (I haven't pushed the changes to fix all the Django 2.0 warnings though so the suite will fail), or more simply use a minimal project in an environment with Django 1.10 installed:

hg clone https://bitbucket.org/tkhyn/dj10_syncdb
cd dj10_syncdb
hg update a7614832e38a7f8f21cf474dc328c295a9d8fc3c

In this state, the sample project has one unmigrated app app with one model MyModel. Lets migrate.

manage.py migrate

The django_content_type table now looks like:

id app_label model
1 admin logentry
2 auth permission
3 auth user
4 auth group
5 contenttypes contenttype
6 sessions session
7 app mymodel

... and everything is ok so far. Let's add a new model to our unmigrated app.

hg update dfd8067076c1ceb7d9a1ae48ddc27c0dfd64044c
manage.py migrate

The django_content_type table has not changed. There should be a new line in django_content_type (app_label = app and model = mysecondmodel), and it's not there because update_contenttypes cannot proceed further than line 102 when migrate is called a second time.

A possible solution would be to transform app into a migrated app (I haven't checked it though), but given that it worked ok in django 1.9.9 I see this as a regression.

Feel free to let me know if you require additional info on this.

Last edited 3 years ago by tkhyn (previous) (diff)

comment:6 Changed 3 years ago by tkhyn

I should have added that using migrate --run-syncdb does not change the outcome (which can be expected when looking at the migrate management command implementation as it has no influence on the apps instance that is passed to the post_migrate signal)

comment:7 Changed 3 years ago by Simon Charette

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Hi tkhyn,

I can't look at this in details in the next few days but update_contenttypes should definitively be running for applications without migrations.

The fact applications with no migrations are present in apps on the first migrate run but not on subsequent ones look like a bug to me.

comment:8 Changed 3 years ago by tkhyn

Thanks @charettes.

The fact applications with no migrations are present in apps on the first migrate run but not on subsequent ones look like a bug to me.

Note that on subsequent runs it's not app's config that is missing in the pre_migrate_apps' app configs, but actually contenttypes'config.

Last edited 3 years ago by tkhyn (previous) (diff)

comment:9 Changed 3 years ago by Simon Charette

Thanks for the follow up @tkhyn,

In this case I suspect the bug lies MigrationExecutor, it might be related to #26647.

comment:10 Changed 3 years ago by Simon Charette

Owner: changed from nobody to Simon Charette
Status: newassigned

comment:11 Changed 3 years ago by Simon Charette

Has patch: set

comment:12 Changed 3 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:13 Changed 3 years ago by Simon Charette <charette.s@…>

Resolution: fixed
Status: assignedclosed

In d1757d8d:

Fixed #27044 -- Included already applied migration changes in the post-migrate state when the execution plan is empty.

Refs #24100.

Thanks tkhyn for the report and Tim for the review.

comment:14 Changed 3 years ago by Simon Charette <charette.s@…>

In 1c60765d:

[1.10.x] Fixed #27044 -- Included already applied migration changes in the post-migrate state when the execution plan is empty.

Refs #24100.

Thanks tkhyn for the report and Tim for the review.

Backport of d1757d8df486b689172d2584ded52fad916bcc33 from master

Note: See TracTickets for help on using tickets.
Back to Top