[Django] #22951: Adding 'deconstruct' method breaks serialization of type
Django
-------------------------------------------------+-------------------------
Reporter: Sam Hartsfield | Owner: nobody
<samh.public+django@…> | Status: new
Type: Bug | Version: 1.7-rc-1
Component: Migrations | Keywords:
Severity: Normal | Has patch: 0
Triage Stage: Unreviewed | UI/UX: 0
Easy pickings: 0 |
-------------------------------------------------+-------------------------
A custom field may take a type as as argument (for example, an Enum type).
Normally, class references are one of the things the migrations framework
can serialize by default
([https://docs.djangoproject.com/en/dev/topics/migrations/#serializing-
values]). However, we probably also want to serialize the value, otherwise
we get an error (see `ValueEnum` in the example).
To serialize the value, we add a `deconstruct` method (in the example of
`ValueEnum2`, this uses the `deconstructible` decorator). I would expect
that, after adding this, the field would work. However,
'''`django.db.migrations.autodetector.deep_deconstruct` tries to call
`deconstruct` on the class.'''
`ValueEnum3` is an example of a workaround, which should probably not be
required.
I would expect Django to either check whether something is a type before
calling `deconstruct`, or to not call `deconstruct` if it is an instance
method (e.g. you might want to be able to also add a `deconstruct`
classmethod to customize deconstruction of the class).
{{{
#!python
# Requires Python 3.4 or the 'enum34' package
import enum
import types
from django.db import models
from django.utils.deconstruct import deconstructible
from django.utils.six import with_metaclass
class SimpleEnumField(with_metaclass(models.SubfieldBase,
models.IntegerField)):
def __init__(self, enum_type, **kwargs):
self.enum = enum_type
choices = [(e.value, e.name) for e in enum_type]
super(SimpleEnumField, self).__init__(choices=choices, **kwargs)
def deconstruct(self):
name, path, args, kwargs = super(SimpleEnumField,
self).deconstruct()
kwargs['enum_type'] = self.enum
kwargs.pop('choices', None)
return name, path, args, kwargs
def get_prep_value(self, value):
return value.value
def to_python(self, value):
return self.enum(value)
class ValueEnum(enum.Enum):
"""
Django will be able to serialize the type, but not the instance::
ValueError: Cannot serialize: <ValueEnum.A: 0>
"""
A = 0
B = 1
@deconstructible
class ValueEnum2(enum.Enum):
"""
The deconstructible decorator allows Django to serialize the
instances,
but breaks serialization of the type--it will try to call deconstruct
on the type, but it will fail because it is an instance method::
TypeError: deconstruct() missing 1 required positional argument:
'obj'
"""
A = 0
B = 1
def deconstruct_enum(self):
return (
'%s.%s' % (self.__class__.__module__, self.__class__.__name__),
(self.value,),
{}
)
class ValueEnum3(enum.Enum):
"""
Workaround: if we add the deconstruct method to the instances, but not
to the type, then Django will do the right thing.
"""
A = 0
B = 1
def __init__(self, value):
self.deconstruct = types.MethodType(deconstruct_enum, self)
class Foo(models.Model):
# This field works
field1 = SimpleEnumField(ValueEnum)
# ValueError: Cannot serialize: <ValueEnum.A: 0>
# field2 = SimpleEnumField(ValueEnum, default=ValueEnum.A)
# TypeError: deconstruct() missing 1 required positional argument:
'obj'
# field3 = SimpleEnumField(ValueEnum2, default=ValueEnum2.A)
field4 = SimpleEnumField(ValueEnum3, default=ValueEnum3.A)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22951>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Sam Hartsfield | Owner: nobody
<samh.public+django@…> | Status: new
Type: Bug | Version: 1.7-rc-1
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by charettes):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/22951#comment:1>
Django
Reporter: Sam Hartsfield | Owner: nobody
<samh.public+django@…> | Status: new
Type: Bug | Version: 1.7-rc-1
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by andrewgodwin):
* easy: 0 => 1
Comment:
This should be an easy change to deep_deconstruct to change "doesn't have
deconstruct: return" to "doesn't have deconstruct or is a type: return".
--
Ticket URL: <https://code.djangoproject.com/ticket/22951#comment:2>
Django
Reporter: Sam Hartsfield | Owner:
Type: Bug | Status: assigned
Component: Migrations | Version: 1.7-rc-1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by jambonrose):
* status: new => assigned
* owner: nobody => jambonrose
--
Ticket URL: <https://code.djangoproject.com/ticket/22951#comment:3>
Django
Reporter: Sam Hartsfield | Owner:
<samh.public+django@…> | jambonrose
Type: Bug | Status: assigned
Component: Migrations | Version: 1.7-rc-1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by jambonrose):
I have just issued [https://github.com/django/django/pull/3177 pull-
request 3177] to fix the issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/22951#comment:4>
Django
Reporter: Sam Hartsfield | Owner:
<samh.public+django@…> | jambonrose
Type: Bug | Status: assigned
Component: Migrations | Version: 1.7-rc-1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/22951#comment:5>
Django
Reporter: Sam Hartsfield | Owner:
<samh.public+django@…> | jambonrose
Component: Migrations | Version: 1.7-rc-1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"4680d25df23875bced0f17d361782bda80ce79d6"]:
{{{
#!CommitTicketReference repository=""
revision="4680d25df23875bced0f17d361782bda80ce79d6"
Fixed #22951 -- Checked for types during deep_deconstruct migration
serialization process.
Thanks Sam Hartsfield for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22951#comment:6>
Django
Reporter: Sam Hartsfield | Owner:
<samh.public+django@…> | jambonrose
Type: Bug | Status: closed
Component: Migrations | Version: 1.7-rc-1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"27e7972e63c6f7d6555a724a3b356b4e08b0fefa"]:
{{{
#!CommitTicketReference repository=""
revision="27e7972e63c6f7d6555a724a3b356b4e08b0fefa"
[1.7.x] Fixed #22951 -- Checked for types during deep_deconstruct
migration serializ
Thanks Sam Hartsfield for the report.
Backport of 4680d25df2 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22951#comment:7>
Django
Reporter: Sam Hartsfield | Owner:
<samh.public+django@…> | jambonrose
Component: Migrations | Version: 1.7-rc-1
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: fixed =>
Comment:
I don't think this was fixed everywhere it needed to be -- the migration
writer is still calling deconstruct on types here:
https://github.com/django/django/blob/master/django/db/migrations/writer.py#L334
{{{
Traceback (most recent call last):
File "./manage.py", line 11, in <module>
execute_from_command_line(sys.argv)
File "django/django/core/management/__init__.py", line 385, in
execute_from_command_line
utility.execute()
File "django/core/management/__init__.py", line 377, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "django/core/management/base.py", line 288, in run_from_argv
self.execute(*args, **options.__dict__)
File "django/core/management/base.py", line 338, in execute
output = self.handle(*args, **options)
File "django/core/management/commands/makemigrations.py", line 124, in
handle
self.write_migration_files(changes)
File "django/core/management/commands/makemigrations.py", line 152, in
write_migration_files
migration_string = writer.as_string()
File "django/db/migrations/writer.py", line 131, in as_string
operation_string, operation_imports =
OperationWriter(operation).serialize()
File "django/db/migrations/writer.py", line 88, in serialize
arg_string, arg_imports = MigrationWriter.serialize(arg_value)
File "django/db/migrations/writer.py", line 331, in serialize
return cls.serialize_deconstructed(path, args, kwargs)
File "django/db/migrations/writer.py", line 239, in
serialize_deconstructed
arg_string, arg_imports = cls.serialize(arg)
File "django/db/migrations/writer.py", line 334, in serialize
return cls.serialize_deconstructed(*value.deconstruct())
TypeError: unbound method deconstruct() must be called with FooEnum
instance as first argument (got nothing instead)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22951#comment:8>
Django
Reporter: Sam Hartsfield | Owner:
<samh.public+django@…> | jambonrose
Component: Migrations | Version: 1.7-rc-1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
Could you please open a new ticket? The fix for this one was already
released. Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/22951#comment:9>