Re: Defaulting to use BigAutoField in models

2020-06-18 Thread Caio Ariede

> On 17 Jun 2020, at 16:56, Adam Johnson  wrote:
> 
> I think special casing the migrations framework is an avenue to explore, so I 
> created this today and to my surprise it seems to work quite well: 
> https://github.com/orf/django/commit/0a335f208cee1376c25eff55c6f866de122c7839 
> .
> 
> That is a pretty neat solution. I think it would work quite well.

I really like that solution as well, it’s pretty fair :)


> 
> What about a way to warn users approaching the limit? I think a database 
> check that detects AutoFields where the next ID is >50% of the way through 
> would be a good solution. That's the kind of monitoring I've built in the 
> past.
> 
> (50% might sound like it's near the limit, but the table is growing with an 
> exponential curve, even quite shallow, it's quite close in terms of time.)
> 

I think this is indeed a good to have.

Would we be able to provide instructions on how to migrate from SIGNED INT to 
UNSIGNED BIG INT? Perhaps a management command that outputs SQL like sqlmigrate?


--
Caio

-- 
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/5ECD26CA-339E-4954-A0BA-E7D8296E6988%40gmail.com.


Re: Defaulting to use BigAutoField in models

2020-06-18 Thread Adam Johnson
>
> Would we be able to provide instructions on how to migrate from SIGNED INT
> to UNSIGNED BIG INT? Perhaps a management command that outputs SQL like
> sqlmigrate?
>

It would only be needed to add this to the model:
id = BigAutoField()

Then migrations would pick it up, because the field is no longer
auto_created.

On Thu, 18 Jun 2020 at 13:24, Caio Ariede  wrote:

>
> On 17 Jun 2020, at 16:56, Adam Johnson  wrote:
>
> I think special casing the migrations framework is an avenue to explore,
>> so I created this today and to my surprise it seems to work quite well:
>> https://github.com/orf/django/commit/0a335f208cee1376c25eff55c6f866de122c7839
>> .
>
>
> That is a pretty neat solution. I think it would work quite well.
>
>
> I really like that solution as well, it’s pretty fair :)
>
>
>
> What about a way to warn users approaching the limit? I think a database
> check that detects AutoFields where the next ID is >50% of the way through
> would be a good solution. That's the kind of monitoring I've built in the
> past.
>
> (50% might sound like it's near the limit, but the table is growing with
> an exponential curve, even quite shallow, it's quite close in terms of
> time.)
>
>
> I think this is indeed a good to have.
>
> Would we be able to provide instructions on how to migrate from SIGNED INT
> to UNSIGNED BIG INT? Perhaps a management command that outputs SQL like
> sqlmigrate?
>
>
> --
> Caio
>
> --
> 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/5ECD26CA-339E-4954-A0BA-E7D8296E6988%40gmail.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/CAMyDDM0W-8ywE%3D3fHpdoMn3%3Dw-3J%2BN2e_nEgJHaNYia-3Xv%2BWw%40mail.gmail.com.


Re: Defaulting to use BigAutoField in models

2020-06-18 Thread Caio Ariede
What about third party apps?

It would be great to migrate every AutoField to BigAutoField without the need 
to specify them manually.


--
Caio



