Re: [Django] #25701: Add warning to an admin list_view if too many queries are being used

[Django]

#25701: Add warning to an admin list_view if too many queries are being used
+
 Reporter:  Jacinda Shelly  |Owner:  (none)
 Type:  New feature |   Status:  new
Component:  contrib.admin   |  Version:  dev
 Severity:  Normal  |   Resolution:
 Keywords:  | Triage Stage:  Accepted
Has patch:  0   |  Needs documentation:  0
  Needs tests:  0   |  Patch needs improvement:  0
Easy pickings:  0   |UI/UX:  0
+
Changes (by Ülgen Sarıkavak):

 * cc: Ülgen Sarıkavak (added)

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018e506f5665-530129f3-9127-4da3-ae55-11dc1ff584c8-00%40eu-central-1.amazonses.com.

Re: [Django] #25701: Add warning to an admin list_view if too many queries are being used

[Django]

#25701: Add warning to an admin list_view if too many queries are being used
+
 Reporter:  Jacinda Shelly  |Owner:  (none)
 Type:  New feature |   Status:  new
Component:  contrib.admin   |  Version:  dev
 Severity:  Normal  |   Resolution:
 Keywords:  | Triage Stage:  Accepted
Has patch:  0   |  Needs documentation:  0
  Needs tests:  0   |  Patch needs improvement:  0
Easy pickings:  0   |UI/UX:  0
+
Changes (by Mariusz Felisiak):

 * owner:  Jacinda Shelly => (none)
 * status:  assigned => new


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.79357e8e66064968c0c6c774d7f29e2c%40djangoproject.com.

Re: [Django] #25701: Add warning to an admin list_view if too many queries are being used

[Django]

#25701: Add warning to an admin list_view if too many queries are being used
---+
 Reporter:  jacinda|Owner:  jacinda
 Type:  New feature|   Status:  assigned
Component:  contrib.admin  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Accepted
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+

Comment (by jacinda):

 I'll try to get an example setup with messages going to logging instead of
 `django.contrib.messages`.  Maybe it would be more visible if the messages
 were logged at a WARNING level by default?  We could add an option to set
 this level or disable the logging entirely if a developer didn't want it.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.3c42516ebdb8e505645926edd2ff0ce1%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25701: Add warning to an admin list_view if too many queries are being used

[Django]

#25701: Add warning to an admin list_view if too many queries are being used
---+
 Reporter:  jacinda|Owner:  jacinda
 Type:  New feature|   Status:  assigned
Component:  contrib.admin  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Accepted
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+

Comment (by aaugustin):

 Indeed, if the messages go to logging, there's no need for a mechanism to
 opt-out of this feature.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.371f8e7a655161c1925dde8fd98dd027%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25701: Add warning to an admin list_view if too many queries are being used

[Django]

#25701: Add warning to an admin list_view if too many queries are being used
---+
 Reporter:  jacinda|Owner:  jacinda
 Type:  New feature|   Status:  assigned
Component:  contrib.admin  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Accepted
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+
Changes (by timgraham):

 * version:   => master
 * stage:  Unreviewed => Accepted


Comment:

 Using the console log handler seems reasonable to me (maybe at the INFO
 level?). Aymeric, could you elaborate on why you want a `ModelAdmin`
 option to disable it? I think it would be easy enough to ignore the
 message if you don't care about it, but maybe there's some other reason
 you had in mind? If we do add such an option, I wonder if it's better
 suited to `ChangeList` (which we'd likely need to start documenting as a
 public API).

 Related the last point, Ola pointed out that the
 `ModelAdmin.get_search_results()` method really belongs on `ChangeList`,
 but we put it on `ModelAdmin` because the latter class isn't documented
 and requires a bit more work to override (using
 `ModelAdmin.get_changelist()`).

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.db7b7cc7a469aeeba22161d09972d5f1%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25701: Add warning to an admin list_view if too many queries are being used

[Django]

#25701: Add warning to an admin list_view if too many queries are being used
---+--
 Reporter:  jacinda|Owner:  jacinda
 Type:  New feature|   Status:  assigned
Component:  contrib.admin  |  Version:
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Unreviewed
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+--

