Re: Proposal: Drop dependency on pytz in favor of zoneinfo

[Jure Erznožnik]

I would definitely be in favor of an opt-in: it would give developers 
time to move to the new system at their convenience.


Example: we're about to try and tackle the TZ issue in our apps and we 
want to do it "globally" with one definitive solution. I'd much rather 
do it on a library that is currently favoured, but not yet default than 
on a deprecated one, even if it's not yet officially deprecated. We do 
have some "import pytz", but currently they are few. Once we have a 
proper approach to handling timezone stuff, there's likely going to be 
more of them... or less, depending on the solution ;-)


LP,
Jure

On 7. 10. 20 17:25, Paul Ganssle wrote:


This sounds like a reasonable timeline to me. I think the breakage 
will be relatively small because I suspect many end-users don't really 
even know to use `normalize` in the first place, and when introducing 
the shim into a fundamental library at work I did not get a huge 
number of breakages, but I am still convinced that it is reasonably 
categorized as a breaking change.


I do think that there's one additional stage that we need to add here 
(and we chatted about this on twitter a bit), which is a stage that is 
fully backwards compatible where Django supports using non-pytz zones 
for users who bring their own time zone. I suspect that will help ease 
any breaking pain between 3.2 and 4.0, because no one would be forced 
to make any changes, but end users could proactively migrate to 
zoneinfo for a smoother transition.


I think most of what needs to be done is already in my original PR, it 
just needs a little conditional logic to handle pytz as well as the shim.


I am not sure how you feel about feature flags, but as a "nice to 
have", I imagine it would also be possible to add a feature flag that 
opts you in to `zoneinfo` as time zone provider even in 3.2, so that 
people can jump straight to the 5.0 behavior if they are ready for it.


I should be able to devote some time to at least the first part — 
making Django compatible with zoneinfo even if not actively using it — 
but likely not for a few weeks at minimum. If anyone wants to jump on 
either of these ahead of me I don't mind at all and feel free to ping 
me for review.


Best,
Paul

On 10/7/20 10:48 AM, Carlton Gibson wrote:

Hi Paul.

Thanks for the input here, and for your patience

> I am fairly certain this is going to be a tricky migration and will 
inevitably come with /some/ user pain. I don't think this will be 
Python 2 → 3 style pain, but some users who have been doing the 
"right thing" with pytz will need to make changes to their code in 
the long run, which is unfortunate.


Looking at all the docs, your migration guide on 
pytz_deprecation_shim, the example Kevin gave 
, where we do some 
arithmetic in a local timezone, and call `normalize()` in case we 
crossed a DST boundary, there's no way we can do this without forcing 
a breaking change somewhere.


So, probably, I've been staring at this too long today, but I think 
we should introduce the shim in Django 4.0. Django 3.2, the next 
major release will be an LTS. If we hold-off introducing the change 
until 4.0, we can flag it as a breaking change in the 4.0 release 
notes, with big warnings, allowing folks extra time to hang out on 
the previous LTS if they need it.


What I wouldn't want to do is to bring the breaking change in in 
Django 3.2, because we'll have a whole load of folks updating from 
the 2.2 LTS at about the time when it goes End of Life, and with no 
warning, that'd be a hard breaking change to throw on top of their 
other issues.


We'd keep the shim in place for the entire 4.x series, removing in 
Django 5.0 as per the deprecation policy 
.


I think the advantages of doing it this way are two-fold:

* We allow people to focus on the semantic breaking change (in folds) 
separately from the code changes per se — the logic may have changed 
slightly in these cases, but it'll still run.
* It looks easier to migrate Django's code vs branching on a new 
setting. (I didn't think through exactly what that might look like, 
so happy to see a PoC from anyone.)


I'm more attached to the timeline (i.e. making the change after the 
next LTS) than whether we use the deprecation shim or not, but can I 
ask others to give this their thought too?


