[Django] #29514: Regression in get_default_timezone()
Django
-------------------------------------+-------------------------------------
Reporter: Carlton | Owner: Carlton Gibson
Gibson |
Type: Bug | Status: assigned
Component: Utilities | Version: 2.1
Severity: Release | Keywords:
blocker |
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
[https://github.com/encode/django-rest-framework/pull/6044 CI testing on
Django REST Framework] revealed a change in behaviour in
`timezone.get_default_timezone()`.
The essence is:
{{{
with override_settings(USE_TZ=True, TIME_ZONE='UTC'):
self.assertIs(timezone.get_default_timezone(), timezone.utc)
}}}
This passes fails 2.0.x and (currently) fails on 2.1/master.
Regression in 27ca5ce19f5f184018a61611c1bc319113b1d107.
--
Ticket URL: <https://code.djangoproject.com/ticket/29514>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
| Gibson
Type: Bug | Status: assigned
Component: Utilities | Version: 2.1
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/29514#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Carlton Gibson | Owner: Carlton
Type: Bug | Status: assigned
Component: Utilities | Version: 2.1
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Sergey Fedoseev (added)
Comment:
I'm not sure. It seems reasonable to fix the problem but having
`get_default_timezone()` return a different type for a one-off case might
lead to some issue.
While investing, I noticed that `pytz.utc` is
[https://github.com/stub42/pytz/blob/9643b268e1b2df822bed3dfa134c90bd56275f4e/src/pytz/__init__.py#L194
described] as "Optimized UTC implementation." yet the
[https://github.com/django/django/pull/9484 offending commit] in Django
says "Used datetime.timezone.utc instead of pytz.utc for better
performance."
--
Ticket URL: <https://code.djangoproject.com/ticket/29514#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Carlton Gibson | Owner: Carlton
Type: Bug | Status: assigned
Component: Utilities | Version: 2.1
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
The issue is also present anywhere a timezone is constructed with
`pytz.timezone()`, e.g.
[https://github.com/django/django/blob/7cdeb23ae7ef2d125276840ff298dbb9683959f2/django/utils/timezone.py#L121
timezone.activate()]:
{{{
from django.conf import settings
settings.configure(USE_TZ=True)
from django.utils.timezone import get_current_timezone, utc
from django.utils import timezone
timezone.activate('UTC')
get_current_timezone() is utc # False in Django 2.1, True in older
versions
}}}
If we accept the proposed patch, I'd think we would also want to fix the
issue there, and perhaps add a backwards incompatible release note for
anyone using `pytz.timezone()` in their code. It might be better to
reconsider the original patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/29514#comment:3>
Django
Reporter: Carlton Gibson | Owner: (none)
Type: Bug | Status: new
Component: Utilities | Version: 2.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* owner: Carlton Gibson => (none)
* status: assigned => new
Comment:
I'm going to (temporarily) deassign myself here, as I will be unable to
work on this for the next week. Happy to pick it up if it's still pending
after that.
--
Ticket URL: <https://code.djangoproject.com/ticket/29514#comment:4>
Django
Reporter: Carlton Gibson | Owner: Tim
| Graham <timograham@…>
Type: Bug | Status: closed
Component: Utilities | Version: 2.1
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"f4ef71c689b7bf3b0237d12c634fe5557f646a79" f4ef71c6]:
{{{
#!CommitTicketReference repository=""
revision="f4ef71c689b7bf3b0237d12c634fe5557f646a79"
Refs #29514 -- Added test for get_default_timezone()/timezone.utc
equality.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29514#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Carlton Gibson | Owner: Tim
| Graham <timograham@…>
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: (none) => Tim Graham <timograham@…>
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"2ec151e35da93047acfeea1b79c27010f2cb8594" 2ec151e3]:
{{{
#!CommitTicketReference repository=""
revision="2ec151e35da93047acfeea1b79c27010f2cb8594"
Fixed #29514 -- Reverted "Used datetime.timezone.utc instead of pytz.utc
for better performance."
This reverts commit 27ca5ce19f5f184018a61611c1bc319113b1d107 due to a
regression.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29514#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Carlton Gibson | Owner: Tim
| Graham <timograham@…>
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Daniel Hahler):
Just for information:
> While investing, I noticed that pytz.utc is described as "Optimized UTC
implementation." yet the offending commit in Django says "Used
datetime.timezone.utc instead of pytz.utc for better performance."
The performance improvement basically comes from datetime being
implemented in C completely, while using ``pytz.utc`` calls into Python
code (at least) 2 times (for `utcoffset`/`dst`).
--
Ticket URL: <https://code.djangoproject.com/ticket/29514#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Carlton Gibson | Owner: Tim
| Graham <timograham@…>
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"278457988a976e64dabe63ca410315b2d06aacfe" 2784579]:
{{{
#!CommitTicketReference repository=""
revision="278457988a976e64dabe63ca410315b2d06aacfe"
[2.1.x] Refs #29514 -- Added test for get_default_timezone()/timezone.utc
equality.
Backport of f4ef71c689b7bf3b0237d12c634fe5557f646a79 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29514#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Carlton Gibson | Owner: Tim
| Graham <timograham@…>
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"3a6040246f16f92c7a45e606822d6940d94eae2a" 3a604024]:
{{{
#!CommitTicketReference repository=""
revision="3a6040246f16f92c7a45e606822d6940d94eae2a"
[2.1.x] Fixed #29514 -- Reverted "Used datetime.timezone.utc instead of
pytz.utc for better performance."
This reverts commit 27ca5ce19f5f184018a61611c1bc319113b1d107 due to a
regression.
Backport of 2ec151e35da93047acfeea1b79c27010f2cb8594 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29514#comment:8>