[Django] #24278: squashmigrations throws ValueError: need more than 1 value to unpack
Django
----------------------------+--------------------
Reporter: riklaunim | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------
When squashing several migrations I got an exception during
squashmigrations:
{{{
Traceback (most recent call last):
File "/env/bin/django-admin.py", line 5, in <module>
management.execute_from_command_line()
File "/env/lib/python3.4/site-
packages/django/core/management/__init__.py", line 385, in
execute_from_command_line
utility.execute()
File "/env/lib/python3.4/site-
packages/django/core/management/__init__.py", line 377, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/env/lib/python3.4/site-packages/django/core/management/base.py",
line 288, in run_from_argv
self.execute(*args, **options.__dict__)
File "/env/lib/python3.4/site-packages/django/core/management/base.py",
line 338, in execute
output = self.handle(*args, **options)
File "/env/lib/python3.4/site-
packages/django/core/management/commands/squashmigrations.py", line 122,
in handle
fh.write(writer.as_string())
File "/env/lib/python3.4/site-packages/django/db/migrations/writer.py",
line 130, in as_string
operation_string, operation_imports =
OperationWriter(operation).serialize()
File "/env/lib/python3.4/site-packages/django/db/migrations/writer.py",
line 87, in serialize
arg_string, arg_imports = MigrationWriter.serialize(arg_value)
File "/env/lib/python3.4/site-packages/django/db/migrations/writer.py",
line 260, in serialize
item_string, item_imports = cls.serialize(item)
File "/env/lib/python3.4/site-packages/django/db/migrations/writer.py",
line 327, in serialize
return cls.serialize_deconstructed(*value.deconstruct())
File "/env/lib/python3.4/site-packages/django/db/migrations/writer.py",
line 223, in serialize_deconstructed
module, name = path.rsplit(".", 1)
ValueError: need more than 1 value to unpack
}}}
The "path" variable got a value of "RenameField" instead of
django.something.someField. I found one "migrations.RenameField" in the
migrations set and after removing it (and "migrations.AlterField" which
also caused such exception) the squash passed.
The migration (operations) with those fields looks like so:
{{{
operations = [
migrations.SeparateDatabaseAndState(state_operations=[
migrations.RenameField(
model_name='somemodel',
old_name='some_old_name',
new_name='new_name',
),
migrations.AlterField(
model_name='somemodel',
name='new_name',
field=models.BooleanField(db_column='some_old_name',
default=False),
preserve_default=True,
),
]),
]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24278>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: riklaunim | Owner: knbk
Type: Bug | Status: assigned
Component: Migrations | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by knbk):
* status: new => assigned
* needs_better_patch: => 0
* needs_tests: => 0
* owner: nobody => knbk
* needs_docs: => 0
* stage: Unreviewed => Accepted
Comment:
Confirmed, working on this now.
--
Ticket URL: <https://code.djangoproject.com/ticket/24278#comment:1>
Django
Reporter: riklaunim | Owner: knbk
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by knbk):
Failing test case:
{{{
diff --git a/tests/migrations/test_writer.py
b/tests/migrations/test_writer.py
index 38065fb..4130e32 100644
--- a/tests/migrations/test_writer.py
+++ b/tests/migrations/test_writer.py
@@ -346,6 +346,18 @@ class WriterTests(TestCase):
self.assertSerializedEqual(FoodManager('a', 'b'))
self.assertSerializedEqual(FoodManager('x', 'y', c=3, d=4))
+ def test_serialize_operation(self):
+ """
+ Tests serializing an operation with MigrationWriter serialize.
This happens when
+ serializing an operation within an operation, e.g. with
SeparateDatabaseAndState.
+ """
+ operation = migrations.RenameField(
+ model_name="SomeModel",
+ old_name="old_name",
+ new_name="new_name",
+ )
+ self.assertSerializedEqual(operation)
+
def test_simple_migration(self):
"""
Tests serializing a simple migration.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24278#comment:2>
Django
Reporter: riklaunim | Owner: knbk
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by knbk):
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/4075
`MigrationsWriter._serialize_path()` now checks if there is a dot in the
path, and if there isn't, assumes it is a class in `django.db.migrations`.
As this module is always imported in generated migrations, I changed the
imports to an empty dict and the name to `migrations.<ClassName>`.
This bug was caused by the fact that `Operation.deconstruct()` returns the
class name as the first value instead of the full import path.
--
Ticket URL: <https://code.djangoproject.com/ticket/24278#comment:3>
Django
Reporter: riklaunim | Owner: knbk
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* cc: MarkusH (added)
* needs_better_patch: 0 => 1
Comment:
I don't think this is the way to go. I'd prefer to find a way to use the
`django.db.migrations.writer.OperationWriter` instead. This would also
take care of indenting the respective output.
--
Ticket URL: <https://code.djangoproject.com/ticket/24278#comment:4>
Django
Reporter: riklaunim | Owner: knbk
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by knbk):
Replying to [comment:4 MarkusH]:
> I don't think this is the way to go. I'd prefer to find a way to use the
`django.db.migrations.writer.OperationWriter` instead. This would also
take care of indenting the respective output.
Yes, that indeed seems to be a better approach. I've pushed a new patch
that uses the `OperationWriter` instead.
([https://github.com/django/django/pull/4075 PR])
I think I'm gonna try to refactor this so that the `isinstance(value,
Operation)` check is moved to `MigrationWriter.serialize()`. This would
leave the `_write` function in `OperationWriter.serialize()` as a more
general function, while `MigrationWriter.serialize()` handles the value-
specific serialization. `_write` would still need to be able to handle
multi-line values, but this also opens up the possibility for other
objects to serialize into a multi-line string.
--
Ticket URL: <https://code.djangoproject.com/ticket/24278#comment:5>
Django
Reporter: riklaunim | Owner: knbk
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by knbk):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24278#comment:6>
Django
Reporter: riklaunim | Owner: knbk
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
Markus reviewed the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/24278#comment:7>
Django
Reporter: riklaunim | Owner: knbk
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by mvantellingen):
* cc: michaelvantellingen@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/24278#comment:8>
Django
Reporter: riklaunim | Owner: knbk
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* needs_better_patch: 1 => 0
* version: 1.7 => 1.8
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24278#comment:9>
Django
Reporter: riklaunim | Owner: knbk
Component: Migrations | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markus Holtermann <info@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e8e4f978dd4b7a3d0c689c6e3301e3c6f9e50003" e8e4f978]:
{{{
#!CommitTicketReference repository=""
revision="e8e4f978dd4b7a3d0c689c6e3301e3c6f9e50003"
Fixed #24278 -- Fixed serialization of migration operations.
Fixed MigrationWriter.serialize() to correctly handle migration
operations by utilizing OperationWriter.
Thanks Piotr Maliński for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24278#comment:11>
Django
Reporter: riklaunim | Owner: knbk
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Markus Holtermann <info@…>):
In [changeset:"d597174bd4cfd9a0e1b34dae8b4d8d027a8cab72" d597174b]:
{{{
#!CommitTicketReference repository=""
revision="d597174bd4cfd9a0e1b34dae8b4d8d027a8cab72"
Refs #24278 -- Allowed multi-line serializations in OperationWriter.
Changed OperationWriter to support multi-line serialized values with
correct indentation.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24278#comment:10>
Django
Reporter: riklaunim | Owner: knbk
Component: Migrations | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Markus Holtermann <info@…>):
In [changeset:"651cc369ad803e116268c5ec7f0f8389858ae10b" 651cc369]:
{{{
#!CommitTicketReference repository=""
revision="651cc369ad803e116268c5ec7f0f8389858ae10b"
[1.8.x] Refs #24278 -- Allowed multi-line serializations in
OperationWriter.
Changed OperationWriter to support multi-line serialized values with
correct indentation.
Backport of d597174bd4cfd9a0e1b34dae8b4d8d027a8cab72 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24278#comment:12>
Django
Reporter: riklaunim | Owner: knbk
Type: Bug | Status: closed
Component: Migrations | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Markus Holtermann <info@…>):
In [changeset:"773387ce42de1c355e6ed6b9ddd49dc52474fc0a" 773387ce]:
{{{
#!CommitTicketReference repository=""
revision="773387ce42de1c355e6ed6b9ddd49dc52474fc0a"
[1.8.x] Fixed #24278 -- Fixed serialization of migration operations.
Fixed MigrationWriter.serialize() to correctly handle migration
operations by utilizing OperationWriter.
Thanks Piotr Maliński for the report.
Backport of e8e4f978dd4b7a3d0c689c6e3301e3c6f9e50003 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24278#comment:13>