[Django] #35882: Loop on all errors in `InteractiveMigrationQuestioner._ask

Django

unread,

Nov 2, 2024, 3:44:26 AM11/2/24

to django-...@googlegroups.com

#35882: Loop on all errors in `InteractiveMigrationQuestioner._ask_default()`
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Type:
| Cleanup/optimization
Status: new | Component:
| Migrations
Version: dev | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
When changing some fields, the migration questioner can ask for new field
default values entered through a Python prompt.

Yesterday, I made a typo which made the makemigrations process exit. This
was a little frustrating as there were several fields to fix, and my work
answering for the first fields was lost.

Currently, the questioner loops on `SyntaxError` and `NameError`
([https://github.com/django/django/blob/1feedc8ef8a34484cb5afe33f5c45b543b860210/django/db/migrations/questioner.py#L163
source]). My mistake lead to an `AttributeError`, hence the crash and
lost work. I propose we extend this clause to `Exception` - there's little
harm in displaying the error and looping.

Here's a reproducing example. Say you drop `null=True` from two model
fields:

{{{#!diff
diff --git example/models.py example/models.py
index 816fe37..9743877 100644
--- example/models.py
+++ example/models.py
@@ -2,5 +2,5 @@

class Book(models.Model):
- title = models.CharField(max_length=100, null=True)
- published_date = models.DateField(null=True)
+ title = models.CharField(max_length=100)
+ published_date = models.DateField()
}}}

`makemigrations` asks you about the first one:

{{{
$ ./manage.py makemigrations example
It is impossible to change a nullable field 'title' on book to non-
nullable without providing 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) Ignore for now. Existing rows that contain NULL values will have to be
handled manually, for example with a RunPython or RunSQL operation.
3) 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
>>> "Unknown book"
}}}

Immediately after this, it asks about the second field. Making a typo like
`.dat()` instead of `.date()` crashes the process:

{{{
It is impossible to change a nullable field 'published_date' on book to
non-nullable without providing 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) Ignore for now. Existing rows that contain NULL values will have to be
handled manually, for example with a RunPython or RunSQL operation.
3) 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
>>> datetime.dat(1970, 1, 1)
Traceback (most recent call last):
File "/..././manage.py", line 21, in <module>
main()
...
File "/.../.venv/.../django/db/migrations/questioner.py", line 162, in
_ask_default
return eval(code, {}, {"datetime": datetime, "timezone": timezone})
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<string>", line 1, in <module>
AttributeError: module 'datetime' has no attribute 'dat'. Did you mean:
'date'?
}}}

But if you instead made a typo in the `datetime` name, it would loop:

{{{
>>> datetme.date(1970,1,1)
Invalid input: name 'datetme' is not defined
>>>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35882>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Nov 2, 2024, 3:57:16 AM11/2/24

to django-...@googlegroups.com

#35882: Loop on all errors in `InteractiveMigrationQuestioner._ask_default()`
-------------------------------------+-------------------------------------

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Adam Johnson):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35882#comment:1>

Django

unread,

Nov 2, 2024, 4:23:19 AM11/2/24

to django-...@googlegroups.com

#35882: Loop on all errors in `InteractiveMigrationQuestioner._ask_default()`
-------------------------------------+-------------------------------------

Reporter: Adam Johnson | Owner: Harsh
Type: | Bansal
Cleanup/optimization | Status: assigned

Changes (by Harsh Bansal):

* owner: (none) => Harsh Bansal
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35882#comment:2>

Django

unread,

Nov 2, 2024, 4:47:29 AM11/2/24

to django-...@googlegroups.com

#35882: Loop on all errors in `InteractiveMigrationQuestioner._ask_default()`
-------------------------------------+-------------------------------------

Reporter: Adam Johnson | Owner: (none)
Type: | Status: new
Cleanup/optimization |

* owner: Harsh Bansal => (none)
* status: assigned => new

--
Ticket URL: <https://code.djangoproject.com/ticket/35882#comment:3>

Django

unread,

Nov 2, 2024, 6:44:17 AM11/2/24

to django-...@googlegroups.com

#35882: Loop on all errors in `InteractiveMigrationQuestioner._ask_default()`
-------------------------------------+-------------------------------------

Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned

Changes (by Adam Johnson):

* owner: (none) => Adam Johnson

* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35882#comment:4>

Django

unread,

Nov 2, 2024, 10:20:41 AM11/2/24

to django-...@googlegroups.com

#35882: Loop on all errors in `InteractiveMigrationQuestioner._ask_default()`
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* stage: Unreviewed => Accepted

Comment:

As long as `KeyboardInterrupt(BaseException)` is allowed to bubble up
(which catching for `Exception` allows) I agree that not trying to catch a
predefined set of exceptions is a better default.
--
Ticket URL: <https://code.djangoproject.com/ticket/35882#comment:5>

Django

unread,

Nov 14, 2024, 5:30:29 AM11/14/24

to django-...@googlegroups.com

#35882: Loop on all errors in `InteractiveMigrationQuestioner._ask_default()`
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/35882#comment:6>

Django

unread,

Nov 18, 2024, 9:15:55 AM11/18/24

to django-...@googlegroups.com

#35882: Loop on all errors in `InteractiveMigrationQuestioner._ask_default()`
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"3434fab758c5a293c8f376bb5990af6acbf89e32" 3434fab7]:
{{{#!CommitTicketReference repository=""
revision="3434fab758c5a293c8f376bb5990af6acbf89e32"
Refs #35882 -- Added test for migration questioner KeyboardInterrupt.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35882#comment:7>

Django

unread,

Nov 18, 2024, 9:15:56 AM11/18/24

to django-...@googlegroups.com

#35882: Loop on all errors in `InteractiveMigrationQuestioner._ask_default()`
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson

Cleanup/optimization | Status: closed
Component: Migrations | Version: dev
Severity: Normal | Resolution: fixed

Changes (by Sarah Boyce <42296566+sarahboyce@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"e035db1bc3bbeb4282a177ad106add3b07d97b09" e035db1]:
{{{#!CommitTicketReference repository=""
revision="e035db1bc3bbeb4282a177ad106add3b07d97b09"
Fixed #35882 -- Made migration questioner loop on all errors.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35882#comment:8>

Reply all

Reply to author

Forward

[Django] #35882: Loop on all errors in `InteractiveMigrationQuestioner._ask_default()`

Django

Django

Django

Django

Django

Django

Django

Django

Django