Technical Board statement on type hints for Django

2020-04-14 Thread Carlton Gibson
Hi all. 

The question of using typing, or type hints, or type checking, in (or with) 
Django
has come up several times. Whether we would add inline annotations, or use 
stub
files, or what? 

Most recently, this resulted in a draft DEP[0] to try and formalize the 
situation,
followed by a PR[1] adding the `__class_getitem__()` to `QuerySet` and 
`Manager`
that would ease the job of working on external stub files. 

[0]: https://github.com/django/deps/pull/65
[1]: https://github.com/django/django/pull/12405

There was extended discussion on both the draft DEP and the PR, which I
recommend for anyone particularly interested in this topic. 

Given the lack of consensus on the issue, I asked the Technical Board to 
review 
the situation in full, and make a judgement on the way forward.

Having been charged to do so, I here post the Technical Board's response:  


It is the view of the Django Technical Board that inline type 
annotations
should not be added to Django at the current time. 

A brief idea of the considerations are these:

* Typing in Python is still young and in flux. As such it is not
  sufficiently stable.  
* There are competing technologies, which may settle with time, but at 
this
  moment Django is not in a position to favor any one.
* Writing correct type hints is hard. And reviewing them to ensure
  correctness isn't any easier. Even ostensibly simple examples often 
have
  hidden complexity. Thus the barrier to contribution and maintenance is
  raised. 
* Due to the very dynamic nature of much of Django, example type hints 
very 
  often do not pass an acceptable standard for readability. This might 
be
  manageable with extensive aliasing but that effort itself involves
  further overhead. 
  
The Technical Board acknowledges the type checking is gaining ground in
Python and that many users are keen to employ it.

The django-stubs project is notable for the impressive work that it has
done adding external stub files for Django.

It is the view of the Technical Board that non-invasive changes in 
order to
assist the work of external projects such as django-stubs are 
acceptable.

In particular, this means that the pull request to add the
`__class_getitem__()` method to QuerySet and Manager should be accepted.

A small number of further changes of a similar nature may be accepted, 
but
the Technical Board wishes to emphasise that where possible typing 
should
be done in external stub files, and that the barrier for further inline
changes will be high.

We're looking forward to seeing how typing evolves in Python and
re-evaluating Django's position as things change.


On the basis of this, Mariusz and I (in our role as Mergers) shall review 
and
merge the PR #12405 in time for Django 3.1. (We will also close the draft 
DEP,
and any related tickets, over the coming period.)

Personally, I wish to thank everybody who has participated in the 
discussion to 
bring us to this point. 

Kind Regards,

Carlton

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/47283a09-7899-4449-8792-e31b5c779c4e%40googlegroups.com.


Re: Proposal to not implicitly create varchar/text-pattern opclass indexes on PostgreSQL

2020-04-14 Thread Hannes Ljungberg
Thanks for your reply Tim,

I also fail to see a "clean" upgrade path. The way I’ve been thinking of, 
is to just as you wrote, keep the code for deletion of the index so people 
who migrate to the Index-class will get it removed properly. Since it uses 
`IF EXISTS` it should be future-proof. We could add a system check that 
uses introspection to check if the `*_like` index is present on affected 
fields with `db_index=True` and warn about its deprecation. Then after a 
couple (?) of versions we could remove all traces of this index. 

The negative performance impact of removing this index for users of `LIKE` 
queries is very large but it’s hard to give numbers since it depends on the 
amount and nature of the data in the affected rows. Since we wouldn’t do 
any automatic removal of the index and add documentation about the 
importance of the opclass-indexes for `LIKE` queries I’m not that worried.

I realise that removing the creation of these indexes result in a not so 
pretty deprecation path but I think users of Django would benefit from 
having to be more explicit about their indexes and learn about the quirks 
of their database engines.


