Re: [Django] #33024: filter_horizontal doesn't use full height when in collapsed fieldset

2021-08-14 Thread Django
#33024: filter_horizontal doesn't use full height when in collapsed fieldset
-+-
 Reporter:  Peter Tillema|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  contrib.admin|  Version:  3.2
 Severity:  Normal   |   Resolution:
 Keywords:  filter_horizontal,   | Triage Stage:
  fieldsets, admin   |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  1
-+-
Changes (by Peter Tillema):

 * type:  Uncategorized => Bug


-- 
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/070.c58539b8126b41b8fb20e37891cc889f%40djangoproject.com.


Re: [Django] #33018: Incorrect annotation value when doing a subquery with empty queryset

2021-08-14 Thread Django
#33018: Incorrect annotation value when doing a subquery with empty queryset
-+-
 Reporter:  decomorreno  |Owner:  David
 |  Wobrock
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  3.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  orm, annotate,   | Triage Stage:  Accepted
  EmptyQuerySet, empty, count|
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by David Wobrock):

 * cc: David Wobrock (added)
 * owner:  nobody => David Wobrock
 * has_patch:  0 => 1
 * status:  new => assigned


Comment:

 Hi!

 Thanks for the hints Simon, I tried a first patch where we catch empty
 values here https://github.com/django/django/pull/14770

 Should we expand the solution a bit further and rename
 `empty_aggregate_value` as you suggested, and use it in the `SQLCompiler`
 too?

-- 
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/069.dbadd5aedf73a36b1a089d239cec2528%40djangoproject.com.


Re: [Django] #33025: Avoid accessing ConnectionsHandler.__getitem__ in Query.build_lookup unless necessary, for performance.

2021-08-14 Thread Django
#33025: Avoid accessing ConnectionsHandler.__getitem__ in Query.build_lookup 
unless
necessary, for performance.
-+-
 Reporter:  Keryn Knight |Owner:  Keryn
 Type:   |  Knight
  Cleanup/optimization   |   Status:  assigned
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by David Smith):

 * stage:  Unreviewed => Accepted


Comment:

 Yes asgiref's `Local` is "slow" see #32312, although I'm not sure what's
 possible to improve its performance.

-- 
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/067.b3105d6ca3b38c6705ace400cfa679c4%40djangoproject.com.


Re: [Django] #28919: Add support for Common Table Expression (CTE) queries

2021-08-14 Thread Django
#28919: Add support for Common Table Expression (CTE) queries
-+-
 Reporter:  Daniel Miller|Owner:  Moses
 |  Mugisha
 Type:  New feature  |   Status:  assigned
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  QuerySet.extra   | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by crypted):

 * cc: crypted (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/067.c01805420589a4856afec101bddad02b%40djangoproject.com.


Re: [Django] #31503: Moving a unique constraint from unique_together to Field.unique generate an invalid migration.

2021-08-14 Thread Django
#31503: Moving a unique constraint from unique_together to Field.unique 
generate an
invalid migration.
-+-
 Reporter:  Xiang Wang   |Owner:  David
 |  Wobrock
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  3.0
 Severity:  Normal   |   Resolution:
 Keywords:  unique_together  | Triage Stage:  Accepted
  unique migrations  |
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by David Wobrock):

 * needs_better_patch:  1 => 0


Comment:

 Removing "patch needs improvement" to get some eyes to look at 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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/064.afbe9e2efca07652908d88d3206d1c99%40djangoproject.com.


[Django] #33026: The Testing Tools docs should highlight RequestFactory

2021-08-14 Thread Django
#33026: The Testing Tools docs should highlight RequestFactory
-+
   Reporter:  Roy Smith  |  Owner:  nobody
   Type:  Uncategorized  | Status:  new
  Component:  Documentation  |Version:  3.2
   Severity:  Normal |   Keywords:
   Triage Stage:  Unreviewed |  Has patch:  0
Needs documentation:  0  |Needs tests:  0
Patch needs improvement:  0  |  Easy pickings:  0
  UI/UX:  0  |
