Re: QuerySet.iterator together with prefetch_related because of chunk_size

2019-01-12 Thread charettes
Replying to my concerns about the P * C queries.

Throwing that idea out there but what if we required chunk_size to be 
explicitly specified when the queryset has prefetch lookups?

That would at least force the developers to consider how many prefetching 
queries iterating through the results would require. Plus since the 
argument was only recently introduced it is less likely to silently cause 
changes under developers feet.

Simon

Le vendredi 26 octobre 2018 20:12:02 UTC-4, charettes a écrit :
>
> Josh, I agree that silently not working is problematic but it has been 
> this way since prefetch_related() was introduced.
>
> Something to keep in mind as well is that silently turning it on would 
> also perform P * C extra queries where P is the number of prefetches 
> requested through prefetch_related() and C the number of chunks the results 
> contains. This is non negligible IMO.
>
> What I'd be in favor off is raising an exception on 
> prefetch_related(*prefetches).iterator() in the next release at least to 
> allow users using this pattern to adjust their code and then ship the 
> optimization with all the warnings related to the interactions between 
> prefetch_related(*prefetches) and iterator(chunk_size) in the next one.
>
> I'm not completely completely against skipping the exception release phase 
> entirely given there might be users out there accessing attributes expected 
> to be prefetched on iterated instances and inadvertently performing tons of 
> queries but the exception phase just feels safer given iterator() is 
> usually used in memory constrained contexts.
>
> Simon
>
> Le vendredi 26 octobre 2018 19:27:55 UTC-4, Josh Smeaton a écrit :
>>
>> I tend to agree with Tobi. Prefetching silently not working on iterator 
>> can be quite confusing, unless you have a good understanding of both APIs. 
>> It might be possible to do what you're asking, but it'd mean that django is 
>> now actually caching the result when it explicitly says it isn't - even if 
>> the result is a much smaller moving cache. Prefetching chunk_size results 
>> per chunk is unlikely to make a material difference to memory usage. Users 
>> are usually concerned about the entire result set of the primary table.
>>
>> I don't know if you can change the API to make these suggested changes 
>> without also impacting how we iterate over result sets - but I'd be 
>> interested in seeing a proof of concept at the very least.
>>
>>
>>
>> On Monday, 15 October 2018 20:41:13 UTC+11, tobias@truffls.com wrote:
>>>
>>> Thank you for your feedback. I would like to answer some statements to 
>>> either convince you or make it more clear, where my idea stems from:
>>>
>>> The fundamental reason why iterator() cannot be used with 
 prefetch_related() is that the latter requires a set of model instance to 
 be materialized to work appropriately which chunk_size doesn't control at 
 all.
 In other words chunk_size only controls how many rows should be fetched 
 from the database cursor and kept into memory at a time. Even when this 
 parameter is used, iterator() will only materialize a single model 
 instance 
 per yield.

>>>  
>>> It should be easily possible to change the involved code of 
>>> ModelIterable to materialize the retrieved rows in batches. After 
>>> materializing the batch / chunk, it could do the prefetching.
>>>  
>>>
 Given that iterator() always ignored prefetch related lookups instead 
 of erroring out when they were specified make me feel like turning such a 
 feature on by default could be problematic as it could balloon the memory 
 usage which is the main reason why iterator is useful anyway.

>>>
>>> I would argue, that users who thoughtlessly applied prefetching together 
>>> with iterator now actually get, what they thought of: less DB query round 
>>> trips traded against a little more memory usage.
>>>
>>> Best,
>>> Tobi
>>>
>>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/1f5d8b5a-8c7c-4575-b904-adfaa9d1900d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Fellow Reports - January 2019

2019-01-12 Thread Tim Graham


Week ending January 12, 2019

Triaged

---

https://code.djangoproject.com/ticket/30075 - Better error message for "App 
'bar.foo' could not be found. Is it in INSTALLED_APPS?" (fixed)

https://code.djangoproject.com/ticket/30084 - Setting 
DATABASES['default']['TEST']['engine'] to SQLite does not cause Django to 
use an in-memory database as expected (invalid)

https://code.djangoproject.com/ticket/30083 - Model instance state not 
correctly set when initializing a model with Model.from_db() (accepted)

https://code.djangoproject.com/ticket/30086 - Document how floatformat 
template filter interacts with the localize template tag (accepted)