> On 18 Jun 2020, at 09:40, Adam Johnson  wrote:
> 
> Would we be able to provide instructions on how to migrate from SIGNED INT to 
> UNSIGNED BIG INT? Perhaps a management command that outputs SQL like 
> sqlmigrate?
> 
> It would only be needed to add this to the model:
> id = BigAutoField() 
> 
> Then migrations would pick it up, because the field is no longer auto_created.
> 
> On Thu, 18 Jun 2020 at 13:24, Caio Ariede  > wrote:
> 
>> On 17 Jun 2020, at 16:56, Adam Johnson > > wrote:
>> 
>> I think special casing the migrations framework is an avenue to explore, so 
>> I created this today and to my surprise it seems to work quite well: 
>> https://github.com/orf/django/commit/0a335f208cee1376c25eff55c6f866de122c7839
>>  
>> .
>> 
>> That is a pretty neat solution. I think it would work quite well.
> 
> I really like that solution as well, it’s pretty fair :)
> 
> 
>> 
>> What about a way to warn users approaching the limit? I think a database 
>> check that detects AutoFields where the next ID is >50% of the way through 
>> would be a good solution. That's the kind of monitoring I've built in the 
>> past.
>> 
>> (50% might sound like it's near the limit, but the table is growing with an 
>> exponential curve, even quite shallow, it's quite close in terms of time.)
>> 
> 
> I think this is indeed a good to have.
> 
> Would we be able to provide instructions on how to migrate from SIGNED INT to 
> UNSIGNED BIG INT? Perhaps a management command that outputs SQL like 
> sqlmigrate?
> 
> 
> --
> Caio
> 
> -- 
> 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/5ECD26CA-339E-4954-A0BA-E7D8296E6988%40gmail.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/CAMyDDM0W-8ywE%3D3fHpdoMn3%3Dw-3J%2BN2e_nEgJHaNYia-3Xv%2BWw%40mail.gmail.com
>  
> .

-- 
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/AFE3161D-6571-46B1-AAD3-C21E4A9CE0C0%40gmail.com.


Re: Defaulting to use BigAutoField in models

2020-06-18 Thread Adam Johnson
This would make the change only affect existing installs of third party
apps. I think documentation on using RunSQL() in a migration (in a project
app) could be warranted.

On Thu, 18 Jun 2020 at 14:50, Caio Ariede  wrote:

> What about third party apps?
>
> It would be great to migrate every AutoField to BigAutoField without the
> need to specify them manually.
>
>
> --
> Caio
>
>
>
> On 18 Jun 2020, at 09:40, Adam Johnson  wrote:
>
> Would we be able to provide instructions on how to migrate from SIGNED INT
>> to UNSIGNED BIG INT? Perhaps a management command that outputs SQL like
>> sqlmigrate?
>>
>
> It would only be needed to add this to the model:
> id = BigAutoField()
>
> Then migrations would pick it up, because the field is no longer
> auto_created.
>
> On Thu, 18 Jun 2020 at 13:24, Caio Ariede  wrote:
>
>>
>> On 17 Jun 2020, at 16:56, Adam Johnson  wrote:
>>
>> I think special casing the migrations framework is an avenue to explore,
>>> so I created this today and to my surprise it seems to work quite well:
>>> https://github.com/orf/django/commit/0a335f208cee1376c25eff55c6f866de122c7839
>>> .
>>
>>
>> That is a pretty neat solution. I think it would work quite well.
>>
>>
>> I really like that solution as well, it’s pretty fair :)
>>
>>
>>
>> What about a way to warn users approaching the limit? I think a database
>> check that detects AutoFields where the next ID is >50% of the way through
>> would be a good solution. That's the kind of monitoring I've built in the
>> past.
>>
>> (50% might sound like it's near the limit, but the table is growing with
>> an exponential curve, even quite shallow, it's quite close in terms of
>> time.)
>>
>>
>> I think this is indeed a good to have.
>>
>> Would we be able to provide instructions on how to migrate from SIGNED
>> INT to UNSIGNED BIG INT? Perhaps a management command that outputs SQL like
>> sqlmigrate?
>>
>>
>> --
>> Caio
>>
>> --
>> 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/5ECD26CA-339E-4954-A0BA-E7D8296E6988%40gmail.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/CAMyDDM0W-8ywE%3D3fHpdoMn3%3Dw-3J%2BN2e_nEgJHaNYia-3Xv%2BWw%40mail.gmail.com
> 
> .
>
>
> --
> 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/AFE3161D-6571-46B1-AAD3-C21E4A9CE0C0%40gmail.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/CAMyDDM2PYgN6Nnyk%3D2khTzZELBJbDotueQOAV%3DfJFv4zOA0KPA%40mail.gmail.com.


Re: Defaulting to use BigAutoField in models

2020-06-18 Thread charettes
+1 from me as well but I don't think we should add any logic to the 
migration framework to handle the transition.

I think the release plan should be something along the following

Phase 1:
- New projects are generated with MODEL_DEFAULT_PK = 
'django.db.models.BigAutoField'
- A system check or warning is emitted when the setting is not defined and 
MODEL_DEFAULT_PK default to 'django.db.models.AutoField' during the 
transition period. The warning should warn that the value will default to 
'django.db.models.AutoField' in a future version of Django.
- An upgrade path is mentioned in the docs to mention that explicit ` id = 
AutoField()` must be assigned to preserve previous behaviour and avoid 
migrations. This one of the thing projects such as django-codemod could 
really help with[0]