Den tisdag 14 april 2020 kl. 03:01:03 UTC+2 skrev Tim Graham:
>
> I have some sympathy for this issue as I'm trying to make the 
> createcachetable management command use SchemaEditor rather than some 
> custom SQL construction logic*. The related problem I ran into is that the 
> primary key column (a CharField) uses unique=True which means those 
> undesired opclasses indexes are created. I couldn't find a way to prevent 
> that index besides filtering it out of the list of SQL statements.
>
> As for your proposal, how would you handle the upgrade path for existing 
> projects? I imagine we could provide a script to run upon upgrade to remove 
> all such existing indexes. No doubt some users won't run it. Would you keep 
> around the code in Django's SchemaEditor like 
> https://github.com/django/django/blob/53d229ff632c2a3e547f2820a94239f38ba4d4ac/django/db/backends/postgresql/schema.py#L178-L180
>  
> that assumes those indexes exist?
>
> If Django stops creating those indexes, it could create a somewhat murky 
> situation for some developers as they try to figure out the state of 
> indexes in their database. It might depend on which version of Django was 
> in use when certain migrations ran.
>
> Some developers might be left debugging performance issues if those 
> indexes are removed. It could be helpful to gives some numbers as to the 
> possible performance impact on applications if these indexes are removed 
> without a developer realizing it.
>
> Third-party apps that need it could add an Index with opclasses but then 
> they'd face the issue of duplicate opclasses indexes if their app is used 
> on an older version of Django.
>
> * https://github.com/django/django/pull/12590
>
> On Sunday, April 12, 2020 at 10:30:50 AM UTC-4, Hannes Ljungberg wrote:
>>
>> Hi all, 
>>
>> I would like to continue the discussion started in this very old thread:
>> https://groups.google.com/d/msg/django-developers/H2QFcQYsbo8/RmRb-8FVypwJ
>>
>> I’m sorry if I should've continued the discussion in that thread but it 
>> felt a bit wrong to bring a 5 year old thread back to life :-)
>>
>> Anyway, as previously described in that thread the implicit creation of 
>> the `*_pattern_ops` index when creating indexes on `CharField` and 
>> `TextField` with `db_index=True` is not ideal. 
>>
>> In my experience `LIKE` expressions is not that common that it warrants 
>> to always create an index to cover this. 
>>
>> For very large tables this can become a problem where insertion/update 
>> performance is negatively affected and disc space usage is twice what 
>> really is needed if no `LIKE` queries are used.
>>
>> And even if one would like to use `LIKE` expressions it’s not obvious 
>> that the `*_pattern_ops` is the correct index. For leading wildcard patters 
>> as `LIKE %foo` one has to use a GIN/GiST index with the `*_trgm_ops` 
>> opclass. With the current implementation we would end up with 3 indexes 
>> when 2 would be sufficient for this use case (the regular b-tree and the 
>> trigram).
>>
>> One could also argue that we’re not consistent with these implicit 
>> indexes. The `iexact`/`icontains` lookups require an expression index on 
>> `(UPPER(col))`  but that’s not created.
>>
>> One important detail is that this implicit index is _not_ created when 
>> using the class based `Index` . In my opinion it’s not very clear that one 
>> needs to handle the creation of a `*_pattern_ops` index manually when using 
>> it.  
>>
>> This is the only documentation that I’ve been able to find about the 
>> creation of these implicit indexes: 
>> https://docs.djangoproject.com/en/dev/ref/databases/#indexes-for-varchar-and-text-columns
>>
>> My proposal is to remove the implicit index created by `db_index=True` 
>> and add documentation that one should use `Index.opclasses` to utilise 

Re: Proposal to not implicitly create varchar/text-pattern opclass indexes on PostgreSQL

2020-04-14 Thread Adam Johnson
For some numbers: I just checked on a long-running client project using
PostgreSQL, and it looks like several of the *_like  indexes are in use:

core=> select right(indexrelname, 10), idx_tup_read, idx_tup_fetch from
pg_stat_all_indexes where indexrelname like '%_like' order by 1;
   right| idx_tup_read | idx_tup_fetch
