[Django] #23408: Automaticly created migration calls callable for default only once
Django
----------------------------+--------------------
Reporter: Harper04 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------
Callables on properties for ModelFields are used for various reasons. One
use case is to autocreate random file names or user passwords if not
present.
The migration seems to call them only once because after the migration
every "Buchung" has the same wlan_password.
My Model:
{{{
def random_wlan_key():
return
''.join(random.SystemRandom().choice("1234567890abcdefghkmnpqrstuvwxyz")
for i in range(9))
class Buchung(models.Model):
[...]
wlan_passwort = models.CharField(max_length=10,
default=random_wlan_key)
}}}
The generated migration:
{{{
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
from django.db import models, migrations
import main.models
class Migration(migrations.Migration):
dependencies = [
('main', '0001_initial'),
]
operations = [
migrations.AddField(
model_name='buchung',
name='wlan_passwort',
field=models.CharField(default=main.models.random_wlan_key,
max_length=10),
preserve_default=True,
),
]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23408>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Harper04 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
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 bmispelon):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* needs_tests: => 0
* needs_docs: => 0
Comment:
Hi,
I can reproduce this indeed. At this point, I'm not sure if this behavior
is by design or not.
If not, we should definitely fix this and if it is, we should document it.
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:1>
Django
Reporter: Harper04 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Harper04):
Hi,
thanks for your time.
I would argue that this behavior is not intended as best practice to
create fields like "created_at" fields is to assign a callable.
Calling it only once during a migration is killing dynamic behavior. What
if the default value of this field is dependent on another one?
regards
Tom
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:2>
Django
Reporter: Harper04 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markush2010):
* cc: Markush2010 (added)
Comment:
I wouldn't consider this a bug as the default value is nothing that is
enforced on database level but inside Django. If you want to have random
values per row, you need to add a
[https://docs.djangoproject.com/en/1.7/ref/migration-
operations/#django.db.migrations.operations.RunPython RunPython operation]
that updates the value per row.
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:3>
Django
Reporter: Harper04 | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: 1.7
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by andrewgodwin):
* status: new => closed
* resolution: => invalid
Comment:
This is exactly as designed; when creating columns databases only take a
single argument for the DEFAULT clause, so we can only provide a single
value, so we only call a callable once. If you want unique values per row,
you'll need to populate them in RunPython as Markush2010 suggests.
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:4>
Django
Reporter: Harper04 | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: 1.7
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by stavros):
I do indeed want unique values per row, and will populate them myself,
but, in that case, I don't want Django setting the default and having me
wonder down the line why the rows for which I didn't specify a value have
a default of 2014-06-24. The correct behavior there would be to ignore the
default completely, since there's no reasonable use case where it's
supposed to call the callable once and use it for all the columns.
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:5>
Django
Reporter: Harper04 | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: 1.7
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by michaeljohnbarr):
I am sorry - I modified the wrong widow - wasn't supposed to submit the
main window.
Callables on properties for ModelFields are used for various reasons. One
use case is to autocreate uuids.
My Models:
{{{
import uuid
class AbstractUUIDModel(models.Model):
[...]
uuid = models.UUIDField(default=uuid.uuid4, editable=False,
max_length=32, help_text='Universally unique identifier', unique=True,
verbose_name='UUID')
class Meta:
abstract = True
class Company(AbstractUUIDModel):
pass
}}}
The generated migration:
{{{
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
from django.db import models, migrations
import main.models
import uuid
class Migration(migrations.Migration):
dependencies = [
('main', '0001_initial'),
]
operations = [
migrations.AddField(
model_name='company',
name='uuid',
field=models.UUIDField(default=uuid.uuid4, editable=False,
max_length=32, help_text='Universally unique identifier', unique=True,
verbose_name='UUID'),
preserve_default=True,
),
]
}}}
Based on the documentation, you should add the default=uuid.uuid4 on the
models.UUIDField. However, you cannot do this with the current way
migrations work. Django currently tries to generate the same UUID for each
instance that is created. In this instance (Django 1.8 isn't released
yet), you will run into issues. If default is set to a callable, I believe
that it is implied that the result should be dynamically generated in
Python/Django. If the functionality is to stay as stated in the result of
this ticket, I would have to hand-code this migration for each app that
previously existed to first create a nullable UUIDField with no default,
then create a data migration to add distinct UUIDs to the Company, then
write a migrations.AlterField migration to set null=False. The alternative
might be to consider creating a special case for the UUIDField to allow
for non-duplicated UUIDs to be created during migrations.
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:6>
Django
Reporter: Harper04 | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: 1.7
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by dbrgn):
* cc: mail@… (added)
Comment:
I just ran into the same issue (trying to set default UUIDs on an UUID
field). Using per-row callables as default value in migrations would be
very useful, although it should probably be specified in an explicit way.
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:7>
Django
----------------------------+------------------------------------
Reporter: Harper04 | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: 1.7
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:8>
Django
----------------------------+------------------------------------
Type: Bug | Status: closed
Component: Migrations | Version: 1.7
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by rowanseymour):
I also ran into this problem with a UUID field. I would have thought that
this is a sufficiently common use case to be catered for in core. But core
devs feel that the onus is on people to write custom 3-step migrations
each time, then makemigrations should at least show a warning. I'd imagine
that in most cases, a single value for all rows is not what the user is
expecting. If makemigrations sees unique=True on the field then it knows
that the migration will fail.
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:9>
Django
----------------------------+------------------------------------
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by spookylukey):
* status: closed => new
* resolution: invalid =>
Comment:
It seems reasonable to me that `makemigrations` should, at the very least,
issue a warning when it finds a field with a `default` which is callable
and `unique==True`, and probably for all callable defaults when the field
is being added. I'm re-opening on that basis, hope that's OK with other
devs.
I think an ideal solution is that the warning should reference some
shortcut/info for implementing the needed migration.
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:10>
Django
-----------------------------+------------------------------------
Reporter: Harper04 | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* version: 1.7 => master
* type: Bug => New feature
Comment:
The warning could point to this documentation:
https://docs.djangoproject.com/en/dev/howto/writing-migrations
/#migrations-that-add-unique-fields
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:11>
Django
-----------------------------+------------------------------------
Type: New feature | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by max13fr):
Hello,
I'm still running this issue on Django five years later, is there any hope
to see this issue fixed ?
I don't understand why default id with primary key (so is unique too)
doesn't have the same issue.
Default values are normally used only when we are not providing a value
(or providing None value), so I don't understand why when I specify a
value, Django just uses it instead of replace it with default value.
What I'm missing ?
If needed here my example:
api/models.py :
{{{
class Model Test(models.Model):
id = models.UUIDField(primary_key=True, default=uuid.uuid4,
editable=False)
name = models.CharField(max_length=100)
}}}
api/fixtures/initial_data.json :
{{{
{
"model": "api.Test",
"pk": "724ea599-10a2-49a1-be2f-c1fcb04d003e",
"fields": {
"name": "Test 1"
}
}, {
"model": "api.Test",
"pk": "6bcb72f0-099b-416d-8b2c-fcd0bef76cbf",
"fields": {
"name": "Test 2"
}
}
}}}
Thanks in advance
Max
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:12>
Django
Reporter: Harper04 | Owner: Jacob Walls
Type: New feature | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jacob Walls):
* owner: nobody => Jacob Walls
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:13>
Django
-----------------------------+---------------------------------------
Reporter: Harper04 | Owner: Jacob Walls
Type: New feature | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jacob Walls):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/14935 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:14>
Django
-----------------------------+---------------------------------------
Reporter: Harper04 | Owner: Jacob Walls
Type: New feature | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jacob Walls):
* needs_better_patch: 0 => 1
Comment:
Marking PNI until I can move this over the to the interactive questioner
per Simon's review.
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:15>
Django
-----------------------------+---------------------------------------
Reporter: Harper04 | Owner: Jacob Walls
Type: New feature | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jacob Walls):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:16>
Django
Reporter: Harper04 | Owner: Jacob Walls
Type: New feature | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:17>
Django
-----------------------------+---------------------------------------------
Reporter: Harper04 | Owner: Jacob Walls
Component: Migrations | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"47f791f1328bb434bae312aa7ff190d49884395b" 47f791f]:
{{{
#!CommitTicketReference repository=""
revision="47f791f1328bb434bae312aa7ff190d49884395b"
Fixed #23408 -- Added migrations questioner prompt for adding unique
fields with a callable default.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23408#comment:18>