[Django] #23699: Migrate in unittest still loads initial data
Django
--------------------------------------------+--------------------
Reporter: tony-zhu | Owner: nobody
Type: Bug | Status: new
Component: Core (Management commands) | Version: 1.7
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------------+--------------------
The migrate command in Django 1.7 does not load the initial_data fixtures
as claims in the release note.
However it still loads the initial_data in unittests, which crashes the
unittest when using some libraries (Django-helpdesk in my case).
Here is what I found:
When initializing unit tests, the
create_test_db in db/backends/creation calls the migration command with
test_flush=True
The migrate command checks the test_flush in line 135. Then calls the
flush command which loads the initial data.
Adding "load_initial_data=False" to the migrate command fixed it in my
local test environment
Here is the section changed:
{{{
# The test runner requires us to flush after a syncdb but before
migrations,
# so do that here.
if options.get("test_flush", False):
call_command(
'flush',
verbosity=max(self.verbosity - 1, 0),
interactive=False,
database=db,
reset_sequences=False,
inhibit_post_migrate=True,
load_initial_data=False,
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23699>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: tony-zhu | Owner: nobody
Type: Bug | Status: new
commands) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
This looks like a duplicate of #23077, in which case it looks like it
can't be fixed. The suggestion is simply to remove the initial data
directory when you add migrations to your application. Will this work for
you? Perhaps more documentation is needed.
--
Ticket URL: <https://code.djangoproject.com/ticket/23699#comment:1>
Django
Reporter: tony-zhu | Owner: nobody
Type: Bug | Status: new
commands) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by tony-zhu):
It is a duplicate ticket.
But the 1.7 release does break the apps which have initial_data and want
to be compatible with both Django 1.6+South and Django 1.7.
My pull request https://github.com/tony-
zhu/django/commit/f533a949af2da4d2e8a59edade03772c7b74d0a4 at least makes
the "flush" command do what it claims doing, that is only load
initial_data for unmigrated apps.
My local test shows it will keep the behavior of unmigrated app as the old
syncdb and ignore the initial_data for migrated apps. Which I think might
be a better solution for the users to adapt the new migration framework.
--
Ticket URL: <https://code.djangoproject.com/ticket/23699#comment:2>
Django
Reporter: tony-zhu | Owner: nobody
Type: Bug | Status: new
commands) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* cc: andrewgodwin (added)
Comment:
Yes, I don't understand Andrew's rationale for doing nothing on the other
ticket, "there is no way to prevent people mentioning models that are
migrated in fixtures in unmigrated apps (Django doesn't segregate models
during fixture loading)". Why do we need to prevent that? Adding him just
to be sure.
--
Ticket URL: <https://code.djangoproject.com/ticket/23699#comment:3>
Django
Reporter: tony-zhu | Owner: nobody
Type: Bug | Status: new
commands) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by andrewgodwin):
The problem I was encountering was that people would have one initial_data
migration for a project containing entries for all models in a project -
not just the ones in the specific app (as it's common to just "dumpdata"
to a single fixture file)
In particular, while you can make flush only load the initial_data from
apps without migrations, you cannot prevent people from putting instances
of models with migrations into those initial_data files.
Thus, while you can modify flush to only load unmigrated apps, it's of
limited use in actually preventing the error occurring in the initial
ticket. That said, I don't see why it can't be fixed, and then documented
as such, it might be a little less surprising.
--
Ticket URL: <https://code.djangoproject.com/ticket/23699#comment:4>
Django
Reporter: tony-zhu | Owner: nobody
Type: Bug | Status: new
commands) | Resolution:
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timgraham):
* has_patch: 0 => 1
* needs_tests: 0 => 1
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/23699#comment:5>
Django
Reporter: tony-zhu | Owner: nobody
Type: Bug | Status: new
commands) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by tony-zhu):
Replying to [comment:4 andrewgodwin]:
> The problem I was encountering was that people would have one
initial_data migration for a project containing entries for all models in
a project - not just the ones in the specific app (as it's common to just
"dumpdata" to a single fixture file)
>
> In particular, while you can make flush only load the initial_data from
apps without migrations, you cannot prevent people from putting instances
of models with migrations into those initial_data files.
>
> Thus, while you can modify flush to only load unmigrated apps, it's of
limited use in actually preventing the error occurring in the initial
ticket. That said, I don't see why it can't be fixed, and then documented
as such, it might be a little less surprising.
I understand that one app could load initial data for other apps with
migrations. There is no way we could prevent that. The only solution might
be abandon the whole loaddata command for Django 1.7+ apps.
However, my propose flush command change does make sense for some reusable
Django apps that could be dropped to any existing Django project. In my
case, it is Django-helpdesk (https://github.com/rossp/django-helpdesk),
which people just pip install it and put it into installed apps.
Supporting both Django 1.6+South and Django 1.7+ will require such apps to
have both old initial_data and the new migration based initial data.
--
Ticket URL: <https://code.djangoproject.com/ticket/23699#comment:6>
Django
Reporter: tony-zhu | Owner: nobody
Type: Bug | Status: new
commands) | Resolution:
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_tests: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/23699#comment:7>
Django
Reporter: tony-zhu | Owner: nobody
Type: Bug | Status: closed
Component: Core (Management | Version: 1.7
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"dd1ea70779adff294140eddb0229c382e12f32f3"]:
{{{
#!CommitTicketReference repository=""
revision="dd1ea70779adff294140eddb0229c382e12f32f3"
Fixed #23699 -- Prevented flush from loading initial data for apps with
migrations.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23699#comment:8>
Django
Reporter: tony-zhu | Owner: nobody
Type: Bug | Status: closed
Component: Core (Management | Version: 1.7
commands) | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"5cc8180a651b11f00a0a05296f8db15db1e48cdc"]:
{{{
#!CommitTicketReference repository=""
revision="5cc8180a651b11f00a0a05296f8db15db1e48cdc"
[1.7.x] Fixed #23699 -- Prevented flush from loading initial data for apps
with migrations.
Backport of dd1ea70779 from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23699#comment:9>
Django
Reporter: tony-zhu | Owner: nobody
Type: Bug | Status: closed
Component: Core (Management | Version: 1.7
commands) | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"abee4f718e6f08b523fc61fb812ef88d13a2e4b2"]:
{{{
#!CommitTicketReference repository=""
revision="abee4f718e6f08b523fc61fb812ef88d13a2e4b2"
[1.7.x] Fixed stable/1.7.x test failures from refs #23699.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23699#comment:10>