Opened 8 years ago
Closed 8 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 , 8 years ago
Easy pickings: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
Version: | 1.10 → master |
follow-up: 3 comment:2 by , 8 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 , 8 years ago
Attachment: | uudserializer.patch added |
---|
comment:3 by , 8 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 , 8 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 , 8 years ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
Owner: | changed from | to
Status: | new → assigned |
comment:8 by , 8 years ago
Owner: | removed |
---|---|
Patch needs improvement: | set |
Status: | assigned → new |
comment:9 by , 8 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.