Opened 10 years ago
Closed 10 years ago
#24483 closed Bug (fixed)
keepdb migrations break choices as generators
Reported by: | David Szotten | Owned by: | |
---|---|---|---|
Component: | Migrations | Version: | 1.8beta2 |
Severity: | Normal | Keywords: | migrations test keepdb |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
So, not got to the bottom of this yet, but wanted to get a ticket in, in case we want to block the release on this
To reproduce:
# models.py from django.db import models def gen(): for x in 'abc': yield (x, x) class MyModel(models.Model): foo = models.CharField(choices=gen(), max_length=1) # settings.py DATABASES = { 'default': { 'ENGINE': 'django.db.backends.sqlite3', 'NAME': os.path.join(BASE_DIR, 'db.sqlite3'), 'TEST': { # make sure we use a db we can keep between tests 'NAME': os.path.join(BASE_DIR, 'test_db.sqlite3'), }, } } # tests.py from django.test import TestCase from .models import MyModel class Test(TestCase): def test_choices(self): field = MyModel._meta.get_field('foo') expected = [('a', 'a'), ('b', 'b'), ('c', 'c')] self.assertEqual( list(field.choices), expected, ) # shell (note; need to run twice; only happens when re-using db) $ ./manage.py test -v2 -k $ ./manage.py test -v2 -k ====================================================================== FAIL: test_choices (app.tests.Test) ---------------------------------------------------------------------- Traceback (most recent call last): File "/Users/david/dev/django_bugs/choices_generator/app/tests.py", line 12, in test_choices expected, AssertionError: Lists differ: [] != [('a', 'a'), ('b', 'b'), ('c',... Second list contains 3 additional elements. First extra element 0: ('a', 'a') - [] + [('a', 'a'), ('b', 'b'), ('c', 'c')]
interestingly, this only breaks when verbosity
is 2, i think because this is the trigger:
https://github.com/django/django/blob/a52cd407b86a51e1badf6771e590361e24fd7155/django/core/management/commands/migrate.py#L177
Change History (10)
comment:1 by , 10 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
Type: | Uncategorized → Bug |
comment:2 by , 10 years ago
comment:3 by , 10 years ago
#3490 was specifically added to support the use of generators with choices, so we probably should find and fix the problem. Though I'm unsure whether or not this should be a blocker.
comment:4 by , 10 years ago
not sure what you mean by "Does this even work on a live website?" I have a generator providing choices in an app i've had in production for a good few years now (though probably using a generator mostly by accident). I noticed this issue a few days ago as i was trying out the new 1.8 beta and --keepdb
to speed up my test suite.
the patch updates code that was definitely put in place to support generic iterators, and as you point out, the docs also talk about iterables.
not sure of the correct protocol, so the pr is raised against 1.8.x, not master (i might be going crazy, but every other time i look i can't find the pr listed on the ticket, so here's a direct link: https://github.com/django/django/pull/4326 )
comment:5 by , 10 years ago
Owner: | changed from | to
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:8 by , 10 years ago
Owner: | removed |
---|---|
Patch needs improvement: | set |
Status: | assigned → new |
comment:9 by , 10 years ago
Patch needs improvement: | unset |
---|
I'm not so sure that django really supports the use of generators for choices. The docs do say "An iterable (e.g., a list or tuple) consisting itself of iterables of exactly two items (e.g. [(A, B), (A, B) ...])" which a generator is, but I can't see the value in having it as a generator in the first place as it's going to be consumed in its entirety fairly quickly in the load process.
Does this even work on a live website? If yes, then we should probably fix it. But I doubt we'd mark it as a release blocker due to the necessary combinations of actually triggering the problem (the impact is small).