Phase 2:
- settings.MODEL_DEFAULT_PK now defaults to 'django.db.models.BigAutoField'
- System check/warning about undefined MODEL_DEFAULT_PK setting is removed.
- Let the migration framework generate migrations just like it normally 
would for projects that didn't follow the documented migration path.
- Optionally add a system check that warns about usage of `AutoField` 
because of its possible overflow.

> I think we should restrict the setting between normal and big auto fields 
only. Allowing UUID's would be changing the type, with the potential for 
havoc with code incompalities throughout django. It's also not possible to 
migrate tables over to the new type.

I'm not sure I agree here. For folks that are setting a up a new project 
starting with UUIDField primary keys can be useful and if we're adding a 
setting for this purpose I think we should it make it as flexible as 
possible.


[0] https://github.com/browniebroke/django-codemod

Le jeudi 11 juin 2020 11:28:59 UTC-4, Tom Forbes a écrit :
>
> I’d like to re-propose switching Django to use BigAutoField’s rather than 
> the current AutoField. This has been proposed[1] before (and a MR made[2]) 
> but it was closed due to implementation issues and not much else has 
> happened since then.
>
> As many of you are aware the max value a standard AutoField can hold is 
> 2,147,483,647 (2.1 billion) which sounds like more than you can ever need. 
> But it’s often not, and you only find out at the worst possible time - out 
> of the blue at night and during a period of rapid growth. The process for 
> fixing this issue also becomes a lot harder as your data grows - when 
> you’ve hit the limit you’re looking at a multi-hour `ALTER TABLE` on 
> Postgres during which writes and reads are blocked, or a risky operation to 
> create a new table with the correct primary key and copy old data over in 
> batches. Basically if you’ve experienced this before you wouldn’t wish it 
> on your worst enemy.
>
> I’m proposing that we add a `MODELS_PRIMARY_KEY` (name improvements 
> welcome!) setting that _defaults_ to `BigAutoField`, with prominent 
> docs/release notes saying that to preserve the existing behaviour this 
> should be set to `AutoField`. I think this the only realistic way we can 
> implement this for new projects in a way that ensures it will be used 
> meaningfully and not forgotten about until it’s too late.
>
> Rails managed to do this migration somewhat painlessly due the big 
> differences between Rails and Django models. As Django migrations are 
> derived from the current model state so there’s no way I can think of to 
> express “make this auto-generated field a BigAutoField only if this model 
> is *new*”. The way I see it is that a global setting is very easy to 
> toggle and there is little chance of missing the large numbers of 
> migrations that would be generated during the Django update. Smaller 
> applications could apply the migrations with little issue and larger 
> applications would be able to opt-out (as well as be reminded that this is 
> a problem they could face!).
>
> Some specifics:
> - The setting would take any dotted path to a class, or a single class 
> name for a build in field. This would potentially solve [3], and could be 
> useful to people who want to default to other fields like UUIDs (or a 
> custom BigAutoField) for whatever reason
> - The setting would also be used for GenericForeignKeys, which right now 
> are backed by a PositiveIntegerField and so suffer from the same AutoField 
> limitations
>
> Any thoughts on this?
>
> Tom
>
> 1. 
> https://groups.google.com/d/msg/django-developers/imBJwRrtJkk/P4g0Y87lAgAJ
>
> 2. https://github.com/django/django/pull/8924/
>
> 3. https://code.djangoproject.com/ticket/56 
> 
>
>
>
>

-- 
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

Re: Defaulting to use BigAutoField in models

2020-06-18 Thread charettes
Email got sent too fast

I meant

> The warning should warn that the value will default to 
'django.db.models.BigAutoField' in a future version of Django.

Regarding third party apps a solution could be to allow a per-app config 
option or encourage them to explicitly choose which primary key they use 
for their model but I'm not a big fan of baking logic in the migration 
framework to treat some fields in a special way.

Simon

