[Django] #22289: Field with Validator always considered changed in migrations
Django
----------------------------+-------------------------
Reporter: blueyed | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7-alpha-2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+-------------------------
I am having the following model, and `schemamigration --auto` considers it
to be changed always.
This appears to get triggered through the RegexValidator (commenting it,
does not cause the model to be considered different every time).
{{{
from django.core.validators import RegexValidator
class Model(TimeStampedModel):
foo_id = models.CharField(
verbose_name='FOO ID',
max_length=200, # allow for URL in form, cleaned in clean_foo_id
validators =
[RegexValidator(r'(?i)^(?:(?:https?://)?www.foo.com/title/)?(tt\d+)(?:/.*)?$')],
help_text=_('…'),
blank=True,
null=True,
unique=True)
}}}
This _might_ be related to #22255, but probably only because it's about
the same field.. ;)
--
Ticket URL: <https://code.djangoproject.com/ticket/22289>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: blueyed | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7-alpha-2
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 erikr):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
I don't think it is related to #22255 - but it is related to #21275.
--
Ticket URL: <https://code.djangoproject.com/ticket/22289#comment:1>
Django
Reporter: blueyed | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: 1.7-alpha-2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by erikr):
* status: new => closed
* resolution: => invalid
Comment:
I don't think this is a bug in Django. In current master, there is no
command `schemamigration --auto`, there is only `makemigrations`. When I
test a fresh project with your model, using the new built-in schema
migrations, I can not reproduce this issue - it consistently works
correctly. Therefore, this may be a bug in South (which implements
`schemamigration --auto`, but does not appear to be one in Django.
--
Ticket URL: <https://code.djangoproject.com/ticket/22289#comment:2>
Django
Reporter: blueyed | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7-alpha-2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by blueyed):
* status: closed => new
* resolution: invalid =>
Comment:
Sorry for the confusion: I was copying the commented line from the
Makefile.
I am not using South anymore (removed from the virtualenv also), and use
`manage.py makemigrations app`.
Good to know that you cannot reproduce it though. I will see if I can come
up with a test case in the next days.
--
Ticket URL: <https://code.djangoproject.com/ticket/22289#comment:3>
Django
Reporter: blueyed | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7-alpha-2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by blueyed):
@erikr: have you tested it using PostgreSQL? (I am using it)
--
Ticket URL: <https://code.djangoproject.com/ticket/22289#comment:4>
Django
Reporter: blueyed | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7-alpha-2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by blueyed):
The problem is that the deconstructor (`<bound method
CharField.deconstruct of <django.db.models.fields.CharField: imdb_id>>`)
returns different values/objects for the validators:
{{{
ipdb> old_field_dec
(u'django.db.models.CharField', [], {u'null': True, u'validators':
[<django.core.validators.RegexValidator object at 0x1cac790>],
u'max_length': 200, u'blank': True, u'help_text': u'IMDB ID (starts with
"tt") or IMDB URL.', u'unique': True, u'verbose_name': 'IMDB ID'})
ipdb> new_field_dec
(u'django.db.models.CharField', [], {u'null': True, u'validators':
[<django.core.validators.RegexValidator object at 0x2cb5ad0>],
u'max_length': 200, u'blank': True, u'help_text':
<django.utils.functional.__proxy__ object at 0x2d872d0>, u'unique': True,
u'verbose_name': 'IMDB ID'})
ipdb> l
334 new_model_state = self.to_state.models[app_label,
model_name]
335 old_field_name = renamed_fields.get((app_label,
model_name, field_name), field_name)
336 old_field_dec =
old_model_state.get_field_by_name(old_field_name).deconstruct()[1:]
337 new_field_dec =
new_model_state.get_field_by_name(field_name).deconstruct()[1:]
338 if old_field_dec != new_field_dec:
4-> 339 self.add_to_migration(
340 app_label,
341 operations.AlterField(
342 model_name=model_name,
343 name=field_name,
344
field=new_model_state.get_field_by_name(field_name),
}}}
I have noticed that there is `AlterField.__eq__`, which does never get
called though?!
{{{
def __eq__(self, other):
return (
(self.__class__ == other.__class__) and
(self.name == other.name) and
(self.model_name.lower() == other.model_name.lower()) and
(self.field.deconstruct()[1:] ==
other.field.deconstruct()[1:])
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22289#comment:5>
Django
Reporter: blueyed | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7-alpha-2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by erikr):
I used SQLite, not PostgreSQL, but I strongly doubt that this is a
database-specific issue: validators aren't actually stored in the
database, as far as I know.
Regarding comment:5: it makes sense that the instances might be two
different ones, that's why there's a custom `__eq__`. The `__eq__` you're
looking at doesn't seem to be the right one, when comparing these two
tuples, it should call `__eq__` of `RegexValidator`. I notice that in
there, it compares `self.regex`, which at that point is a compiled regex,
not the original string. So the next step would be to look into whether,
in your case, `RegexValidator.__eq__` says they are not equal, and why.
--
Ticket URL: <https://code.djangoproject.com/ticket/22289#comment:6>
Django
Reporter: blueyed | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7-alpha-2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by blueyed):
Compiled patterns (_sre.SRE_Pattern) are considered to equal/identical for
the same pattern (from ipyhon shell).
RegexValidator's `__eq__` only tests for `==` for the SRE_Pattern, which
fails in my case:
{{{
ipdb> v1.regex
<_sre.SRE_Pattern object at 0x1b0e470>
ipdb> _v2.regex
<_sre.SRE_Pattern object at 0x2abbdd0>
ipdb> _v1.regex == _v2.regex
False
ipdb> _v1.regex.
_v1.regex.findall _v1.regex.flags _v1.regex.groups
_v1.regex.pattern _v1.regex.search _v1.regex.sub
_v1.regex.finditer _v1.regex.groupindex _v1.regex.match
_v1.regex.scanner _v1.regex.split _v1.regex.subn
ipdb> _v1.regex.pattern
'(?i)^(?:(?:https?://)?www.imdb.com/title/)?(tt\\d+)(?:/.*)?$'
ipdb> _v2.regex.pattern
'(?i)^(?:(?:https?://)?www.imdb.com/title/)?(tt\\d+)(?:/.*)?$'
ipdb> _v2.regex.flags
2
ipdb> _v1.regex.flags
2
ipdb> pprint(_v1)
<django.core.validators.RegexValidator object at 0x1aa3350>
ipdb> pprint(_v1.__dict__)
{'_constructor_args':
(('(?i)^(?:(?:https?://)?www.imdb.com/title/)?(tt\\d+)(?:/.*)?$',),
{}),
'regex': <_sre.SRE_Pattern object at 0x1b0e470>}
ipdb> pprint(_v2.__dict__)
{'_constructor_args':
(('(?i)^(?:(?:https?://)?www.imdb.com/title/)?(tt\\d+)(?:/.*)?$',),
{}),
'regex': <_sre.SRE_Pattern object at 0x2abbdd0>}
}}}
Maybe `RegexValidator.__eq__` needs to get enhanced, to compare the
relevant regex attributes?
I am using Python 2.7.5+ (Ubuntu Linux), on a 64bit.
Something must causing the SRE_Pattern objects to not get re-used in my
case.
{{{
(full output of all attributes:
ipdb> [_v1.regex.__getattribute__(x) for x in ['findall', 'finditer',
'flags', 'groupindex', 'groups', 'match', 'pattern', 'scanner', 'search',
'split', 'sub', 'subn']]
[<built-in method findall of _sre.SRE_Pattern object at 0x1b0e470>,
<built-in method finditer of _sre.SRE_Pattern object at 0x1b0e470>, 2, {},
1, <built-in method match of _sre.SRE_Pattern object at 0x1b0e470>,
'(?i)^(?:(?:https?://)?www.imdb.com/title/)?(tt\\d+)(?:/.*)?$', <built-in
method scanner of _sre.SRE_Pattern object at 0x1b0e470>, <built-in method
search of _sre.SRE_Pattern object at 0x1b0e470>, <built-in method split of
_sre.SRE_Pattern object at 0x1b0e470>, <built-in method sub of
_sre.SRE_Pattern object at 0x1b0e470>, <built-in method subn of
_sre.SRE_Pattern object at 0x1b0e470>]
ipdb> [_v2.regex.__getattribute__(x) for x in ['findall', 'finditer',
'flags', 'groupindex', 'groups', 'match', 'pattern', 'scanner', 'search',
'split', 'sub', 'subn']]
[<built-in method findall of _sre.SRE_Pattern object at 0x2abbdd0>,
<built-in method finditer of _sre.SRE_Pattern object at 0x2abbdd0>, 2, {},
1, <built-in method match of _sre.SRE_Pattern object at 0x2abbdd0>,
'(?i)^(?:(?:https?://)?www.imdb.com/title/)?(tt\\d+)(?:/.*)?$', <built-in
method scanner of _sre.SRE_Pattern object at 0x2abbdd0>, <built-in method
search of _sre.SRE_Pattern object at 0x2abbdd0>, <built-in method split of
_sre.SRE_Pattern object at 0x2abbdd0>, <built-in method sub of
_sre.SRE_Pattern object at 0x2abbdd0>, <built-in method subn of
_sre.SRE_Pattern object at 0x2abbdd0>]
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22289#comment:7>
Django
Reporter: blueyed | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7-alpha-2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by blueyed):
btw: the tests in `tests/validators` work fine, and they also compare
RegexValidator instances.
--
Ticket URL: <https://code.djangoproject.com/ticket/22289#comment:8>
Django
Reporter: blueyed | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7-alpha-2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by erikr):
Perhaps, but then I still find it suspicious that the tests work for you,
and that it seems to work for me. Could you publish a small demo
somewhere, which replicates the problem for you? Preferably with SQLite,
assuming you can replicate the problem with that. Then I can see whether I
can reproduce this myself somehow.
--
Ticket URL: <https://code.djangoproject.com/ticket/22289#comment:9>
Django
Reporter: blueyed | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: 1.7-alpha-2
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by bmispelon):
* status: new => closed
* resolution: => needsinfo
Comment:
I'm also unabled to reproduce this (tried sqlite and postgres).
I did make one change compared to the models you provided: I replaced
`TimeStampedModel` with a regular `models.Model`.
So maybe the problem is in `TimeStampedModel`?
On a sidenote, compiled regexp don't implement `__eq__` in python, so two
different regexp objects will always be different, even if they use the
same `pattern` and `flags`.
I'll close this as `needsinfo`. Can you reopen with either the full
definition of `TimestampedModel` or a fully contained testcase (something
that doesn't use external models)?
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/22289#comment:10>