Thanks again!

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/ce04a6b7-4409-4b20-ba30-4cd64dc0cabfn%40googlegroups.com 

async_unsafe's get_event_loop usage leads to "Too many open files" in tests

[patrick91]

Hi folks, I was going to write this in django developers, but looks like I 
might have found a small area for improvement for the async_unsafe 
decorator, but first let me give you a bit of background:

We're upgrading to Django 3.0 and we've been encountering this error on ci 
when running all the tests: "OSError: [Errno 24] Too many open files".

Here's the full stack trace:  https://dpaste.com/ABYJFP6CR

By inspecting it and the source code it seems to be related to the 
async_unsafe decorator: 
https://github.com/django/django/blob/855fc06236630464055b4f9ea422c68a07c6d02a/django/utils/asyncio.py#L19

This decorator calls `get_event_loop`: 
https://github.com/python/cpython/blob/3.7/Lib/asyncio/events.py#L632

which creates a new loop every time, which seems to be the cause of the too 
many files open. Wouldn't be best to use `get_running_loop` 
https://github.com/python/cpython/blob/3.7/Lib/asyncio/events.py#L681 
?

We are already checking for RuntimeError so it should be an easy fix, but I 
wonder if the usage of `get_event_loop` is intentional and I'm missing 
something here.

Happy to provide a patch for this :)

Patrick

-- 
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/a7cedcc5-b041-4f21-9a22-e9ffb92d3c91n%40googlegroups.com.

Re: async_unsafe's get_event_loop usage leads to "Too many open files" in tests

[Adam Johnson]

>
>  I was going to write this in django developers


This is django-developers?

which creates a new loop every time, which seems to be the cause of the too
> many files open.
>

Can you give a bit more explanation how it creates a new loop every time?

Two successive calls to get_event_loop() in ipython give me the same object:

In [4]: l = asyncio.get_event_loop()

In [5]: l2 = asyncio.get_event_loop()

In [6]: id(l)
Out[6]: 4561753280

In [7]: id(l2)
Out[7]: 4561753280

Additionally, the docs for get_running_event_loop() say:

> This function can only be called from a coroutine or a callback.
>
Plus it was added in Python 3.7, whilst Django 3.1 supports 3.6+. So it's
not clear we can use it.

That said, it would be nice to avoid the overhead of creating an event loop
for sync-only use cases.

On Thu, 8 Oct 2020 at 21:28, patrick91  wrote:

> Hi folks, I was going to write this in django developers, but looks like I
> might have found a small area for improvement for the async_unsafe
> decorator, but first let me give you a bit of background:
>
> We're upgrading to Django 3.0 and we've been encountering this error on ci
> when running all the tests: "OSError: [Errno 24] Too many open files".
>
> Here's the full stack trace:  https://dpaste.com/ABYJFP6CR
>
> By inspecting it and the source code it seems to be related to the
> async_unsafe decorator:
> https://github.com/django/django/blob/855fc06236630464055b4f9ea422c68a07c6d02a/django/utils/asyncio.py#L19
>
> This decorator calls `get_event_loop`:
> https://github.com/python/cpython/blob/3.7/Lib/asyncio/events.py#L632
>
> which creates a new loop every time, which seems to be the cause of the
> too many files open. Wouldn't be best to use `get_running_loop`
> https://github.com/python/cpython/blob/3.7/Lib/asyncio/events.py#L681 ?
>
> We are already checking for RuntimeError so it should be an easy fix, but
> I wonder if the usage of `get_event_loop` is intentional and I'm missing
> something here.
>
> Happy to provide a patch for this :)
>
> Patrick
>
> --
> 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/a7cedcc5-b041-4f21-9a22-e9ffb92d3c91n%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/CAMyDDM2_jHz54gfwcqwib9G%2BfxKxh9HER8dYJjJ4G1hNxvysuw%40mail.gmail.com.

Re: async_unsafe's get_event_loop usage leads to "Too many open files" in tests

