Opened 7 years ago

Closed 7 years ago

#27378 closed New feature (fixed)

Add UUID serialization support to migration writer

Reported by: Yuriy Korobko Owned by: Tim Graham <timograham@…>
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)

uudserializer.patch (3.9 KB ) - added by Yuriy Korobko 7 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 by Simon Charette, 7 years ago

Easy pickings: set
Triage Stage: UnreviewedAccepted
Version: 1.10master

comment:2 by Tim Graham, 7 years ago

Needs documentation: set
Needs tests: set
Summary: makemigration unable to serialize UUID when UUIDField have choices setAdd UUID serialization support to migration writer
Type: BugNew feature

Tests and documentation are needed for a complete patch. See 998894e1b9f5a8715f4cac5f6201b5e9ac80510c for a similar commit.

by Yuriy Korobko, 7 years ago

Attachment: uudserializer.patch added

in reply to:  2 comment:3 by Yuriy Korobko, 7 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 Tim Graham, 7 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 Maxime Lorant, 7 years ago

Needs documentation: unset
Needs tests: unset
Owner: changed from nobody to Maxime Lorant
Status: newassigned

comment:6 by Tim Graham, 7 years ago

comment:7 by Tobias McNulty, 7 years ago

Owner: changed from Maxime Lorant to Tobias McNulty

Reviewing this now (at the DUTH sprint).

comment:8 by Tobias McNulty, 7 years ago

Owner: Tobias McNulty removed
Patch needs improvement: set
Status: assignednew

comment:9 by Simon Charette, 7 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

Patch does LGTM.

comment:10 by Tim Graham <timograham@…>, 7 years ago

Owner: set to Tim Graham <timograham@…>
Resolution: fixed
Status: newclosed

In cb3fb34b:

Fixed #27378 -- Added support for serialization of uuid.UUID in migrations.

Thanks Yuriy Korobko for the initial patch and Tobias McNulty for review.

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