Reviewed/committed

--

https://github.com/django/django/pull/10825 - Refs #23748 -- Added 
AutoField introspection for SQLite.

https://github.com/django/django/pull/10813 - Fixed #30071 -- Fixed error 
message when a 'default' database isn't provided.

https://github.com/django/django/pull/9174 - Fixed #28658 -- Added DISTINCT 
handling to the Aggregate class.

https://github.com/django/django/pull/10751 - Fixed #30037 -- Added request 
arg to RemoteUserBackend.configure_user().

https://github.com/django/django/pull/9543 - Refs #28643 -- Added NullIf 
database function.

https://github.com/django/django/pull/10175 - Fixed #28478 -- Make 
DiscoverRunner skip creating unused test databases.

https://github.com/django/django/pull/10835 - Fixed #23829 -- Made 
ping_google command/function use https for the sitemap URL.

https://github.com/django/django/pull/10673 - Fixed #29738 --  Allowed 
registering serializers with MigrationWriter.

https://github.com/django/django/pull/10833 -  Fixed #30097 -- Made 'obj' 
arg of InlineModelAdmin.has_add_permission() optional.

https://github.com/django/django/pull/10796 - Fixed #30062 -- Added support 
for unique conditional constraints.

https://github.com/django/django/pull/10827 - Refs #28643 -- Added Reverse 
database function.

https://github.com/django/django/pull/10838 - Fixed #30057 -- Fixed 
diffsettings ignoring custom configured settings.
https://github.com/django/django/pull/8819 - Fixed #27685 -- Added watchman 
support to the autoreloader.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/4160ed22-a68e-482f-aba4-0c9f308a3bd6%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Request to reconsider ticket #27910: using an Enum class in model Field choices

2019-01-12 Thread Tom Forbes
While I agree that Enum’s are a bit clunky (and IMO removing in is a poor
choice), is this going to really take be that hard to work with or that
difficult to validate?

Enums are types that raise ValueError, like any other, so it’s not that
confusing and fits in with Django’s existing validation code. Once we have
coerced the input into an enum member then validation against choices is
simple no?




On 11 January 2019 at 13:30:40, James Bennett (ubernost...@gmail.com) wrote:

Shai, your rebuttal still misses an important use case, which is
containment.

To continue with your example of a 'SchoolYear(str, Enum)', the following
will both be False:

'FR' in SchoolYear
'FR' in SchoolYear.__members__

The first of those is also becoming illegal soon -- attempting an 'in'
comparison on an Enum, using an operand that isn't an instance of Enum,
will become a TypeError in Python 3.8.

Instead you have to do something like:

'FR' in [choice.value for choice in SchoolYear]

to get a containment check. And you need a containment check to perform
validation.

There are other ways to do it, but they're all clunky, and this is going to
be code that people have to write (in some form) over and over again,
unless we build our own choice abstraction that hides this by wrapping an
Enum and implementing a __contains__ that does what people want. And you'd
basically have to do it in a wrapper, because Enum does metaclass stuff
that interferes with a typical "just subclass and override" approach.

So I still don't think this is going to work for the model-choice use case
without a lot of fiddling (and I'm still not a fan of the enum module in
general, largely because it seems to have gone out of its way to make this
use case difficult to implement).
--
You received this message because you are subscribed to the Google Groups
"Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-developers/CAL13Cg_KEtgmNzRo%3DC8cnCiNsukr5YOCTv3MF8AybV%3D%3DiUXP0g%40mail.gmail.com

.
For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAFNZOJOSdesTHqak%3Dstj%2Be65YBYOPg2PiZD7iMpoAH2kSpa%2Bgw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: Can we make HTTP 308 the default for CommonMiddleware / APPEND_SLASH?

2019-01-12 Thread Tobias Wiese
Hello,

On 1/11/19 6:42 PM, George-Cristian Bîrzan wrote:
> the only thing that I can think of is whether HSTS preload will
> support anything except 301.

All my pages redirect via a 308 from the http to the https version.
And most of them are HSTS preloaded. So no problem here.

-- 
Tobias Wiese

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/4fe097e2-0f41-e4f7-8075-1c7acac8933c%40tobiaswiese.com.
For more options, visit https://groups.google.com/d/optout.