squashmigrations throws ValueError: need more than 1 value to unpack

[Piotr Maliński]

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,
            ),
        ]),
    ]

[Marten Kenbeek]

Confirmed, working on this now.

[Marten Kenbeek]

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.

[Marten Kenbeek]

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.

[Markus Holtermann]

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.

[Marten Kenbeek]

Replying to 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. (​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.

[Tim Graham]

Markus reviewed the PR.

[Markus Holtermann <info@…>]

In d597174b:

Refs #24278 -- Allowed multi-line serializations in OperationWriter.

Changed OperationWriter to support multi-line serialized values with
correct indentation.

[Markus Holtermann <info@…>]

In e8e4f978:

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.

[Markus Holtermann <info@…>]

In 651cc369:

[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

[Markus Holtermann <info@…>]

In 773387ce:

[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