Le jeudi 18 juin 2020 10:18:35 UTC-4, charettes a écrit :
>
> +1 from me as well but I don't think we should add any logic to the 
> migration framework to handle the transition.
>
> I think the release plan should be something along the following
>
> Phase 1:
> - New projects are generated with MODEL_DEFAULT_PK = 
> 'django.db.models.BigAutoField'
> - A system check or warning is emitted when the setting is not defined and 
> MODEL_DEFAULT_PK default to 'django.db.models.AutoField' during the 
> transition period. The warning should warn that the value will default to 
> 'django.db.models.AutoField' in a future version of Django.
> - An upgrade path is mentioned in the docs to mention that explicit ` id = 
> AutoField()` must be assigned to preserve previous behaviour and avoid 
> migrations. This one of the thing projects such as django-codemod could 
> really help with[0]
>
> Phase 2:
> - settings.MODEL_DEFAULT_PK now defaults to 'django.db.models.BigAutoField'
> - System check/warning about undefined MODEL_DEFAULT_PK setting is removed.
> - Let the migration framework generate migrations just like it normally 
> would for projects that didn't follow the documented migration path.
> - Optionally add a system check that warns about usage of `AutoField` 
> because of its possible overflow.
>
> > I think we should restrict the setting between normal and big auto 
> fields only. Allowing UUID's would be changing the type, with the potential 
> for havoc with code incompalities throughout django. It's also not possible 
> to migrate tables over to the new type.
>
> I'm not sure I agree here. For folks that are setting a up a new project 
> starting with UUIDField primary keys can be useful and if we're adding a 
> setting for this purpose I think we should it make it as flexible as 
> possible.
>
>
> [0] https://github.com/browniebroke/django-codemod
>
> Le jeudi 11 juin 2020 11:28:59 UTC-4, Tom Forbes a écrit :
>>
>> I’d like to re-propose switching Django to use BigAutoField’s rather than 
>> the current AutoField. This has been proposed[1] before (and a MR made[2]) 
>> but it was closed due to implementation issues and not much else has 
>> happened since then.
>>
>> As many of you are aware the max value a standard AutoField can hold is 
>> 2,147,483,647 (2.1 billion) which sounds like more than you can ever need. 
>> But it’s often not, and you only find out at the worst possible time - out 
>> of the blue at night and during a period of rapid growth. The process for 
>> fixing this issue also becomes a lot harder as your data grows - when 
>> you’ve hit the limit you’re looking at a multi-hour `ALTER TABLE` on 
>> Postgres during which writes and reads are blocked, or a risky operation to 
>> create a new table with the correct primary key and copy old data over in 
>> batches. Basically if you’ve experienced this before you wouldn’t wish it 
>> on your worst enemy.
>>
>> I’m proposing that we add a `MODELS_PRIMARY_KEY` (name improvements 
>> welcome!) setting that _defaults_ to `BigAutoField`, with prominent 
>> docs/release notes saying that to preserve the existing behaviour this 
>> should be set to `AutoField`. I think this the only realistic way we can 
>> implement this for new projects in a way that ensures it will be used 
>> meaningfully and not forgotten about until it’s too late.
>>
>> Rails managed to do this migration somewhat painlessly due the big 
>> differences between Rails and Django models. As Django migrations are 
>> derived from the current model state so there’s no way I can think of to 
>> express “make this auto-generated field a BigAutoField only if this model 
>> is *new*”. The way I see it is that a global setting is very easy to 
>> toggle and there is little chance of missing the large numbers of 
>> migrations that would be generated during the Django update. Smaller 
>> applications could apply the migrations with little issue and larger 
>> applications would be able to opt-out (as well as be reminded that this is 
>> a problem they could face!).
>>
>> Some specifics:
>> - The setting would take any dotted path to a class, or a single class 
>> name for a build in field. This would potentially solve [3], and could be 
>> useful to people who want to default to other fields like UUIDs (or a 
>> custom BigAutoField) for whatever reason
>> - The setting would also be used for GenericForeignKeys, which right now 
>> are backed by a PositiveIntegerField and so suffer from the same AutoField 
>> limitations
>>
>> Any thoughts on this?
>>
>> Tom
>>
>

Mechanics of the EmailValidator.domain_allowlist rename.

2020-06-18 Thread Carlton Gibson
Hi All, 

We merged the docs changes remove whitelisted &co last week. 
https://github.com/django/django/pull/13031

There's a PR now ready to adjust EmailValidator to use domain_allowlist. 
https://github.com/django/django/pull/13079/

This is all fine *except* it will require editing any historical migrations 
that feature EmailValidator, using the `domain_whitelist` attribute, since 
validators are serialized in migrations. 

* Thinking of in user projects, and any third-party apps.
   * An affected third-party would need to think about timing of a change, 
