Opened 9 years ago
Closed 9 years ago
#27378 closed New feature (fixed)
Add UUID serialization support to migration writer
| Reported by: | Yuriy Korobko | Owned by: | |
|---|---|---|---|
| Component: | Migrations | Version: | dev |
| Severity: | Normal | Keywords: | uuid choices migration |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description
Using following model
import uuid
from django.db import models
# Create your models here.
class Student(models.Model):
FRESHMAN = uuid.UUID('5c859437-d061-4847-b3f7-e6b78852f8c8')
SOPHOMORE = uuid.UUID('c7853ec1-2ea3-4359-b02d-b54e8f1bcee2')
JUNIOR = uuid.UUID('94a43637-6dfc-4209-852a-bd9bd8c0101f')
SENIOR = uuid.UUID('dfb9cc50-c332-4809-9b4c-69c81a7489d0')
MENTOR = uuid.UUID('b8f3897a-0a0a-4630-8f1b-1ca4b0ee37ca')
YEAR_IN_SCHOOL_CHOICES = (
(FRESHMAN, 'Freshman'),
(SOPHOMORE, 'Sophomore'),
(JUNIOR, 'Junior'),
(SENIOR, 'Senior'),
(MENTOR, 'Mentor'),
)
year_in_school = models.UUIDField(choices=YEAR_IN_SCHOOL_CHOICES, default=FRESHMAN)
According to documentation it should work, and it works until you try to make migrations for this model, it fails with exception
Migrations for 'testapp':
testapp/migrations/0001_initial.py:
- Create model Student
Traceback (most recent call last):
File "./manage.py", line 22, in <module>
execute_from_command_line(sys.argv)
File "/home/yuriy/dev/uuidtest/env/lib/python3.5/site-packages/django/core/management/__init__.py", line 367, in execute_from_command_line
utility.execute()
File "/home/yuriy/dev/uuidtest/env/lib/python3.5/site-packages/django/core/management/__init__.py", line 359, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/home/yuriy/dev/uuidtest/env/lib/python3.5/site-packages/django/core/management/base.py", line 294, in run_from_argv
self.execute(*args, **cmd_options)
File "/home/yuriy/dev/uuidtest/env/lib/python3.5/site-packages/django/core/management/base.py", line 345, in execute
output = self.handle(*args, **options)
File "/home/yuriy/dev/uuidtest/env/lib/python3.5/site-packages/django/core/management/commands/makemigrations.py", line 189, in handle
self.write_migration_files(changes)
File "/home/yuriy/dev/uuidtest/env/lib/python3.5/site-packages/django/core/management/commands/makemigrations.py", line 224, in write_migration_files
migration_string = writer.as_string()
File "/home/yuriy/dev/uuidtest/env/lib/python3.5/site-packages/django/db/migrations/writer.py", line 163, in as_string
operation_string, operation_imports = OperationWriter(operation).serialize()
File "/home/yuriy/dev/uuidtest/env/lib/python3.5/site-packages/django/db/migrations/writer.py", line 120, in serialize
_write(arg_name, arg_value)
File "/home/yuriy/dev/uuidtest/env/lib/python3.5/site-packages/django/db/migrations/writer.py", line 84, in _write
arg_string, arg_imports = MigrationWriter.serialize(_arg_value)
File "/home/yuriy/dev/uuidtest/env/lib/python3.5/site-packages/django/db/migrations/writer.py", line 293, in serialize
return serializer_factory(value).serialize()
File "/home/yuriy/dev/uuidtest/env/lib/python3.5/site-packages/django/db/migrations/serializer.py", line 228, in serialize
return self.serialize_deconstructed(path, args, kwargs)
File "/home/yuriy/dev/uuidtest/env/lib/python3.5/site-packages/django/db/migrations/serializer.py", line 100, in serialize_deconstructed
arg_string, arg_imports = serializer_factory(arg).serialize()
File "/home/yuriy/dev/uuidtest/env/lib/python3.5/site-packages/django/db/migrations/serializer.py", line 43, in serialize
item_string, item_imports = serializer_factory(item).serialize()
File "/home/yuriy/dev/uuidtest/env/lib/python3.5/site-packages/django/db/migrations/serializer.py", line 43, in serialize
item_string, item_imports = serializer_factory(item).serialize()
File "/home/yuriy/dev/uuidtest/env/lib/python3.5/site-packages/django/db/migrations/serializer.py", line 388, in serializer_factory
"topics/migrations/#migration-serializing" % (value, get_docs_version())
ValueError: Cannot serialize: UUID('5c859437-d061-4847-b3f7-e6b78852f8c8')
There are some values Django cannot serialize into migration files.
For more, see https://docs.djangoproject.com/en/1.10/topics/migrations/#migration-serializing
Here is a small patch that add UUIDSerializer to migration serializers and tells serializer_factory to use it in case of UUID value
--- django/db/migrations/serializer.py.orig 2016-10-24 12:16:21.000000000 +0300
+++ django/db/migrations/serializer.py 2016-10-24 15:32:33.908818046 +0300
@@ -6,6 +6,7 @@
import functools
import math
import types
+import uuid
from importlib import import_module
from django.db import models
@@ -319,6 +320,10 @@
else:
return "%s.%s" % (module, self.value.__name__), {"import %s" % module}
+class UUIDSerializer(BaseSerializer):
+ def serialize(self):
+ return "uuid.UUID('%s')" % self.value, {"import uuid"}
+
def serializer_factory(value):
from django.db.migrations.writer import SettingsReference
@@ -382,6 +387,8 @@
return IterableSerializer(value)
if isinstance(value, (COMPILED_REGEX_TYPE, RegexObject)):
return RegexSerializer(value)
+ if isinstance(value, uuid.UUID):
+ return UUIDSerializer(value)
raise ValueError(
"Cannot serialize: %r\nThere are some values Django cannot serialize into "
"migration files.\nFor more, see https://docs.djangoproject.com/en/%s/"
After applying this change everything works as expected
(env) yuriy@yuriy-desktop:~/dev/uuidtest/src/uuidtest$ ./manage.py makemigrations
Migrations for 'testapp':
testapp/migrations/0001_initial.py:
- Create model Student
(env) yuriy@yuriy-desktop:~/dev/uuidtest/src/uuidtest$ ./manage.py migrate
Operations to perform:
Apply all migrations: admin, auth, contenttypes, sessions, testapp
Running migrations:
Applying contenttypes.0001_initial... OK
Applying auth.0001_initial... OK
Applying admin.0001_initial... OK
Applying admin.0002_logentry_remove_auto_add... OK
Applying contenttypes.0002_remove_content_type_name... OK
Applying auth.0002_alter_permission_name_max_length... OK
Applying auth.0003_alter_user_email_max_length... OK
Applying auth.0004_alter_user_username_opts... OK
Applying auth.0005_alter_user_last_login_null... OK
Applying auth.0006_require_contenttypes_0002... OK
Applying auth.0007_alter_validators_add_error_messages... OK
Applying auth.0008_alter_user_username_max_length... OK
Applying sessions.0001_initial... OK
Applying testapp.0001_initial... OK
(env) yuriy@yuriy-desktop:~/dev/uuidtest/src/uuidtest$
migratin produced is
# -*- coding: utf-8 -*-
# Generated by Django 1.10.2 on 2016-10-24 13:30
from __future__ import unicode_literals
from django.db import migrations, models
import uuid
class Migration(migrations.Migration):
initial = True
dependencies = [
]
operations = [
migrations.CreateModel(
name='Student',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('year_in_school', models.UUIDField(choices=[(uuid.UUID('5c859437-d061-4847-b3f7-e6b78852f8c8'), 'Freshman'), (uuid.UUID('c7853ec1-2ea3-4359-b02d-b54e8f1bcee2'), 'Sophomore'), (uuid.UUID('94a43637-6dfc-4209-852a-bd9bd8c0101f'), 'Junior'), (uuid.UUID('dfb9cc50-c332-4809-9b4c-69c81a7489d0'), 'Senior'), (uuid.UUID('b8f3897a-0a0a-4630-8f1b-1ca4b0ee37ca'), 'Mentor')], default=uuid.UUID('5c859437-d061-4847-b3f7-e6b78852f8c8'))),
],
),
]
I have no idea if it is correct, but works for me, if it should be fixed in other place could you point me where to look?
Thanks
Attachments (1)
Change History (11)
comment:1 by , 9 years ago
| Easy pickings: | set |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
| Version: | 1.10 → master |
follow-up: 3 comment:2 by , 9 years ago
| Needs documentation: | set |
|---|---|
| Needs tests: | set |
| Summary: | makemigration unable to serialize UUID when UUIDField have choices set → Add UUID serialization support to migration writer |
| Type: | Bug → New feature |
by , 9 years ago
| Attachment: | uudserializer.patch added |
|---|
comment:3 by , 9 years ago
Replying to Tim Graham:
Tests and documentation are needed for a complete patch. See 998894e1b9f5a8715f4cac5f6201b5e9ac80510c for a similar commit.
Can you please take look into attached patch?
comment:4 by , 9 years ago
Please create a pull request if possible and uncheck "Needs documentation" and "Needs tests" on the ticket so that it appears in the review queue.
comment:5 by , 9 years ago
| Needs documentation: | unset |
|---|---|
| Needs tests: | unset |
| Owner: | changed from to |
| Status: | new → assigned |
comment:8 by , 9 years ago
| Owner: | removed |
|---|---|
| Patch needs improvement: | set |
| Status: | assigned → new |
comment:9 by , 9 years ago
| Patch needs improvement: | unset |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
Patch does LGTM.
Tests and documentation are needed for a complete patch. See 998894e1b9f5a8715f4cac5f6201b5e9ac80510c for a similar commit.