Comment (by aaugustin):

 We discussed this at Django under the Hood. There aren't that many ways to
 detect and report the problem at runtime. I think a check when `DEBUG =
 True` is a good idea.

 I've had this issue on sites where I use the admin and where I have the
 debug toolbar installed in dev. I only discovered and fixed in when I
 wondered "doesn't this page feel a bit slow?" I wasn't obvious — just the
 difference between 50 and 250 ms.

 In my opinion this feature would achieve two results:

 1. educate newcomers to Django about the N + 1 queries problem in a
 textbook example
 2. quickly alert more experienced developers when the accidentally create
 such a problem

 It would need a way to turn it off on with a ModelAdmin option.

 I agree that it feels weird to use `django.contrib.messages` to relay
 messages to the developer at runtime. Furthermore, I just realized we have
 an ordering problem: the admin templates renders messages before iterating
 on the queryset to render the changelist. I can't see how we'd implement
 that.

 I suppose the more traditional alternative to relay that information to
 the developer is through logging.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.03c0aa2bbe2a5cd80ab64981d60bb410%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25701: Add warning to an admin list_view if too many queries are being used

[Django]

#25701: Add warning to an admin list_view if too many queries are being used
---+--
 Reporter:  jacinda|Owner:  jacinda
 Type:  New feature|   Status:  assigned
Component:  contrib.admin  |  Version:
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Unreviewed
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+--

Comment (by jacinda):

 I don't think this would be a replacement for django-debug-toolbar.  I
 think it's actually most useful if the warning suggests to the user that
 they use django-debug-toolbar or a similar tool to diagnose the cause of
 the high number of queries.  This just lets them know that there's
 something that should be investigated.

 I think the checks framework suggestion is good, but wouldn’t help people
 detect if they had a poorly constructed callable in list_display since it
 would be the callable name in list_select_related and not a field name.

 Yep, I was assuming this feature would only work in DEBUG mode.  I don't
 think this is something most people would want to show up in production.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.2eb850d1a54e2bcbfb0f5483b380ae0c%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25701: Add warning to an admin list_view if too many queries are being used

[Django]

#25701: Add warning to an admin list_view if too many queries are being used
---+--
 Reporter:  jacinda|Owner:  jacinda
 Type:  New feature|   Status:  assigned
Component:  contrib.admin  |  Version:
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Unreviewed
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+--

Comment (by timgraham):

 After some thought, I'm a bit hesitant to duplicate functionality (to some
 extent) provided by django-debug-toolbar. I think there's consensus to
 recommend that package in the docs (perhaps in the tutorial). While this
 will require the developer to understand things at a slightly deeper
 level, it seems like a more teachable solution in the long run. What do
 you think?

 One partial solution would be to use the checks framework (at the INFO
 level, perhaps) to detect relations in `list_display` that aren't in
 `list_select_related`. Of course this won't cover O(n) queries in
 callables, but I'm not yet convinced a more sophisticated solution is
 worthwhile. I guess your proposal would only work in DEBUG mode (which
 makes sense to me since we probably wouldn't want end users to see the
 warning) since `connection.queries` is disabled otherwise.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.985cfa0b0692c256ec54e2ddff95cf98%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25701: Add warning to an admin list_view if too many queries are being used

[Django]

#25701: Add warning to an admin list_view if too many queries are being used
---+--
 Reporter:  jacinda|Owner:  jacinda
 Type:  New feature|   Status:  assigned
Component:  contrib.admin  |  Version:
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Unreviewed
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+--
Description changed by jacinda:

Old description:

> If you use a related field in a callable, and then use that callable in
> list_display, Django doesn't currently have a way to automatically detect
> that select_related should be used and performs a query for every row in
> the list.
>
> While it might be possible to do this automatically, a quick way to help
> with this problem would be to warn a developer with something like
> django.contrib.messages (if enabled) that a list_view is performing O(n)
> queries and that they should investigate this as a potential performance
> issue.

New description:

 If you use a related field in a callable on a Model or ModelAdmin, and
 then use that callable in list_display, Django doesn't currently have a
 way to automatically detect that select_related should be used and
 performs a query for every row in the list.

 While it might be possible to do this automatically, a quick way to help
 with this problem would be to warn a developer with something like
 django.contrib.messages (if enabled) that a list_view is performing O(n)
 queries and that they should investigate this as a potential performance
 issue.

--

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.34d195097e376d35d01bcb819b85d4e7%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25701: Add warning to an admin list_view if too many queries are being used

[Django]

#25701: Add warning to an admin list_view if too many queries are being used
---+--
 Reporter:  jacinda|Owner:  jacinda
 Type:  New feature|   Status:  assigned
Component:  contrib.admin  |  Version:
 Severity:  Normal |   Resolution:
 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 jacinda):

 * status:  new => assigned
 * cc: jacinda.shelly@… (added)
 * needs_better_patch:   => 0
 * needs_tests:   => 0
 * owner:  nobody => jacinda
 * needs_docs:   => 0


--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.6c408b73d6980e130c01096695905740%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.