[Django] #23473: Django prompts to create new migrations when none are needed when a class is used in a field declaration
Django
is used in a field declaration
----------------------------+--------------------
Reporter: dlo | 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
----------------------------+--------------------
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('At this time, we do not
carry half-sizes. We recommend rounding up.')})
}}}
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23473>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
is used in a field declaration
Reporter: dlo | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by dlo):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Example project available at: https://github.com/dlo/django-migration-
bug-23473
Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/23473#comment:1>
Django
is used in a field declaration
Reporter: dlo | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Description changed by dlo:
Old description:
> 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('At this time, we do not
> carry half-sizes. We recommend rounding up.')})
> }}}
>
> 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.
New description:
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.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/23473#comment:2>
Django
is used in a field declaration
Reporter: dlo | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23473#comment:3>
Django
is used in a field declaration
Reporter: dlo | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markush2010):
* stage: Unreviewed => Accepted
Comment:
I can confirm this behavior. I suggest to document the requirement for a
`__eq__` method.
--
Ticket URL: <https://code.djangoproject.com/ticket/23473#comment:4>
Django
is used in a field declaration
Reporter: dlo | Owner: Markush2010
Type: Bug | Status: assigned
Component: Migrations | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markush2010):
* owner: nobody => Markush2010
* cc: info+coding@… (added)
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/23473#comment:5>
Django
is used in a field declaration
Reporter: dlo | Owner: Markush2010
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 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23473#comment:6>
Django
is used in a field declaration
Reporter: dlo | Owner: Markush2010
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 Markush2010):
Replying to [comment:6 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`:
{{{#!python
diff --git a/django/utils/deconstruct.py b/django/utils/deconstruct.py
index a51a169..b0443ee 100644
--- a/django/utils/deconstruct.py
+++ b/django/utils/deconstruct.py
@@ -49,6 +49,13 @@ def deconstructible(*args, **kwargs):
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:
{{{#!python
diff --git a/django/utils/deconstruct.py b/django/utils/deconstruct.py
index a51a169..7c3abac 100644
--- a/django/utils/deconstruct.py
+++ b/django/utils/deconstruct.py
@@ -49,6 +49,16 @@ def deconstructible(*args, **kwargs):
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:
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23473#comment:7>
Django
is used in a field declaration
Reporter: dlo | Owner: Markush2010
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 Markush2010):
* has_patch: 0 => 1
Comment:
I added a pull request with documentation updates:
https://github.com/django/django/pull/3271
--
Ticket URL: <https://code.djangoproject.com/ticket/23473#comment:8>
Django
is used in a field declaration
Reporter: dlo | Owner: Markush2010
Component: Migrations | Version: 1.7
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"066e672d79ceb416ac87e51c8fa400221fa8604f"]:
{{{
#!CommitTicketReference repository=""
revision="066e672d79ceb416ac87e51c8fa400221fa8604f"
Fixed #23473 -- Documented that @deconstructible classes need __eq__.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23473#comment:9>
Django
is used in a field declaration
Reporter: dlo | Owner: Markush2010
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"cf78a0ccc39902f73a1e1540b59c3b6c89aa51f6"]:
{{{
#!CommitTicketReference repository=""
revision="cf78a0ccc39902f73a1e1540b59c3b6c89aa51f6"
[1.7.x] Fixed #23473 -- Documented that @deconstructible classes need
__eq__.
Backport of 066e672d79 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23473#comment:10>