+--+---
 090f8_like |   168181 |168143
 0b77e_like |0 | 0
 0cc84_like |0 | 0
 1ab7c_like | 6260 |  6137
 32a5e_like |  2443202 |   2412585
 33678_like |0 | 0
 37b91_like |0 | 0
 6c02a_like |   15 |15
 75088_like |56746 |  9640
 90e0f_like | 8371 |  8089
 a08ec_like | 2815 |  2815
 b4ddf_like |0 | 0
(12 rows)

The first highest read index is accidental and the query will be rewritten.
The second-highest read index here though is from the third party app
django-oauth-toolkit:
https://github.com/jazzband/django-oauth-toolkit/blob/master/oauth2_provider/models.py#L284
.
The other indexes are on small tables and I'm not concerned about them from
a performance perspective.


Perhaps it's possible to use these index statistics in the system check to
prioritize which indexes are warned about.

On Tue, 14 Apr 2020 at 13:18, Hannes Ljungberg 
wrote:

> Thanks for your reply Tim,
>
> I also fail to see a "clean" upgrade path. The way I’ve been thinking of,
> is to just as you wrote, keep the code for deletion of the index so people
> who migrate to the Index-class will get it removed properly. Since it uses
> `IF EXISTS` it should be future-proof. We could add a system check that
> uses introspection to check if the `*_like` index is present on affected
> fields with `db_index=True` and warn about its deprecation. Then after a
> couple (?) of versions we could remove all traces of this index.
>
> The negative performance impact of removing this index for users of `LIKE`
> queries is very large but it’s hard to give numbers since it depends on the
> amount and nature of the data in the affected rows. Since we wouldn’t do
> any automatic removal of the index and add documentation about the
> importance of the opclass-indexes for `LIKE` queries I’m not that worried.
>
> I realise that removing the creation of these indexes result in a not so
> pretty deprecation path but I think users of Django would benefit from
> having to be more explicit about their indexes and learn about the quirks
> of their database engines.
>
>
> Den tisdag 14 april 2020 kl. 03:01:03 UTC+2 skrev Tim Graham:
>>
>> I have some sympathy for this issue as I'm trying to make the
>> createcachetable management command use SchemaEditor rather than some
>> custom SQL construction logic*. The related problem I ran into is that the
>> primary key column (a CharField) uses unique=True which means those
>> undesired opclasses indexes are created. I couldn't find a way to
>> prevent that index besides filtering it out of the list of SQL statements.
>>
>> As for your proposal, how would you handle the upgrade path for existing
>> projects? I imagine we could provide a script to run upon upgrade to remove
>> all such existing indexes. No doubt some users won't run it. Would you keep
>> around the code in Django's SchemaEditor like
>> https://github.com/django/django/blob/53d229ff632c2a3e547f2820a94239f38ba4d4ac/django/db/backends/postgresql/schema.py#L178-L180
>> that assumes those indexes exist?
>>
>> If Django stops creating those indexes, it could create a somewhat murky
>> situation for some developers as they try to figure out the state of
>> indexes in their database. It might depend on which version of Django was
>> in use when certain migrations ran.
>>
>> Some developers might be left debugging performance issues if those
>> indexes are removed. It could be helpful to gives some numbers as to the
>> possible performance impact on applications if these indexes are removed
>> without a developer realizing it.
>>
>> Third-party apps that need it could add an Index with opclasses but then
>> they'd face the issue of duplicate opclasses indexes if their app is used
>> on an older version of Django.
>>
>> * https://github.com/django/django/pull/12590
>>
>> On Sunday, April 12, 2020 at 10:30:50 AM UTC-4, Hannes Ljungberg wrote:
>>>
>>> Hi all,
>>>
>>> I would like to continue the discussion started in this very old thread:
>>>
>>> https://groups.google.com/d/msg/django-developers/H2QFcQYsbo8/RmRb-8FVypwJ
>>>
>>> I’m sorry if I should've continued the discussion in that thread but it
>>> felt a bit wrong to bring a 5 year old thread back to life :-)
>>>
>>> Anyway, as previously described in that thread the implicit creation of
>>> the `*_pattern_ops` index when creating indexes on `CharField` and
>>> `TextField` with `db_index=True` is not ideal.
>>>
>>> In my experience `LIKE` expr

