[Django] #26273: Use of @classmethod in queryset can crash manage.py if a model is changed
Django
------------------------------+--------------------------------------------
Reporter: scoenye | Owner: nobody
Type: Bug | Status: new
Component: | Version: 1.9
Uncategorized |
Severity: Normal | Keywords: classmethod queryset manage.py
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------------------------------
Starting with
{{{
class Graduation(models.Model):
ceremony_date = models.DateField()
...
@classmethod
def current_graduation(cls):
early = date.today() - timedelta(183)
late = date.today() + timedelta(183)
return cls.objects.get(ceremony_date__gte=early,
ceremony_date__lte=late)
class Graduate(models.Model):
conferral = models.ForeignKey(Graduation)
first_name = models.CharField(max_length = 14)
last_name = models.CharField(max_length = 19)
...
}}}
and
{{{
class TestReport(ListView):
queryset =
Graduate.objects.filter(conferral=Graduation.current_graduation()) \
.order_by("last_name", "first_name")
template_name = "graduation/reports/applicants.html"
context_object_name = "applicants"
}}}
The object is to automatically show the applicants for the next upcoming
graduation. This works like a charm until the Graduation model is
modified:
{{{
class Graduation(models.Model):
ceremony_date = models.DateField()
crash_test = models.CharField(max_length=10)
...
@classmethod
def current_graduation(cls):
early = date.today() - timedelta(183)
late = date.today() + timedelta(183)
return cls.objects.get(ceremony_date__gte=early,
ceremony_date__lte=late)
}}}
At this point
{{{
python manage.py makemigrations
}}}
crashes (and so does pretty much any invocation besides asking for --help)
Full stack trace attached, but the cause appears to be that the entire
application is loaded, triggering execution of the class method. This in
turn ends up asking the database for a column that does not exist yet as
the ORM generates a select including all field names.
Not defining queryset on the ListView and using get_queryset() fixes the
problem, but as I originally got bitten by this when rolling a changeset
out to production, I thought the condition is worth documenting.
--
Ticket URL: <https://code.djangoproject.com/ticket/26273>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: scoenye | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: classmethod queryset manage.py | Triage Stage: Unreviewed
Has patch: 0 | Easy pickings: 0
UI/UX: 0 |
Changes (by scoenye):
* Attachment "django_manage_crash.log" added.
Stack trace of manage.py crash
Django
schema changed.
-------------------------------------+-------------------------------------
Reporter: scoenye | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
queryset manage.py | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
Comment:
Hi scoenye,
From what I can see this has not much to do with migrations but is related
to the fact you should never execute queries at module level.
This is related to #25454 and probably a large number of closed tickets.
I wonder if should simply prevent `django.db` from executing queries until
`django.apps.apps.ready` or at least issue a `RuntimeWarning`.
We would have to go through deprecation but I'm pretty sure this would
uncover a lot of existing application bugs and prevent future ones.
--
Ticket URL: <https://code.djangoproject.com/ticket/26273#comment:1>
Django
schema changed.
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: classmethod | Triage Stage:
queryset manage.py | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by scoenye):
Thank you. I did search here, the docs, and Google, but it looks like my
keywords were not a good match for the other reported manifestations.
Perhaps adding an explicit note to the CBV documentation would help those
of us who are still feeling their way through Django avoid this pitfall?
As-is, it looks like the only reference to this issue is in the testing
documentation.
--
Ticket URL: <https://code.djangoproject.com/ticket/26273#comment:2>
Django
schema changed.
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Uncategorized | Version: 1.9
Severity: Normal | Resolution: invalid
Keywords: classmethod | Triage Stage:
queryset manage.py | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => closed
* resolution: => invalid
Comment:
I started [https://groups.google.com/d/topic/django-
developers/7JwWatLfP44/discussion a discussion on django-developers] based
on Simon's proposal to add a warning or prohibit module level queries.
Let's open a new ticket with action items based on the results.
--
Ticket URL: <https://code.djangoproject.com/ticket/26273#comment:3>