[Django] #28980: Django doesn't check the type of one-off value
Django
----------------------------------------+------------------------
Reporter: Nikos Katsos | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 2.0
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 |
----------------------------------------+------------------------
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 = [
migrations.AddField(
model_name='x',
name='outcome',
field=models.CharField(default='TO', max_length=16),
preserve_default=False,
),
migrations.AddField(
model_name='x',
name='start_time',
field=models.DateTimeField(default=0 editable=False),
preserve_default=False,
),
]
}}}
when I found my error i couldn't really do anything
I changed
{{{
field=models.DateTimeField(default=0, editable=False),
}}}
to
{{{
field=models.DateTimeField(default=timezone.now,
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28980>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
--------------------------------------+------------------------------------
Reporter: Nikos Katsos | Owner: nobody
Component: Migrations | Version: 2.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/28980#comment:1>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: assigned
Component: Migrations | Version: 2.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jeff):
* cc: Jeff (added)
* owner: nobody => Jeff
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/28980#comment:2>
Django
--------------------------------------+------------------------------------
Reporter: Nikos Katsos | Owner: Jeff
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jeff):
If anyone is willing to give me a hand, I am close to a viable patch, see
[https://github.com/jeffyancey/django/commit/a6b9ee29e06ae10c8eda58a21485ad833a241adc
commit], however I am currently wrangling with an existing test
`tests/migrations/test_commands.py` -
`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/case.py", line 59, in testPartExecutor
yield
File "/usr/lib/python3.6/unittest/case.py", line 605, in run
testMethod()
File "/usr/lib/python3.6/unittest/mock.py", line 1179, in patched
return func(*args, **keywargs)
File "/home/jy/Django/forks/django-
repo/tests/migrations/test_commands.py", 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/__init__.py", line 148, in call_command
return command.execute(*args, **defaults)
File "/home/jy/Django/forks/django-repo/django/core/management/base.py",
line 354, in execute
output = self.handle(*args, **options)
File "/home/jy/Django/forks/django-repo/django/core/management/base.py",
line 83, in wrapped
res = handle_func(*args, **kwargs)
File "/home/jy/Django/forks/django-
repo/django/core/management/commands/makemigrations.py", line 161, in
handle
migration_name=self.migration_name,
File "/home/jy/Django/forks/django-
repo/django/db/migrations/autodetector.py", line 44, in changes
changes = self._detect_changes(convert_apps, graph)
File "/home/jy/Django/forks/django-
repo/django/db/migrations/autodetector.py", line 183, in _detect_changes
self.generate_added_fields()
File "/home/jy/Django/forks/django-
repo/django/db/migrations/autodetector.py", 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/autodetector.py", 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/questioner.py", line 224, in
ask_auto_now_add_addition
return self._ask_default(field_instance, default='timezone.now')
File "/home/jy/Django/forks/django-
repo/django/db/migrations/questioner.py", line 139, in _ask_default
if field_instance.to_python(code):
File "/home/jy/Django/forks/django-
repo/django/db/models/fields/__init__.py", line 1369, in to_python
parsed = parse_datetime(value)
File "/home/jy/Django/forks/django-repo/django/utils/dateparse.py", 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
model.
"""
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 'timezone.now' 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28980#comment:3>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 2.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jeff):
* owner: Jeff => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/28980#comment:4>
Django
--------------------------------------+------------------------------------
Reporter: Nikos Katsos | Owner: (none)
Component: Migrations | Version: 2.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28980#comment:5>