Re: Proposal to not implicitly create varchar/text-pattern opclass indexes on PostgreSQL

2020-04-14 Thread charettes
Makes we wonder under which circumstances an OAuth token would need to be 
looked up using LIKE. I would expect the unique b-tree index which covers 
exact lookups to be sufficient.

Simon

Le mardi 14 avril 2020 09:01:48 UTC-4, Adam Johnson a écrit :
>
> For some numbers: I just checked on a long-running client project using 
> PostgreSQL, and it looks like several of the *_like  indexes are in use:
>
> core=> select right(indexrelname, 10), idx_tup_read, idx_tup_fetch from 
> pg_stat_all_indexes where indexrelname like '%_like' order by 1;
>right| idx_tup_read | idx_tup_fetch
> +--+---
>  090f8_like |   168181 |168143
>  0b77e_like |0 | 0
>  0cc84_like |0 | 0
>  1ab7c_like | 6260 |  6137
>  32a5e_like |  2443202 |   2412585
>  33678_like |0 | 0
>  37b91_like |0 | 0
>  6c02a_like |   15 |15
>  75088_like |56746 |  9640
>  90e0f_like | 8371 |  8089
>  a08ec_like | 2815 |  2815
>  b4ddf_like |0 | 0
> (12 rows)
>
> The first highest read index is accidental and the query will be rewritten.
> The second-highest read index here though is from the third party app 
> django-oauth-toolkit: 
> https://github.com/jazzband/django-oauth-toolkit/blob/master/oauth2_provider/models.py#L284
>   
> .
> The other indexes are on small tables and I'm not concerned about them 
> from a performance perspective.
>
>
> Perhaps it's possible to use these index statistics in the system check to 
> prioritize which indexes are warned about.
>
> On Tue, 14 Apr 2020 at 13:18, Hannes Ljungberg  > wrote:
>
>> Thanks for your reply Tim,
>>
>> I also fail to see a "clean" upgrade path. The way I’ve been thinking of, 
>> is to just as you wrote, keep the code for deletion of the index so people 
>> who migrate to the Index-class will get it removed properly. Since it uses 
>> `IF EXISTS` it should be future-proof. We could add a system check that 
>> uses introspection to check if the `*_like` index is present on affected 
>> fields with `db_index=True` and warn about its deprecation. Then after a 
>> couple (?) of versions we could remove all traces of this index. 
>>
>> The negative performance impact of removing this index for users of 
>> `LIKE` queries is very large but it’s hard to give numbers since it depends 
>> on the amount and nature of the data in the affected rows. Since we 
>> wouldn’t do any automatic removal of the index and add documentation about 
>> the importance of the opclass-indexes for `LIKE` queries I’m not that 
>> worried.
>>
>> I realise that removing the creation of these indexes result in a not so 
>> pretty deprecation path but I think users of Django would benefit from 
>> having to be more explicit about their indexes and learn about the quirks 
>> of their database engines.
>>
>>
>> Den tisdag 14 april 2020 kl. 03:01:03 UTC+2 skrev Tim Graham:
>>>
>>> I have some sympathy for this issue as I'm trying to make the 
>>> createcachetable management command use SchemaEditor rather than some 
>>> custom SQL construction logic*. The related problem I ran into is that the 
>>> primary key column (a CharField) uses unique=True which means those 
>>> undesired opclasses indexes are created. I couldn't find a way to 
>>> prevent that index besides filtering it out of the list of SQL statements.
>>>
>>> As for your proposal, how would you handle the upgrade path for existing 
>>> projects? I imagine we could provide a script to run upon upgrade to remove 
>>> all such existing indexes. No doubt some users won't run it. Would you keep 
>>> around the code in Django's SchemaEditor like 
>>> https://github.com/django/django/blob/53d229ff632c2a3e547f2820a94239f38ba4d4ac/django/db/backends/postgresql/schema.py#L178-L180
>>>  
>>> that assumes those indexes exist?
>>>
>>> If Django stops creating those indexes, it could create a somewhat murky 
>>> situation for some developers as they try to figure out the state of 
>>> indexes in their database. It might depend on which version of Django was 
>>> in use when certain migrations ran.
>>>
>>> Some developers might be left debugging performance issues if those 
>>> indexes are removed. It could be helpful to gives some numbers as to the 
>>> possible performance impact on applications if these indexes are removed 
>>> without a developer realizing it.
>>>
>>> Third-party apps that need it could add an Index with opclasses but 
>>> then they'd face the issue of duplicate opclasses indexes if their app is 
>>> used on an older version of Django.
>>>
>>> * https://github.com/django/django/pull/12590
>>>
>>> On Sunday, April 12, 2020 at 10:30:50 AM UTC-4, Hannes Ljungberg wrote:

 Hi all, 

 I would like to continue the discussion started in this very old thread:

 https://groups.google.com/

