Django prompts to create new migrations when none are needed when a class is used in a field declaration

[Dan Loewenherz]

Create a model like so:

from django.db import models
from django.utils.deconstruct import deconstructible

@deconstructible
class IgnoreFormatString(object):
    def __init__(self, s):
        self.s = s

    def __str__(self):
        return self.s

    def __mod__(self, k):
        return self

class Product(models.Model):
    name = models.CharField(max_length=32, blank=True, null=True, error_messages={'invalid': IgnoreFormatString('Err, this is invalid.')})

Create a migration, and then run it. If you run "./manage.py migrate" once again, you'll get prompted with

  Your models have changes that are not yet reflected in a migration, and so won't be applied.
  Run 'manage.py makemigrations' to make new migrations, and then re-run 'manage.py migrate' to apply them.

Running makemigrations will create another migration, yet this message will continue to appear if the migration has already been applied.

[Dan Loewenherz]

Example project available at: ​https://github.com/dlo/django-migration-bug-23473

Thanks!

[Tim Graham]

I believe this is related to #22959 in that IgnoreFormatString must be comparable (add an __eq__ method). If so, we can probably add this to the documentation that should be added for that ticket.

[Markus Holtermann]

I can confirm this behavior. I suggest to document the requirement for a __eq__ method.

[Dan Loewenherz]

Is there any way we could also make the ./manage.py makemigrations command fail unless __eq__ is implemented? Otherwise, users might not know where to look to fix the problem.

[Markus Holtermann]

Replying to dlo:

Is there any way we could also make the ./manage.py makemigrations command fail unless __eq__ is implemented? Otherwise, users might not know where to look to fix the problem.

This is rather complicated as object itself has a __eq__ method. and would require some crazy comparisons. For what I've tested, this seems to work.

Two solutions that somehow work, but that I'm not really a friend of. The if clause is really error-prone.

1. Fails even on manage.py shell:

django/utils/deconstruct.py

@@ -49 +49 @@
         klass.__new__ = staticmethod(__new__)
         klass.deconstruct = deconstruct
 
+        if not hasattr(klass, '__eq__') or klass.__eq__ == object.__eq__:
+            raise ValueError(
+                "Cannot deconstruct class %s since no __eq__ method exists or "
+                "__eq__ is inherited from 'object'. This method is required "
+                "to prevent infinit migration creation"
+                % klass.__name__)
+
         return klass
 
     if not args:

2. Fails only when __eq__ is called:

django/utils/deconstruct.py

@@ -49 +49 @@
         klass.__new__ = staticmethod(__new__)
         klass.deconstruct = deconstruct
 
+        if not hasattr(klass, '__eq__') or klass.__eq__ == object.__eq__:
+            def __eq__(obj, other):
+                raise ValueError(
+                    "Cannot deconstruct class %s since no __eq__ method "
+                    "exists or __eq__ is inherited from 'object'. This method "
+                    "is required to prevent infinit migration creation"
+                    % obj.__class__.__name__)
+
+            klass.__eq__ = __eq__
+
         return klass
 
     if not args:

[Markus Holtermann]

I added a pull request with documentation updates: ​​https://github.com/django/django/pull/3271

[Tim Graham <timograham@…>]

In 066e672d79ceb416ac87e51c8fa400221fa8604f:

Fixed #23473 -- Documented that @deconstructible classes need eq.

[Tim Graham <timograham@…>]

In cf78a0ccc39902f73a1e1540b59c3b6c89aa51f6:

[1.7.x] Fixed #23473 -- Documented that @deconstructible classes need eq.

Backport of 066e672d79 from master