-+
 I've used django.test.Client for several large projects over the years.
 Recently, I got stuck because I couldn't get session management to work in
 my tests.  Judging from the number of StackOverflow, etc, posts on this
 topic, this is a common issue.  Eventually I found
 https://stackoverflow.com/questions/25132621/ which put me onto using
 RequestFactory instead of the Test Client.  I'm sure I'd seen
 RequestFactory mentioned before, but never explored it.

 I think RequestFactory should be emphasized more in
 https://docs.djangoproject.com/en/3.1/topics/testing/tools/.  Right now,
 the only mention is buried deep down on the page in a "See also" box which
 links to the "Advanced testing topics".  Labeling it as an "advanced"
 topic is misleading.  ​May I suggest that it should be at least briefly
 covered in the introductory section. There's already a couple of
 paragraphs contrasting django.test.client to Selenium.  It would be good
 to say something like:

 A lower-level alternative to the test client is RequestFactory, which lets
 you bypass the routing and middleware layers.  Use RequestFactory to write
 unit tests of individual view functions and methods.  Use the test client
 to write integration tests of how your views work in the context of your
 full application.  Use Selenium to test rendered HTML and the behavior of
 Web pages, namely JavaScript functionality.

 I'd be happy to write up an improved doc if you think this is a reasonable
 way to go.

-- 
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/051.569783be3947d9207625243a8667717e%40djangoproject.com.


Re: [Django] #33025: Avoid accessing ConnectionsHandler.__getitem__ in Query.build_lookup unless necessary, for performance.

2021-08-14 Thread Django
#33025: Avoid accessing ConnectionsHandler.__getitem__ in Query.build_lookup 
unless
necessary, for performance.
-+-
 Reporter:  Keryn Knight |Owner:  Keryn
 Type:   |  Knight
  Cleanup/optimization   |   Status:  assigned
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Keryn Knight):

 * needs_better_patch:  0 => 1


Comment:

 PR is https://github.com/django/django/pull/14769
 So far, a linter is telling me off ... because of course it is. I hate it.
 And 3 CI jobs died ''immediately'' (shockingly quickly, never even saw
 them go to pending) having failed to do a git checkout?

 I'm going to hope that at least some of the others pass...

-- 
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/067.2d7c48bc0d4195b2caf3e93b70136aa3%40djangoproject.com.


Re: [Django] #33014: ProjectState.__init__() can assume its real_apps argument is a set

2021-08-14 Thread Django
#33014: ProjectState.__init__() can assume its real_apps argument is a set
-+-
 Reporter:  Chris Jerdonek   |Owner:  Chris
 Type:   |  Jerdonek
  Cleanup/optimization   |   Status:  assigned
Component:  Migrations   |  Version:  dev
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-
Changes (by David Smith):

 * owner:  nobody => Chris Jerdonek
 * status:  new => assigned


-- 
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/067.60a3311131426460dd14c84e50267bc1%40djangoproject.com.


[Django] #33025: Avoid accessing ConnectionsHandler.__getitem__ in Query.build_lookup unless necessary, for performance.

2021-08-14 Thread Django
#33025: Avoid accessing ConnectionsHandler.__getitem__ in Query.build_lookup 
unless
necessary, for performance.
-+-
   Reporter:  Keryn  |  Owner:  Keryn Knight
  Knight |
   Type: | Status:  assigned
  Cleanup/optimization   |
  Component:  Database   |Version:  dev
  layer (models, ORM)|
   Severity:  Normal |   Keywords:
   Triage Stage: |  Has patch:  1
  Unreviewed |
Needs documentation:  0  |Needs tests:  0
Patch needs improvement:  0  |  Easy pickings:  0
  UI/UX:  0  |