support say 2.2 until EOL. ...
* Suspecting total number of affected lines is low, but not zero. 
* To begin there's a deprecation period, so plenty of warning, but asking 
people to edit old migrations is not ideal shall we say. 

(Hopefully that's not too terse.) 

Options: 


*1. Bite the bullet. *

I'm not too keen on this: `on_delete=models.CASCADE`. 
In the 1.9 release notes there was, "Update models and existing migrations 
to explicitly set the argument."
It was horrible. 

The scale here is not anything like the same but... 


*2. Don't remove the whitelist kwarg. *

We could just leave it forever, undocumented, commented in the code as just 
an alias to the new domain_allowlist. 

If we did this, there's a question as to whether we'd have a warning for 
the kwarg during the deprecation period?
(If we did, we'd remove the warning, but not the actual kwarg.) 

This is horrible too, but given 1 I think we have to consider it. 


*3. Some kind of script to make the changes for us. *

So I see django-codebump just came up in the BigAutoField thread. Similar 
came up in the Remove url() thread recently. 

What's the state of play here? Are we in a position (by 3.2) to ship a 
utility to (*inter alia*) automate updating historical migrations? 
Without having looked into it, this seems like a small, well scoped 
problem, that would make a good test case for such a tool. 

In theory this is better that 1 or 2, but is it feasible? 

Thoughts? 

Thanks. 
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/05567701-5721-469b-830b-11da4f116d4bo%40googlegroups.com.


Re: Mechanics of the EmailValidator.domain_allowlist rename.

2020-06-18 Thread Matthias Kestenholz
Hi

I have survived a few changes in Django over the last >10 years, and this
change is absolutely not comparable to any of the problematic ones.

For example, the get_query_set -> get_queryset rename was hard because it
was unclear how to properly support subclassing querysets.

The on_delete addition was terrible because everything had to be edited by
hand. The time investment is significant even after upgrading dozens of
sites.

The proposed rename can be achieved in projects with a relatively
straightforward invocation of "git grep --name-only domain_whitelist |
xargs sed -i -e s/ domain_whitelist/ domain_allowlist/g". We don't have to
add new arguments by hand or think about subclassing.

In my opinion 1) really is fine in this case. Yes, it will also need a
little bit of time to update this, but it'll be a blip compared to other
problems I'll encounter when the next upgrade comes around.

Thanks,
Matthias

On Thu, Jun 18, 2020 at 5:10 PM Carlton Gibson 
wrote:

> Hi All,
>
> We merged the docs changes remove whitelisted &co last week.
> https://github.com/django/django/pull/13031
>
> There's a PR now ready to adjust EmailValidator to use domain_allowlist.
> https://github.com/django/django/pull/13079/
>
> This is all fine *except* it will require editing any historical
> migrations that feature EmailValidator, using the `domain_whitelist`
> attribute, since validators are serialized in migrations.
>
> * Thinking of in user projects, and any third-party apps.
>* An affected third-party would need to think about timing of a change,
> support say 2.2 until EOL. ...
> * Suspecting total number of affected lines is low, but not zero.
> * To begin there's a deprecation period, so plenty of warning, but asking
> people to edit old migrations is not ideal shall we say.
>
> (Hopefully that's not too terse.)
>
> Options:
>
>
> *1. Bite the bullet. *
>
> I'm not too keen on this: `on_delete=models.CASCADE`.
> In the 1.9 release notes there was, "Update models and existing migrations
> to explicitly set the argument."
> It was horrible.
>
> The scale here is not anything like the same but...
>
>
> *2. Don't remove the whitelist kwarg. *
>
> We could just leave it forever, undocumented, commented in the code as
> just an alias to the new domain_allowlist.
>
> If we did this, there's a question as to whether we'd have a warning for
> the kwarg during the deprecation period?
> (If we did, we'd remove the warning, but not the actual kwarg.)
>
> This is horrible too, but given 1 I think we have to consider it.
>
>
> *3. Some kind of script to make the changes for us. *
>
> So I see django-codebump just came up in the BigAutoField thread. Similar
> came up in the Remove url() thread recently.
>
> What's the state of play here? Are we in a position (by 3.2) to ship a
> utility to (*inter alia*) automate updating historical migrations?
> Without having looked into it, this seems like a small, well scoped
> problem, that would make a good test case for such a tool.
>
> In theory this is better that 1 or 2, but is it feasible?
>
> Thoughts?
>
> Thanks.
> 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/05567701-5721-469b-830b-11da4f116d4bo%40googlegroups.com
> 
> .
>

-- 
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/CANvPqgBJF-weRiYqE5fMwWO1eV02oVwmqDgQPRBo5itynBycww%40mail.gmail.com.


Re: Mechanics of the EmailValidator.domain_allowlist rename.

2020-06-18 Thread Adam Johnson
I also lean towards #1.

I doubt many third party packages use the argument since it seems quite
site specific. I scrolled through 5 random-ish pages of github code search
for "domain_whitelist" in python files and found nothing that looked like a
django related usage.

On Thu, 18 Jun 2020 at 16:42, Matthias Kestenholz  wrote:

> Hi
>
> I have survived a few changes in Django over the last >10 years, and this
> change is absolutely not comparable to any of the problematic ones.
>
> For example, the get_query_set -> get_queryset rename was hard because it
> was unclear how to properly support subclassing querysets.
>
> The on_delete addition was terrible because everything had to be edited by
> hand. The time investment is significant even after upgrading dozens of
> sites.
>
> The proposed rename can be achieved in projects with a relatively
> straightforward invocation of "git grep --name-only domain_whitelist |
> xargs sed -i -e s/ domain_whitelist/ domain_allowlist/g". We don't have to
> add new arguments by hand or think about subclassing.
>
> In my opinion 1) really is fine in this case. Yes, it will also need a
> little bit of time to update this, but it'll be a blip compared to other
> problems I'll encounter when the next upgrade comes around.
>
> Thanks,
> Matthias
>
> On Thu, Jun 18, 2020 at 5:10 PM Carlton Gibson 
> wrote:
>
>> Hi All,
>>
>> We merged the docs changes remove whitelisted &co last week.
>> https://github.com/django/django/pull/13031
>>
>> There's a PR now ready to adjust EmailValidator to use domain_allowlist.
>> https://github.com/django/django/pull/13079/
>>
>> This is all fine *except* it will require editing any historical
>> migrations that feature EmailValidator, using the `domain_whitelist`
>> attribute, since validators are serialized in migrations.
>>
>> * Thinking of in user projects, and any third-party apps.
>>* An affected third-party would need to think about timing of a
>> change, support say 2.2 until EOL. ...
>> * Suspecting total number of affected lines is low, but not zero.
>> * To begin there's a deprecation period, so plenty of warning, but asking
>> people to edit old migrations is not ideal shall we say.
>>
>> (Hopefully that's not too terse.)
>>
>> Options:
>>
>>
>> *1. Bite the bullet. *
>>
>> I'm not too keen on this: `on_delete=models.CASCADE`.
>> In the 1.9 release notes there was, "Update models and existing
>> migrations to explicitly set the argument."
>> It was horrible.
>>
>> The scale here is not anything like the same but...
>>
>>
>> *2. Don't remove the whitelist kwarg. *
>>
>> We could just leave it forever, undocumented, commented in the code as
>> just an alias to the new domain_allowlist.
>>
>> If we did this, there's a question as to whether we'd have a warning for
>> the kwarg during the deprecation period?
>> (If we did, we'd remove the warning, but not the actual kwarg.)
>>
>> This is horrible too, but given 1 I think we have to consider it.
>>
>>
>> *3. Some kind of script to make the changes for us. *
>>
>> So I see django-codebump just came up in the BigAutoField thread. Similar
>> came up in the Remove url() thread recently.
>>
>> What's the state of play here? Are we in a position (by 3.2) to ship a
>> utility to (*inter alia*) automate updating historical migrations?
>> Without having looked into it, this seems like a small, well scoped
>> problem, that would make a good test case for such a tool.
>>
>> In theory this is better that 1 or 2, but is it feasible?
>>
>> Thoughts?
>>
>> Thanks.
>> 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/05567701-5721-469b-830b-11da4f116d4bo%40googlegroups.com
>> 
>> .
>>
>
>
> --
> 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/CANvPqgBJF-weRiYqE5fMwWO1eV02oVwmqDgQPRBo5itynBycww%40mail.gmail.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 d

Re: Mechanics of the EmailValidator.domain_allowlist rename.

2020-06-18 Thread Carlton Gibson
OK, thanks both — perhaps we're worrying too much. 

> For example, the get_query_set -> get_queryset rename... 

I'd managed to suppress that memory. 😀

C.

On Thursday, 18 June 2020 19:05:54 UTC+2, Adam Johnson wrote:
>
> I also lean towards #1.
>
> I doubt many third party packages use the argument since it seems quite 
> site specific. I scrolled through 5 random-ish pages of github code search 
> for "domain_whitelist" in python files and found nothing that looked like a 
> django related usage.
>
> On Thu, 18 Jun 2020 at 16:42, Matthias Kestenholz  > wrote:
>
>> Hi
>>
>> I have survived a few changes in Django over the last >10 years, and this 
>> change is absolutely not comparable to any of the problematic ones.
>>
>> For example, the get_query_set -> get_queryset rename was hard because it 
>> was unclear how to properly support subclassing querysets.
>>
>> The on_delete addition was terrible because everything had to be edited 
>> by hand. The time investment is significant even after upgrading dozens of 
>> sites.
>>
>> The proposed rename can be achieved in projects with a relatively 
>> straightforward invocation of "git grep --name-only domain_whitelist | 
>> xargs sed -i -e s/ domain_whitelist/ domain_allowlist/g". We don't have to 
>> add new arguments by hand or think about subclassing.
>>
>> In my opinion 1) really is fine in this case. Yes, it will also need a 
>> little bit of time to update this, but it'll be a blip compared to other 
>> problems I'll encounter when the next upgrade comes around.
>>
>> Thanks,
>> Matthias
>>
>> On Thu, Jun 18, 2020 at 5:10 PM Carlton Gibson > > wrote:
>>
>>> Hi All, 
>>>
>>> We merged the docs changes remove whitelisted &co last week. 
>>> https://github.com/django/django/pull/13031
>>>
>>> There's a PR now ready to adjust EmailValidator to use domain_allowlist. 
>>> https://github.com/django/django/pull/13079/
>>>
>>> This is all fine *except* it will require editing any historical 
>>> migrations that feature EmailValidator, using the `domain_whitelist` 
>>> attribute, since validators are serialized in migrations. 
>>>
>>> * Thinking of in user projects, and any third-party apps.
>>>* An affected third-party would need to think about timing of a 
>>> change, support say 2.2 until EOL. ...
>>> * Suspecting total number of affected lines is low, but not zero. 
>>> * To begin there's a deprecation period, so plenty of warning, but 
>>> asking people to edit old migrations is not ideal shall we say. 
>>>
>>> (Hopefully that's not too terse.) 
>>>
>>> Options: 
>>>
>>>
>>> *1. Bite the bullet. *
>>>
>>> I'm not too keen on this: `on_delete=models.CASCADE`. 
>>> In the 1.9 release notes there was, "Update models and existing 
>>> migrations to explicitly set the argument."
>>> It was horrible. 
>>>
>>> The scale here is not anything like the same but... 
>>>
>>>
>>> *2. Don't remove the whitelist kwarg. *
>>>
>>> We could just leave it forever, undocumented, commented in the code as 
>>> just an alias to the new domain_allowlist. 
>>>
>>> If we did this, there's a question as to whether we'd have a warning for 
>>> the kwarg during the deprecation period?
>>> (If we did, we'd remove the warning, but not the actual kwarg.) 
>>>
>>> This is horrible too, but given 1 I think we have to consider it. 
>>>
>>>
>>> *3. Some kind of script to make the changes for us. *
>>>
>>> So I see django-codebump just came up in the BigAutoField thread. 
>>> Similar came up in the Remove url() thread recently. 
>>>
>>> What's the state of play here? Are we in a position (by 3.2) to ship a 
>>> utility to (*inter alia*) automate updating historical migrations? 
>>> Without having looked into it, this seems like a small, well scoped 
>>> problem, that would make a good test case for such a tool. 
>>>
>>> In theory this is better that 1 or 2, but is it feasible? 
>>>
>>> Thoughts? 
>>>
>>> Thanks. 
>>> 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-d...@googlegroups.com .
>>> To view this discussion on the web visit 
>>> https://groups.google.com/d/msgid/django-developers/05567701-5721-469b-830b-11da4f116d4bo%40googlegroups.com
>>>  
>>> 
>>> .
>>>
>>
>>
>> -- 
>> 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-d...@googlegroups.com .
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/django-developers/CANvPqgBJF-weRiYqE5fMwWO1eV02oVwmqDgQPRBo5itynBycww%40mail.gmail.com
>>  
>>