Opened 3 years ago

Last modified 2 years ago

#28980 new Cleanup/optimization

Make the autodetector validate the type of one-off default values

Reported by: Nikos Katsos Owned by:
Component: Migrations Version: 2.0
Severity: Normal Keywords:
Cc: Jeff Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


As the title says Django doesn't check the type of one-off value. Recently I had a field like this

    start_time = models.DateTimeField(editable=False)

1) I gave to the field an one-off value

>>> 0 

2) then the migration file had those operations

    operations = [
            field=models.CharField(default='TO', max_length=16),
            field=models.DateTimeField(default=0 editable=False),

when I found my error i couldn't really do anything
I changed

            field=models.DateTimeField(default=0, editable=False),


            field=models.DateTimeField(, editable=False),

so after updating my migration file, I got an error "django.db.utils.OperationalError: (1060, "Duplicate column name "outcome")".
I could neither re-migrate because the operations isn't a bunch of queries in transaction, nor create a new migration.
Only reverting to zero and apply all migrations again worked.

This has to have a workaround.

Change History (5)

comment:1 Changed 3 years ago by Tim Graham

Summary: Django doesn't check the type of one-off valueMake the autodetector validate the type of one-off default values
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

comment:2 Changed 2 years ago by Jeff

Cc: Jeff added
Owner: changed from nobody to Jeff
Status: newassigned

comment:3 Changed 2 years ago by Jeff

If anyone is willing to give me a hand, I am close to a viable patch, see commit, however I am currently wrangling with an existing test tests/migrations/ - MakeMigrationsTests.test_makemigrations_auto_now_add_interactive. The test is failing like so:

ERROR: test_makemigrations_auto_now_add_interactive (migrations.test_commands.MakeMigrationsTests)
Traceback (most recent call last):
  File "/usr/lib/python3.6/unittest/", line 59, in testPartExecutor
  File "/usr/lib/python3.6/unittest/", line 605, in run
  File "/usr/lib/python3.6/unittest/", line 1179, in patched
    return func(*args, **keywargs)
  File "/home/jy/Django/forks/django-repo/tests/migrations/", line 1300, in test_makemigrations_auto_now_add_interactive
    call_command('makemigrations', 'migrations', interactive=True, stdout=out)
  File "/home/jy/Django/forks/django-repo/django/core/management/", line 148, in call_command
    return command.execute(*args, **defaults)
  File "/home/jy/Django/forks/django-repo/django/core/management/", line 354, in execute
    output = self.handle(*args, **options)
  File "/home/jy/Django/forks/django-repo/django/core/management/", line 83, in wrapped
    res = handle_func(*args, **kwargs)
  File "/home/jy/Django/forks/django-repo/django/core/management/commands/", line 161, in handle
  File "/home/jy/Django/forks/django-repo/django/db/migrations/", line 44, in changes
    changes = self._detect_changes(convert_apps, graph)
  File "/home/jy/Django/forks/django-repo/django/db/migrations/", line 183, in _detect_changes
  File "/home/jy/Django/forks/django-repo/django/db/migrations/", line 835, in generate_added_fields
    self._generate_added_field(app_label, model_name, field_name)
  File "/home/jy/Django/forks/django-repo/django/db/migrations/", line 854, in _generate_added_field
    field.default = self.questioner.ask_auto_now_add_addition(field, model_name)
  File "/home/jy/Django/forks/django-repo/django/db/migrations/", line 224, in ask_auto_now_add_addition
    return self._ask_default(field_instance, default='')
  File "/home/jy/Django/forks/django-repo/django/db/migrations/", line 139, in _ask_default
    if field_instance.to_python(code):
  File "/home/jy/Django/forks/django-repo/django/db/models/fields/", line 1369, in to_python
    parsed = parse_datetime(value)
  File "/home/jy/Django/forks/django-repo/django/utils/", line 106, in parse_datetime
    match = datetime_re.match(value)
TypeError: expected string or bytes-like object

I have confirmed that at this point the value being passed to to_python is the integer 1, which should indeed be an invalid default/one-off value to pass for a DateTime field.

At the moment what seems to be blocking me, I believe, is figuring out how unittest.mock.patch() works, and how that value is being set for the interactive test:

@mock.patch('builtins.input', return_value='1')
@mock.patch('django.db.migrations.questioner.sys.stdin', mock.MagicMock(encoding=sys.getdefaultencoding()))
def test_makemigrations_auto_now_add_interactive(self, *args):
    makemigrations prompts the user when adding auto_now_add to an existing
    class Entry(models.Model):
        title = models.CharField(max_length=255)
        creation_date = models.DateTimeField(auto_now_add=True)

        class Meta:
            app_label = 'migrations'

    # Monkeypatch interactive questioner to auto accept
    with mock.patch('django.db.migrations.questioner.sys.stdout', new_callable=io.StringIO) as prompt_stdout:
        out = io.StringIO()
        with self.temporary_migration_module(module='migrations.test_auto_now_add'):
            call_command('makemigrations', 'migrations', interactive=True, stdout=out)
        output = out.getvalue()
        prompt_output = prompt_stdout.getvalue()
        self.assertIn("You can accept the default '' by pressing 'Enter'", prompt_output)
        self.assertIn("Add field creation_date to entry", output)

If anyone could provide some guidance on how to properly work with this test, I'd appreciate it.

comment:4 Changed 2 years ago by Jeff

Owner: Jeff deleted
Status: assignednew

comment:5 Changed 2 years ago by Tim Graham

The value comes from return_value='1'. That test could be updated to use a valid value, however, it seems that DateTimeField.to_python() (and all to_python() methods) would need to be able to handle arbitrary Python types (and raise ValidationError) -- which may be difficult -- unless there's some broader exception catching in the migrations questioner.

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