Opened 11 years ago
Closed 11 years ago
#22289 closed Bug (needsinfo)
Field with Validator always considered changed in migrations
Reported by: | Daniel Hahler | Owned by: | nobody |
---|---|---|---|
Component: | Migrations | Version: | 1.7-alpha-2 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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.. ;)
Change History (10)
comment:1 by , 11 years ago
comment:2 by , 11 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
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.
comment:3 by , 11 years ago
Resolution: | invalid |
---|---|
Status: | closed → new |
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.
comment:5 by , 11 years ago
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:]) )
comment:6 by , 11 years ago
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.
comment:7 by , 11 years ago
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>] )
comment:8 by , 11 years ago
btw: the tests in tests/validators
work fine, and they also compare RegexValidator instances.
comment:9 by , 11 years ago
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.
comment:10 by , 11 years ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
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.
I don't think it is related to #22255 - but it is related to #21275.