Opened 10 years ago

Closed 10 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 10 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 by Simon Charette, 10 years ago

Easy pickings: set
Triage Stage: Unreviewed → Accepted
Version: 1.10 → master

comment:2 by Tim Graham, 10 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

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

by Yuriy Korobko, 10 years ago

Attachment: uudserializer.patch​ added

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

Needs documentation: unset
Needs tests: unset
Owner: changed from nobody to Maxime Lorant
Status: new → assigned

comment:6 by Tim Graham, 10 years ago

comment:7 by Tobias McNulty, 10 years ago

Owner: changed from Maxime Lorant to Tobias McNulty

Reviewing this now (at the DUTH sprint).

comment:8 by Tobias McNulty, 10 years ago

Owner: Tobias McNulty removed
Patch needs improvement: set
Status: assigned → new

comment:9 by Simon Charette, 10 years ago

Patch needs improvement: unset
Triage Stage: Accepted → Ready for checkin

Patch does LGTM.

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

Owner: set to Tim Graham <timograham@…>
Resolution: → fixed
Status: new → closed

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