[Patrick Arminio]

On Thu, 8 Oct 2020 at 22:28, Adam Johnson  wrote:

>  I was going to write this in django developers
>
>
> This is django-developers?
>
I meant django-users, sorry :D


> which creates a new loop every time, which seems to be the cause of the
>> too many files open.
>>
>
> Can you give a bit more explanation how it creates a new loop every time?
>
Uhm, I'll try to investigate a bit more tomorrow, we have a CI failure
where ~152 fail due to the same issue,
I wonder if I can set up a small test suite that can reproduce this. But I
guess every test runs the same

Two successive calls to get_event_loop() in ipython give me the same object:
>
> In [4]: l = asyncio.get_event_loop()
>
> In [5]: l2 = asyncio.get_event_loop()
>
> In [6]: id(l)
> Out[6]: 4561753280
>
> In [7]: id(l2)
> Out[7]: 4561753280
>
> Additionally, the docs for get_running_event_loop() say:
>
>> This function can only be called from a coroutine or a callback.
>>
> That's why we need the RuntimeError check, no?


> Plus it was added in Python 3.7, whilst Django 3.1 supports 3.6+. So it's
> not clear we can use it.
>
Oh, this might be a problem considering the implementation of
`get_running_event_loop` is written in C,
not sure it can be backported easily.


> That said, it would be nice to avoid the overhead of creating an event
> loop for sync-only use cases.
>
Yup :)


> On Thu, 8 Oct 2020 at 21:28, patrick91  wrote:
>
>> Hi folks, I was going to write this in django developers, but looks like
>> I might have found a small area for improvement for the async_unsafe
>> decorator, but first let me give you a bit of background:
>>
>> We're upgrading to Django 3.0 and we've been encountering this error on
>> ci when running all the tests: "OSError: [Errno 24] Too many open files".
>>
>> Here's the full stack trace:  https://dpaste.com/ABYJFP6CR
>>
>> By inspecting it and the source code it seems to be related to the
>> async_unsafe decorator:
>> https://github.com/django/django/blob/855fc06236630464055b4f9ea422c68a07c6d02a/django/utils/asyncio.py#L19
>>
>> This decorator calls `get_event_loop`:
>> https://github.com/python/cpython/blob/3.7/Lib/asyncio/events.py#L632
>>
>> which creates a new loop every time, which seems to be the cause of the
>> too many files open. Wouldn't be best to use `get_running_loop`
>> https://github.com/python/cpython/blob/3.7/Lib/asyncio/events.py#L681 ?
>>
>> We are already checking for RuntimeError so it should be an easy fix, but
>> I wonder if the usage of `get_event_loop` is intentional and I'm missing
>> something here.
>>
>> Happy to provide a patch for this :)
>>
>> Patrick
>>
>> --
>> 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/a7cedcc5-b041-4f21-9a22-e9ffb92d3c91n%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/CAMyDDM2_jHz54gfwcqwib9G%2BfxKxh9HER8dYJjJ4G1hNxvysuw%40mail.gmail.com
> 
> .
>


-- 
Patrick Arminio

-- 
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/CAOUxZct1YqydzG%3DB8hkzq1WFUhYFrjq7Ag3xw-3hDmSr79HBjw%40mail.gmail.com.

Re: Proposal: Drop dependency on pytz in favor of zoneinfo

[smi...@gmail.com]

Hi All,

While I understand the desire to have an early opt-in for some I think the 
important question here is the deprecation warnings. The recent URL() 
change showed that no matter how long there is a new way some?/many? folk 
won't change until they need to. 

Nick -- if we introduced a breaking change in 4.0, would that not have the 
same impact on folk upgrading to 4.2LTS from 3.2LTS as that which Carlton 
is concerned about (3.2 from 2.2), albeit a few years further into the 
future. 


David

On Thursday, 8 October 2020 at 09:08:50 UTC+1 jure.er...@gmail.com wrote:

> I would definitely be in favor of an opt-in: it would give developers time 
> to move to the new system at their convenience.
>
> Example: we're about to try and tackle the TZ issue in our apps and we 
> want to do it "globally" with one definitive solution. I'd much rather do 
> it on a library that is currently favoured, but not yet default than on a 
> deprecated one, even if it's not yet officially deprecated. We do have some 
> "import pytz", but currently they are few. Once we have a proper approach 
> to handling timezone stuff, there's likely going to be more of them... or 
> less, depending on the solution ;-)
>
> LP,
> Jure
> On 7. 10. 20 17:25, Paul Ganssle wrote:
>
> This sounds like a reasonable timeline to me. I think the breakage will be 
> relatively small because I suspect many end-users don't really even know to 
> use `normalize` in the first place, and when introducing the shim into a 
> fundamental library at work I did not get a huge number of breakages, but I 
> am still convinced that it is reasonably categorized as a breaking change.
>
> I do think that there's one additional stage that we need to add here (and 
> we chatted about this on twitter a bit), which is a stage that is fully 
> backwards compatible where Django supports using non-pytz zones for users 
> who bring their own time zone. I suspect that will help ease any breaking 
> pain between 3.2 and 4.0, because no one would be forced to make any 
> changes, but end users could proactively migrate to zoneinfo for a smoother 
> transition.
>
> I think most of what needs to be done is already in my original PR, it 
> just needs a little conditional logic to handle pytz as well as the shim.
>
> I am not sure how you feel about feature flags, but as a "nice to have", I 
> imagine it would also be possible to add a feature flag that opts you in to 
> `zoneinfo` as time zone provider even in 3.2, so that people can jump 
> straight to the 5.0 behavior if they are ready for it.
>
> I should be able to devote some time to at least the first part — making 
> Django compatible with zoneinfo even if not actively using it — but likely 
> not for a few weeks at minimum. If anyone wants to jump on either of these 
> ahead of me I don't mind at all and feel free to ping me for review.
>
> Best,
> Paul
> On 10/7/20 10:48 AM, Carlton Gibson wrote:
>
> Hi Paul.  
>
> Thanks for the input here, and for your patience 
>
> > I am fairly certain this is going to be a tricky migration and will 
> inevitably come with *some* user pain. I don't think this will be Python 
> 2 → 3 style pain, but some users who have been doing the "right thing" with 
> pytz will need to make changes to their code in the long run, which is 
> unfortunate.
>
> Looking at all the docs, your migration guide on pytz_deprecation_shim, 
> the example Kevin gave , 
> where we do some arithmetic in a local timezone, and call `normalize()` in 
> case we crossed a DST boundary, there's no way we can do this without 
> forcing a breaking change somewhere.
>
> So, probably, I've been staring at this too long today, but I think we 
> should introduce the shim in Django 4.0. Django 3.2, the next major release 
> will be an LTS. If we hold-off introducing the change until 4.0, we can 
> flag it as a breaking change in the 4.0 release notes, with big warnings, 
> allowing folks extra time to hang out on the previous LTS if they need it. 
>
> What I wouldn't want to do is to bring the breaking change in in Django 
> 3.2, because we'll have a whole load of folks updating from the 2.2 LTS at 
> about the time when it goes End of Life, and with no warning, that'd be a 
> hard breaking change to throw on top of their other issues. 
>
> We'd keep the shim in place for the entire 4.x series, removing in Django 
> 5.0 as per the deprecation policy 
> 
> .
>
> I think the advantages of doing it this way are two-fold: 
>
> * We allow people to focus on the semantic breaking change (in folds) 
> separately from the code changes per se — the logic may have changed 
> slightly in these cases, but it'll still run. 
> * It looks easier to migrate Django's code vs branching on a new setting. 
> (I didn't think through exactly what that might look like, so happy to see 
> a PoC from anyone.)
>
> I'm