[Django] #34988: Makemigrations shouldn't prompt for default values for non-nullable fields of other apps.
Django
of other apps.
-----------------------------------------+--------------------------------
Reporter: Sarah Boyce | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: dev
Severity: Normal | Keywords: makemigrations
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+--------------------------------
Have more than one app (app_1 and app_2). In app_1 have a model that has
it's migrations applied then add a field that doesn't have a default and
is not nullable (do not run makemigrations).
Then add a new model (or anything that would require a migration) to
app_2.
Run `manage.py makemigrations app_2`
You get prompted for every app that has unapplied migrations missing
defaults even though you're running migrations for a different app.
I would expect it not to care, as you can input temporary defaults to all
these other apps and it doesn't do anything with them.
I think this might be a test case:
{{{
diff --git a/tests/migrations/test_commands.py
b/tests/migrations/test_commands.py
index a9c1cdf893..518ebef872 100644
--- a/tests/migrations/test_commands.py
+++ b/tests/migrations/test_commands.py
@@ -1979,6 +1979,39 @@ class MakeMigrationsTests(MigrationTestBase):
self.assertIn("Remove field silly_field from sillymodel",
out.getvalue())
self.assertIn("Add field silly_rename to sillymodel",
out.getvalue())
+ @override_settings(
+ INSTALLED_APPS=[
+ "migrations",
+ "migrations.migrations_test_apps.migrated_app",
+ ]
+ )
+ def
test_makemigrations_interactive_not_null_addition_multiple_apps_single_call(self):
+ class Author(models.Model):
+ silly_author_field = models.BooleanField(null=False)
+
+ class Meta:
+ app_label = "migrations"
+
+ class NewModel1(models.Model):
+ class Meta:
+ app_label = "migrated_app"
+
+ input_msg = (
+ "It is impossible to add a non-nullable field
'silly_author_field' to "
+ "author without specifying a default. This is because the "
+ "database needs something to populate existing rows.\n"
+ "Please select a fix:\n"
+ " 1) Provide a one-off default now (will be set on all
existing "
+ "rows with a null value for this column)\n"
+ " 2) Quit and manually define a default value in models.py."
+ )
+ with
self.temporary_migration_module(module="migrations.test_migrations"):
+ with mock.patch("builtins.input", return_value="1"):
+ with captured_stdout() as out:
+ call_command("makemigrations", "migrated_app",
interactive=True)
+ output = out.getvalue()
+ self.assertNotIn(input_msg, output)
+
@mock.patch("builtins.input", return_value="Y")
def test_makemigrations_model_rename_interactive(self, mock_input):
class RenamedModel(models.Model):
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34988>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
of other apps.
Reporter: Sarah Boyce | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: dev
Keywords: makemigrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by David Sanders):
* stage: Unreviewed => Accepted
Comment:
Thanks again for another report 🏆🏆
Confirmed this is indeed happening for apps with mutually independent
migrations ✓
What's more is that if you run `makemigrations` a second time it asks for
a default but then reports "No changes detected in app <app-specified>"
{{{
django-sample % dj makemigrations ticket_34988_app_1
It is impossible to add a non-nullable field 'foo' to app2 without
specifying a default. This is because the database needs something to
populate existing rows.
Please select a fix:
1) Provide a one-off default now (will be set on all existing rows with a
null value for this column)
2) Quit and manually define a default value in models.py.
Select an option: 1
Please enter the default value as valid Python.
The datetime and django.utils.timezone modules are available, so it is
possible to provide e.g. timezone.now as a value.
Type 'exit' to exit this prompt
>>> 1
It is impossible to add a non-nullable field 'bar' to app1 without
specifying a default. This is because the database needs something to
populate existing rows.
Please select a fix:
1) Provide a one-off default now (will be set on all existing rows with a
null value for this column)
2) Quit and manually define a default value in models.py.
Select an option: 1
Please enter the default value as valid Python.
The datetime and django.utils.timezone modules are available, so it is
possible to provide e.g. timezone.now as a value.
Type 'exit' to exit this prompt
>>> 1
Migrations for 'ticket_34988_app_1':
ticket_34988_app_1/migrations/0002_app2_foo.py
- Add field foo to app2
django-sample % dj makemigrations ticket_34988_app_1
It is impossible to add a non-nullable field 'bar' to app1 without
specifying a default. This is because the database needs something to
populate existing rows.
Please select a fix:
1) Provide a one-off default now (will be set on all existing rows with a
null value for this column)
2) Quit and manually define a default value in models.py.
Select an option: 1
Please enter the default value as valid Python.
The datetime and django.utils.timezone modules are available, so it is
possible to provide e.g. timezone.now as a value.
Type 'exit' to exit this prompt
>>> 1
No changes detected in app 'ticket_34988_app_1'
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34988#comment:1>
Django
of other apps.
--------------------------------+------------------------------------
Reporter: Sarah Boyce | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: makemigrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+------------------------------------
Comment (by David Sanders):
Just thinking about whether this is feasible to do: The autodetector is
documented as being designed to run on entire projects but I wonder if
it's possible to delay prompting the questioner for various things it
requests until after `_trim_changes()` is called.
--
Ticket URL: <https://code.djangoproject.com/ticket/34988#comment:2>
Django
of other apps.
--------------------------------+------------------------------------
Reporter: Sarah Boyce | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: makemigrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+------------------------------------
Comment (by Mariusz Felisiak):
Follow up to #22791.
--
Ticket URL: <https://code.djangoproject.com/ticket/34988#comment:3>
Django
of other apps.
--------------------------------+------------------------------------
Reporter: Sarah Boyce | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: makemigrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+------------------------------------
* cc: Bhuvnesh (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/34988#comment:4>
Django
of other apps.
Reporter: Sarah Boyce | Owner: nobody
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: makemigrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* type: Uncategorized => Cleanup/optimization
--
Ticket URL: <https://code.djangoproject.com/ticket/34988#comment:5>
Django
of other apps.
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: makemigrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* owner: nobody => Bhuvnesh
* status: new => assigned
Comment:
>The autodetector is documented as being designed to run on entire
projects but I wonder if it's possible to delay prompting the questioner
for various things it requests until after _trim_changes() is called.
Hi, David ! I cannot fully understand what you are trying the propose, it
will be helpful if you could please elaborate this a bit. 😅
I'm working on one more approach - passing app_label from auto-detector
and restricting questioner for apps other than specified apps. This is
working for independent apps but not for dependent apps. Need to look more
into this.
--
Ticket URL: <https://code.djangoproject.com/ticket/34988#comment:6>