Re: Proposal to not implicitly create varchar/text-pattern opclass indexes on PostgreSQL

2020-04-14 Thread Adam Johnson
Yes that one is odd. Looking more closely, django-oauth-toolkit doesn't
make any LIKE queries, and doesn't seem to ever have done so. It seems
rather that PostgreSQL is using the index to fulfill the unique constraint,
as the corresponding '_uniq' index is not being touched:

core=> select indexrelname, idx_tup_read, idx_tup_fetch from
pg_stat_all_indexes where indexrelname like
'oauth2_provider_accesstoken_token_%' order by 1;
  indexrelname   | idx_tup_read |
idx_tup_fetch
-+--+---
 oauth2_provider_accesstoken_token_8af090f8_like |   169875 |
 169835
 oauth2_provider_accesstoken_token_8af090f8_uniq |0 |
  0
(2 rows)

I haven't verified all the queries being made though.

May still be useful for any check added to Django.

On Tue, 14 Apr 2020 at 14:17, charettes  wrote:

> Makes we wonder under which circumstances an OAuth token would need to be
> looked up using LIKE. I would expect the unique b-tree index which covers
> exact lookups to be sufficient.
>
> Simon
>
> Le mardi 14 avril 2020 09:01:48 UTC-4, Adam Johnson a écrit :
>>
>> For some numbers: I just checked on a long-running client project using
>> PostgreSQL, and it looks like several of the *_like  indexes are in use:
>>
>> core=> select right(indexrelname, 10), idx_tup_read, idx_tup_fetch from
>> pg_stat_all_indexes where indexrelname like '%_like' order by 1;
>>right| idx_tup_read | idx_tup_fetch
>> +--+---
>>  090f8_like |   168181 |168143
>>  0b77e_like |0 | 0
>>  0cc84_like |0 | 0
>>  1ab7c_like | 6260 |  6137
>>  32a5e_like |  2443202 |   2412585
>>  33678_like |0 | 0
>>  37b91_like |0 | 0
>>  6c02a_like |   15 |15
>>  75088_like |56746 |  9640
>>  90e0f_like | 8371 |  8089
>>  a08ec_like | 2815 |  2815
>>  b4ddf_like |0 | 0
>> (12 rows)
>>
>> The first highest read index is accidental and the query will be
>> rewritten.
>> The second-highest read index here though is from the third party app
>> django-oauth-toolkit:
>> https://github.com/jazzband/django-oauth-toolkit/blob/master/oauth2_provider/models.py#L284
>> .
>> The other indexes are on small tables and I'm not concerned about them
>> from a performance perspective.
>>
>>
>> Perhaps it's possible to use these index statistics in the system check
>> to prioritize which indexes are warned about.
>>
>> On Tue, 14 Apr 2020 at 13:18, Hannes Ljungberg 
>> wrote:
>>
>>> Thanks for your reply Tim,
>>>
>>> I also fail to see a "clean" upgrade path. The way I’ve been thinking
>>> of, is to just as you wrote, keep the code for deletion of the index so
>>> people who migrate to the Index-class will get it removed properly. Since
>>> it uses `IF EXISTS` it should be future-proof. We could add a system check
>>> that uses introspection to check if the `*_like` index is present on
>>> affected fields with `db_index=True` and warn about its deprecation. Then
>>> after a couple (?) of versions we could remove all traces of this index.
>>>
>>> The negative performance impact of removing this index for users of
>>> `LIKE` queries is very large but it’s hard to give numbers since it depends
>>> on the amount and nature of the data in the affected rows. Since we
>>> wouldn’t do any automatic removal of the index and add documentation about
>>> the importance of the opclass-indexes for `LIKE` queries I’m not that
>>> worried.
>>>
>>> I realise that removing the creation of these indexes result in a not so
>>> pretty deprecation path but I think users of Django would benefit from
>>> having to be more explicit about their indexes and learn about the quirks
>>> of their database engines.
>>>
>>>
>>> Den tisdag 14 april 2020 kl. 03:01:03 UTC+2 skrev Tim Graham:

 I have some sympathy for this issue as I'm trying to make the
 createcachetable management command use SchemaEditor rather than some
 custom SQL construction logic*. The related problem I ran into is that the
 primary key column (a CharField) uses unique=True which means those
 undesired opclasses indexes are created. I couldn't find a way to
 prevent that index besides filtering it out of the list of SQL statements.

 As for your proposal, how would you handle the upgrade path for
 existing projects? I imagine we could provide a script to run upon upgrade
 to remove all such existing indexes. No doubt some users won't run it.
 Would you keep around the code in Django's SchemaEditor like
 https://github.com/django/django/blob/53d229ff632c2a3e547f2820a94239f38ba4d4ac/django/db/backends/postgresql/schema.py#L178-L180
 that assumes those indexes exist?

 If Django stops creating those indexes, it could creat