-+-
 Currently, `build_lookup` contains:
 {{{
 # For Oracle '' is equivalent to null. The check must be done at this
 # stage because join promotion can't be done in the compiler. Using
 # DEFAULT_DB_ALIAS isn't nice but it's the best that can be done here.
 # A similar thing is done in is_nullable(), too.
 if
 (connections[DEFAULT_DB_ALIAS].features.interprets_empty_strings_as_nulls
 and
 lookup_name == 'exact' and lookup.rhs == ''):
 return lhs.get_lookup('isnull')(lhs, True)
 }}}
 but going through `connections` requires accessing the thread critical
 `asgiref.Local` which is only strictly necessary if the cheap comparisons
 are truthy and can be short-circuited like so:
 {{{
 if (lookup_name == 'exact' and lookup.rhs == '' and
 connections[DEFAULT_DB_ALIAS].features.interprets_empty_strings_as_nulls):
 return lhs.get_lookup('isnull')(lhs, True)
 }}}

 This is coming off the back of the report in #33015, which is a nice
 pathological case we can use:
 {{{
 In [1]: from django.db.models import Q
 In [2]: from testapp.models import ExampleModel
 In [3]: items_to_fetch = [(i, j) for i in range(0,100) for j in
 range(0,100)]
 In [4]: query = Q()
...: for v1, v2 in items_to_fetch:
...: query |= Q(val1=v1, val2=v2)
 %timeit x = ExampleModel.objects.filter(query)
 644 ms ± 6.26 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
 }}}
 If we cProfile that `ExampleModel.objects.filter(query)` we get something
 like:
 {{{
ncalls  tottime  percall  cumtime  percall filename:lineno(function)
   30001/10.1780.0001.2681.268 query.py:1221(build_filter)
 40.1420.0000.1570.000 query.py:1457(names_to_path)
   10002/10.0770.0001.2681.268 query.py:1386(_add_q)
 20.0460.0000.3790.000 query.py:1156(build_lookup)
 }}}
 and if we do `%lprun -f Query.build_filter -f Query.build_lookup
 ExampleModel.objects.filter(query)` on it we'll get (snipped for brevity):
 {{{
 Function: build_filter at line 1221
 Line #  Hits Time  Per Hit   % Time  Line Contents
 ==
   1329 2 651989.0 32.6 24.9  condition =
 self.build_lookup(lookups, col, value)

 Function: build_lookup at line 1156
 Line #  Hits Time  Per Hit   % Time  Line Contents
 ==
   1195 2 200730.0 10.0 38.1  if
 (connections[DEFAULT_DB_ALIAS].features.interprets_empty_strings_as_nulls
 and
   1196
 lookup_name == 'exact' and lookup.rhs == ''):
   1197   return
 lhs.get_lookup('isnull')(lhs, True
 }}}
 Because I'm using sqlite, that 38% of time is essentially free to reclaim
 by switching the if condition as above, in which case we get cProfile
 output like so:
 {{{
ncalls  tottime  percall  cumtime  percall filename:lineno(function)
   30001/10.1710.0001.0821.082 query.py:1220(build_filter)
 40.1450.0000.1590.000 query.py:1456(names_to_path)
   10002/10.0760.0001.0821.082 query.py:1385(_add_q)
 20.0420.0000.1770.000 query.py:1559(setup_joins)
2200050.0390.0000.0640.000 {built-in method
 builtins.isinstance}
 20.0370.0000.2230.000 query.py:1156(build_lookup)
 }}}
 and line profile information like so:
 {{{
 Function: build_filter at line 1220
 Line #  Hits Time  Per Hit   % Time  Line Contents
 ==
   1328 2 447682.0 22.4 18.9  condition =
 self.build_lookup(lookups, col, value)

 Function: build_lookup at line 1156
 Line #  Hits Time  Per Hit   % Time  Line Contents
 ==
   1195 2  12744.0  0.6  3.9  if (lookup_name
 == 'exact' and lookup.rhs == '' and
   1196
 connections[DEFAULT_DB_ALIAS].features.interprets_empty_strings_as_nulls):
   

Re: [Django] #32975: ModelAdmin for proxy model with InlineModelAdmin for proxy superclass reference results in admin.E202

2021-08-14 Thread Django
#32975: ModelAdmin for proxy model with InlineModelAdmin for proxy superclass
reference results in admin.E202
-+-
 Reporter:  Lucas Weyne  |Owner:  (none)
 Type:  Bug  |   Status:  new
Component:  contrib.admin|  Version:  3.1
 Severity:  Normal   |   Resolution:
 Keywords:  proxy,   | Triage Stage:  Accepted
  InlineModelAdmin, E202 |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Bal Krishna Jha):

 * owner:  Bal Krishna Jha => (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/068.4ebbab33825d5d6a39536b099ff1cdee%40djangoproject.com.


Re: [Django] #28357: Prepopulated_fields doesn't work for admin.StackedInline.

2021-08-14 Thread Django
#28357: Prepopulated_fields doesn't work for admin.StackedInline.
-+-
 Reporter:  ChristosKon  |Owner:  (none)
 Type:  Bug  |   Status:  new
Component:  contrib.admin|  Version:  dev
 Severity:  Normal   |   Resolution:
 Keywords:  admin,   | Triage Stage:  Accepted
  stackedinline, |
  prepopulated_fields|
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  1|UI/UX:  1
-+-
Changes (by David Smith):

 * owner:  Jakob Köhler => (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/069.e045ca12a0d116b9adbd80bbc52b50fe%40djangoproject.com.


Re: [Django] #6135: Introduce a short-cut for template filters that has needs_autoescape = True

2021-08-14 Thread Django
#6135: Introduce a short-cut for template filters that has needs_autoescape = 
True
-+
 Reporter:  anonymous|Owner:  (none)
 Type:  New feature  |   Status:  new
Component:  Template system  |  Version:  dev
 Severity:  Normal   |   Resolution:
 Keywords:  autoescape   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  1
  Needs tests:  1|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+
Changes (by David Smith):

 * owner:  AbhigyaShridhar => (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/067.9cbcc6a1eb6fe7c96365dbb338ae74b1%40djangoproject.com.


Re: [Django] #32964: Change "setup" to "set up".

2021-08-14 Thread Django
#32964: Change "setup" to "set up".
--+
 Reporter:  Nat S Dunn|Owner:  (none)
 Type:  Cleanup/optimization  |   Status:  new
Component:  Documentation |  Version:  dev
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  1 |UI/UX:  0
--+
Changes (by David Smith):

 * owner:  Mrugakshi Thakare => (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/066.0ce30617e29b478af44a23ca8cfbc599%40djangoproject.com.


[Django] #33024: filter_horizontal doesn't use full height when in collapsed fieldset

2021-08-14 Thread Django
#33024: filter_horizontal doesn't use full height when in collapsed fieldset
-+-
   Reporter:  Peter  |  Owner:  nobody
  Tillema|
   Type: | Status:  new
  Uncategorized  |
  Component: |Version:  3.2
  contrib.admin  |   Keywords:  filter_horizontal,
   Severity:  Normal |  fieldsets, admin
   Triage Stage: |  Has patch:  0
  Unreviewed |
Needs documentation:  0  |Needs tests:  0
Patch needs improvement:  0  |  Easy pickings:  0
  UI/UX:  1  |
-+-
 Say you have a field `field` in model `X`, which is a `ManyToMany` field.
 You can use `filter_horizontal` in the `ModelAdmin` to display the
 available and selected options. However, if you use `filter_horizontal` in
 combination with a fieldset which is collapsed by default, it won't take
 the full height.

 Example:

 {{{
 #!python
 @admin.register(X)
 class XAdmin(admin.ModelAdmin):
 list_display = ('a', 'b', 'c', 'field')
 filter_horizontal = ('field',)

 fieldsets = (
 (None, {
 'fields': ('a', 'b', 'c')
 }),
 ('Other', {
 'classes': ('collapse',),
 'fields': ('field',)
 })
 )
 }}}

 which gets rendered as https://i.imgur.com/jZpUHYN.png

-- 
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/055.f8e69fec5a1b38b927e05de369bc0b22%40djangoproject.com.


Re: [Django] #31354: HttpRequest.get_host() doesn't include the port from META['HTTP_X_FORWARDED_PORT'].

2021-08-14 Thread Django
#31354: HttpRequest.get_host() doesn't include the port from
META['HTTP_X_FORWARDED_PORT'].
-+-
 Reporter:  dgcgh|Owner:  (none)
 Type:  Bug  |   Status:  new
Component:  HTTP handling|  Version:  dev
 Severity:  Normal   |   Resolution:
 Keywords:  request get_host | Triage Stage:  Accepted
  build_absolute_uri |
Has patch:  1|  Needs documentation:  1
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  1|UI/UX:  0
-+-
Changes (by David Smith):

 * owner:  dgcgh => (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/063.bb5f1b49eced4743612e253936d1f38f%40djangoproject.com.


Re: [Django] #33015: QuerySet.extra Use Case: filtering on large lists of tuples

2021-08-14 Thread Django
#33015: QuerySet.extra Use Case: filtering on large lists of tuples
-+-
 Reporter:  kris-swann   |Owner:  nobody
 Type:  Uncategorized|   Status:  new
Component:  Database layer   |  Version:  2.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  QuerySet.extra   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Keryn Knight):

 * cc: Keryn Knight (added)


Comment:

 Having been bitten by this same thing multiple times over the years, I'm
 interested.

 The performance profile of this is reproducible for me on the `2.2.9` and
 `3.2.6` tags:
 {{{
 In [5]: %%timeit
...: query = Q()
...: for v1, v2 in items_to_fetch:
...: query |= Q(val1=v1, val2=v2)
 21.9 s ± 1.01 s per loop (mean ± std. dev. of 7 runs, 1 loop each)
 In [7]: %timeit -r3 -n1 x = ExampleModel.objects.filter(query)
 48.2 s ± 1.54 s per loop (mean ± std. dev. of 3 runs, 1 loop each)
 }}}
 but doesn't appear to be a ''big'' issue on `main` since #32940; my
 suspicion is that both `WhereNode` and `Q` were hitting the pathological
 case for `data in self.children` being O(n), in case that rings a bell
 Simon; below is main as of `8208381ba6`; the problem was present at
 `501a8db465` and basically gone at `ff661dbd50`:
 {{{
 In [5]: %%timeit
...: query = Q()
...: for v1, v2 in items_to_fetch:
...: query |= Q(val1=v1, val2=v2)
 209 ms ± 1.93 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
 In [7]: %timeit x = ExampleModel.objects.filter(query)
 644 ms ± 22.2 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
 }}}
 Though it's still faster to avoid combining Q objects and kwargs:
 {{{
 In [14]: %timeit Q(*(Q(('val1', v1), ('val2', v2)) for v1, v2 in
 items_to_fetch), _connector=Q.OR)
 15.1 ms ± 761 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
 }}}
 That's true for `2.2.9` too, but regrettably has less impact on the query
 compilation itself:
 {{{
 In [4]: %timeit Q(*(Q(('val1', v1), ('val2', v2)) for v1, v2 in
 items_to_fetch), _connector=Q.OR)
 22.9 ms ± 12.7 ms per loop (mean ± std. dev. of 7 runs, 100 loops each)
 In [5]: %timeit -r3 -n1 x = ExampleModel.objects.filter(query)
 20.6 s ± 874 ms per loop (mean ± std. dev. of 3 runs, 1 loop each)
 }}}

 I don't know that the simple Q based version will ever be as fast as
 `extra` (as used above) or the `ExpressionTuple` (also above), but it
 begins to look ''almost'' acceptable, certainly compared to the previous.

-- 
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/068.7d3d0b1a2d3d23558ba619a9aa3218a8%40djangoproject.com.


Re: [Django] #33023: InMemoryUploadedFile - opening with context manager multiple time is throwing error

2021-08-14 Thread Django
#33023: InMemoryUploadedFile - opening with context manager multiple time is
throwing error
+--
 Reporter:  Harsh Bhikadia  |Owner:  nobody
 Type:  Bug |   Status:  new
Component:  Core (Other)|  Version:  3.2
 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 Harsh Bhikadia):

 * status:  closed => new
 * resolution:  fixed =>


-- 
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/063.2ac7753adefa6c79b453f78003117a64%40djangoproject.com.


Re: [Django] #33023: InMemoryUploadedFile - opening with context manager multiple time is throwing error

2021-08-14 Thread Django
#33023: InMemoryUploadedFile - opening with context manager multiple time is
throwing error
+--
 Reporter:  Harsh Bhikadia  |Owner:  nobody
 Type:  Bug |   Status:  closed
Component:  Core (Other)|  Version:  3.2
 Severity:  Normal  |   Resolution:  fixed
 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 Harsh Bhikadia):

 Also realised the implementation is not respecting the `mode` passed in
 arg when calling `open`.

 The current implementation (`master` branch as of now) looks like the
 following:

 {{{
 class InMemoryUploadedFile(UploadedFile):
 """
 A file uploaded into memory (i.e. stream-to-memory).
 """
 def __init__(self, file, field_name, name, content_type, size,
 charset, content_type_extra=None):
 super().__init__(file, name, content_type, size, charset,
 content_type_extra)
 self.field_name = field_name

 def open(self, mode=None):
 self.file.seek(0)
 return self

 def chunks(self, chunk_size=None):
 self.file.seek(0)
 yield self.read()

 def multiple_chunks(self, chunk_size=None):
 # Since it's in memory, we'll never have multiple chunks.
 return False
 }}}

-- 
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/063.5c8d42f0a79504c4d96e24c83abac91d%40djangoproject.com.


Re: [Django] #33023: InMemoryUploadedFile - opening with context manager multiple time is throwing error

2021-08-14 Thread Django
#33023: InMemoryUploadedFile - opening with context manager multiple time is
throwing error
+--
 Reporter:  Harsh Bhikadia  |Owner:  nobody
 Type:  Bug |   Status:  closed
Component:  Core (Other)|  Version:  3.2
 Severity:  Normal  |   Resolution:  fixed
 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 Harsh Bhikadia):

 * status:  new => closed
 * resolution:   => fixed


Old description:

> **What is the issue:**
> when "opening" the "uploaded file" with "context manager (like `with
> file.open('rb') as f:`) multiple times it is throwing `` exception
>
> **Explaination**
> Digging into the code, I realised that `InMemoryUploadedFile` overrides
> the `open` method from `File` instance - the implementation just "seeks
> to 0". Since the context manager will call the `close` method on "exit".
> When "opening" the file again (with or without context manager) it fails
> with "cannot seek(0) of already closed file"
>

> ** Potential solution**
> - Not sure why the "open" method was overridden - if not required then
> just should remove the "overridden method"
> - or making sure the file is "re-opened" (like in `File`) if closed
>

> **To conclude**
>
> Since
> [https://docs.djangoproject.com/en/3.2/ref/files/file/#django.core.files.File.open
> the doc on `File`] mentions that it could be used with "context manager"
> - I think this is unexpected behaviour and should be fixed.
>
> I have been using Django for many years but new to contribution/issue
> reporting, so forgive me when I am being ignorant.
>
> I am interested in fixing this myself if someone here gives me a green
> signal for it.

New description:

 **What is the issue:**
 when "opening" the "uploaded file" with "context manager (like `with
 file.open('rb') as f:`) multiple times it is throwing `ValueError: I/O
 operation on closed file.` exception

 **Explaination**
 Digging into the code, I realised that `InMemoryUploadedFile` overrides
 the `open` method from `File` instance - the implementation just "seeks to
 0". Since the context manager will call the `close` method on "exit". When
 "opening" the file again (with or without context manager) it fails with
 "cannot seek(0) of already closed file"


 ** Potential solution**
 - Not sure why the "open" method was overridden - if not required then
 just should remove the "overridden method"
 - or making sure the file is "re-opened" (like in `File`) if closed


 **To conclude**

 Since
 
[https://docs.djangoproject.com/en/3.2/ref/files/file/#django.core.files.File.open
 the doc on `File`] mentions that it could be used with "context manager"
 - I think this is unexpected behaviour and should be fixed.

 I have been using Django for many years but new to contribution/issue
 reporting, so forgive me when I am being ignorant.

 I am interested in fixing this myself if someone here gives me a green
 signal for 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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.c2673ce005605d768de05e7e0d6ea8d4%40djangoproject.com.


[Django] #33023: InMemoryUploadedFile - opening with context manager multiple time is throwing error

2021-08-14 Thread Django
#33023: InMemoryUploadedFile - opening with context manager multiple time is
throwing error
--+
   Reporter:  Harsh Bhikadia  |  Owner:  nobody
   Type:  Bug | Status:  new
  Component:  Core (Other)|Version:  3.2
   Severity:  Normal  |   Keywords:
   Triage Stage:  Unreviewed  |  Has patch:  0
Needs documentation:  0   |Needs tests:  0
Patch needs improvement:  0   |  Easy pickings:  0
  UI/UX:  0   |
--+
 **What is the issue:**
 when "opening" the "uploaded file" with "context manager (like `with
 file.open('rb') as f:`) multiple times it is throwing `` exception

 **Explaination**
 Digging into the code, I realised that `InMemoryUploadedFile` overrides
 the `open` method from `File` instance - the implementation just "seeks to
 0". Since the context manager will call the `close` method on "exit". When
 "opening" the file again (with or without context manager) it fails with
 "cannot seek(0) of already closed file"


 ** Potential solution**
 - Not sure why the "open" method was overridden - if not required then
 just should remove the "overridden method"
 - or making sure the file is "re-opened" (like in `File`) if closed


 **To conclude**

 Since
 
[https://docs.djangoproject.com/en/3.2/ref/files/file/#django.core.files.File.open
 the doc on `File`] mentions that it could be used with "context manager"
 - I think this is unexpected behaviour and should be fixed.

 I have been using Django for many years but new to contribution/issue
 reporting, so forgive me when I am being ignorant.

 I am interested in fixing this myself if someone here gives me a green
 signal for 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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/048.b31d7493d2ac495225eefe40c2a4907b%40djangoproject.com.