Re: Proposal to not implicitly create varchar/text-pattern opclass indexes on PostgreSQL

2020-04-14 Thread Hannes Ljungberg
The `*_pattern_ops` operators all support ordinary equality comparisons but 
not  <, <=, >, or >= comparisons. So a regular `WHERE col = 1` would be 
able to use a `_like` index. See: 
https://www.postgresql.org/docs/current/indexes-opclass.html

But it's still interesting that the query planner always choose the 
non-unique index, one would think that the UNIQUE-index would weigh higher 
even if the cardinality is the same.

Den tisdag 14 april 2020 kl. 18:07:30 UTC+2 skrev Adam Johnson:
>
> Yes that one is odd. Looking more closely, django-oauth-toolkit doesn't 
> make any LIKE queries, and doesn't seem to ever have done so. It seems 
> rather that PostgreSQL is using the index to fulfill the unique constraint, 
> as the corresponding '_uniq' index is not being touched:
>
> core=> select indexrelname, idx_tup_read, idx_tup_fetch from 
> pg_stat_all_indexes where indexrelname like 
> 'oauth2_provider_accesstoken_token_%' order by 1;
>   indexrelname   | idx_tup_read | 
> idx_tup_fetch
>
> -+--+---
>  oauth2_provider_accesstoken_token_8af090f8_like |   169875 |   
>  169835
>  oauth2_provider_accesstoken_token_8af090f8_uniq |0 | 
> 0
> (2 rows)
>
> I haven't verified all the queries being made though.
>
> May still be useful for any check added to Django.
>
> On Tue, 14 Apr 2020 at 14:17, charettes > 
> wrote:
>
>> Makes we wonder under which circumstances an OAuth token would need to be 
>> looked up using LIKE. I would expect the unique b-tree index which covers 
>> exact lookups to be sufficient.
>>
>> Simon
>>
>> Le mardi 14 avril 2020 09:01:48 UTC-4, Adam Johnson a écrit :
>>>
>>> For some numbers: I just checked on a long-running client project using 
>>> PostgreSQL, and it looks like several of the *_like  indexes are in use:
>>>
>>> core=> select right(indexrelname, 10), idx_tup_read, idx_tup_fetch from 
>>> pg_stat_all_indexes where indexrelname like '%_like' order by 1;
>>>right| idx_tup_read | idx_tup_fetch
>>> +--+---
>>>  090f8_like |   168181 |168143
>>>  0b77e_like |0 | 0
>>>  0cc84_like |0 | 0
>>>  1ab7c_like | 6260 |  6137
>>>  32a5e_like |  2443202 |   2412585
>>>  33678_like |0 | 0
>>>  37b91_like |0 | 0
>>>  6c02a_like |   15 |15
>>>  75088_like |56746 |  9640
>>>  90e0f_like | 8371 |  8089
>>>  a08ec_like | 2815 |  2815
>>>  b4ddf_like |0 | 0
>>> (12 rows)
>>>
>>> The first highest read index is accidental and the query will be 
>>> rewritten.
>>> The second-highest read index here though is from the third party app 
>>> django-oauth-toolkit: 
>>> https://github.com/jazzband/django-oauth-toolkit/blob/master/oauth2_provider/models.py#L284
>>>   
>>> .
>>> The other indexes are on small tables and I'm not concerned about them 
>>> from a performance perspective.
>>>
>>>
>>> Perhaps it's possible to use these index statistics in the system check 
>>> to prioritize which indexes are warned about.
>>>
>>> On Tue, 14 Apr 2020 at 13:18, Hannes Ljungberg  
>>> wrote:
>>>
 Thanks for your reply Tim,

 I also fail to see a "clean" upgrade path. The way I’ve been thinking 
 of, is to just as you wrote, keep the code for deletion of the index so 
 people who migrate to the Index-class will get it removed properly. Since 
 it uses `IF EXISTS` it should be future-proof. We could add a system check 
 that uses introspection to check if the `*_like` index is present on 
 affected fields with `db_index=True` and warn about its deprecation. Then 
 after a couple (?) of versions we could remove all traces of this index. 

 The negative performance impact of removing this index for users of 
 `LIKE` queries is very large but it’s hard to give numbers since it 
 depends 
 on the amount and nature of the data in the affected rows. Since we 
 wouldn’t do any automatic removal of the index and add documentation about 
 the importance of the opclass-indexes for `LIKE` queries I’m not that 
 worried.

 I realise that removing the creation of these indexes result in a not 
 so pretty deprecation path but I think users of Django would benefit from 
 having to be more explicit about their indexes and learn about the quirks 
 of their database engines.


 Den tisdag 14 april 2020 kl. 03:01:03 UTC+2 skrev Tim Graham:
>
> I have some sympathy for this issue as I'm trying to make the 
> createcachetable management command use SchemaEditor rather than some 
> custom SQL construction logic*. The related problem I ran into is that 
> the 
> primary key column (a CharField) uses unique=True which means t

Swappable sessions

2020-04-14 Thread Lorenzo Peña
Hello fellows!

Has there ever been any discussion about making the Session model from 
django.contrib.sessions a swappable model? Sometimes it's required to 
enhance the session model in order to track more information. There are 
alternatives like django-user-sessions, but replacing the contrib session 
app doesn't feel right altogether.

Would something like this make any sense?

Thank you and stay stafe! 

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/4c8e6811-61fc-469a-8cf5-04f30b91810b%40googlegroups.com.


Re: Swappable sessions

2020-04-14 Thread Adam Johnson
Hi Lorenzo

Using a different Session model is already supported - see the docs:
https://docs.djangoproject.com/en/3.0/topics/http/sessions/#extending-database-backed-session-engines
. Or is there something beyond that which you wanted?

Thanks,

Adam

P.S. Please don't start your message addressed to "fellows" :) This mailing
list has more people than the active Django fellows

and many
non-males  ("fellow" is sometimes understood to mean
males only so best to avoid :) ).

On Tue, 14 Apr 2020 at 22:31, Lorenzo Peña  wrote:

> Hello fellows!
>
> Has there ever been any discussion about making the Session model from
> django.contrib.sessions a swappable model? Sometimes it's required to
> enhance the session model in order to track more information. There are
> alternatives like django-user-sessions, but replacing the contrib session
> app doesn't feel right altogether.
>
> Would something like this make any sense?
>
> Thank you and stay stafe!
>
> --
> 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 view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/4c8e6811-61fc-469a-8cf5-04f30b91810b%40googlegroups.com
> 
> .
>


-- 
Adam

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMyDDM0ehuEmohx76GbdThcVyAP5cYf1r8TYJ-cOC4g5SkUVCQ%40mail.gmail.com.


Re: Hashing Session Keys in backends

2020-04-14 Thread Adam Johnson
Hi Mark

Thanks for looking into this tricky security issue.

I'm suggesting to use the names frontend_key and backend_key for these two
> concepts.
>

They seem reasonable to me, as long as there's an explanatory comment.
Perhaps it even needs documenting for third party backends.

My second suggestion is to refactor the SessionBase class to make sure the
> session-key-hashing happens in one place and isn't spread across all
> different backend implementations as is the case now because the subclasses
> have to implemented public methods that receive the frontend_key.
>

Yes that seems reasonable, although I haven't looked closely. Is there a
way to avoid breaking third party backends, but raising deprecation
warnings? Perhaps using func_supports_parameter

or similar that we've used in past deprecations?

I see the PR is quite old and not owned by you. If you want to open a new
PR, rebase the existing code, and refactor it as you see fit, you can use
Co-Authored-By
 to
acknowledge Chris' original work.

Thanks,

Adam

On Fri, 10 Apr 2020 at 10:41, mark  wrote:

> After renewed interest because of potential database timing attacks (
> T31412 ) I'm looking into an
> existing PR (PR8736  for
> T21076 ) for adding the
> possibility of storing hashes of session keys.
>
> I'm looking to get some feedback on two things;
>
> After going through the existing commits of Chris Griffin, I agree with
> Aymeric Augustin (who did an initial review of the pull request) that there
> should be a clearer distinction between the incoming session key (Aymeric
> talks about a "clear text session key") and the key that gets stored in the
> sessions backend (Aymeric talks about a "hashed if needed session key").
> I'm suggesting
>  to
> use the names *frontend_key* and *backend_key* for these two concepts.
>
> My second suggestion
>  is to
> refactor the *SessionBase* class to make sure the
> session-key-hashing happens in one place and isn't spread across all
> different backend implementations as is the case now because the subclasses
> have to implemented public methods that receive the frontend_key. I'm
> suggesting to basically have subclasses implement private methods that
> receive a backend_key, which will be invoked by the public methods in the
> BaseClass. Obviously this will have consequences for any existing custom
> backends out there, though I think those will be affected either way.
>
> I welcome any thoughts on both the naming convention and the refactoring.
>
> --
> 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 view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/41c26919-f15f-4151-aa82-1281e24656da%40googlegroups.com
> 
> .
>


-- 
Adam

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMyDDM0Bd59HR%3DsvODPA15xvZHtvdcmLzaD49X4K52kMp8g8yQ%40mail.gmail.com.


Re: Swappable sessions

2020-04-14 Thread Lorenzo Peña
Thanks Adam, looks like the link you posted covers it all.

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/6cbc147f-c3d8-4911-92de-a35cfce26d4c%40googlegroups.com.