Opened 16 years ago
Closed 15 years ago
#8240 closed defect (fixed)
Can not set completion date on milestone due to timezone daylight saving
Reported by: | Owned by: | Remy Blank | |
---|---|---|---|
Priority: | normal | Milestone: | 0.11.6 |
Component: | ticket system | Version: | 0.11.4 |
Severity: | normal | Keywords: | |
Cc: | info@…, michaelc@…, jappie@… | Branch: | |
Release Notes: | |||
API Changes: | |||
Internal Changes: |
Description
If I want to set a completion date for a milestone very close to the actual time, I get the warning message: "Warning: Completion date may not be in the future". For example at '04/27/09 15:30:00' I can not put a completion date of a milestone which is '04/27/09 15:00:00'.
I think this is something closely related to trac's/Python's datetime handling with timezones (the culprit is probably 'trac.util.datefmt.parse_date').
This is my setup:
- Trac 0.11.4 (seeing this with 0.11stable as well)
- Python 2.3 (also checked with Python 2.5)
- pytz is installed
- server timezone (default_timezone in trac.ini) is set to utc
- in the user preferences a different timezone was stored, in that timezone daylight saving is currently active (e.g. Europe/Berlin)
What happens is that parse_date returns a date with the non-dst timezone. Therefore the computed time is off by one hour and compared to UTC it looks like it is a date in the future (because trac only deducts 1h instead of 2h in Europe/Berlin (CEST → GMT+2).
This does not happen if pytz is not installed because trac's timezones don't imply any daylight saving settings.
You can add a workaround for the specific issue if you just remove the check for future days. However this still affects plugin writers.
I think the correct way to deal with this is to use the normalize()/localize() method of a pytz tzinfo object on the generated datetime. I think something like that should be added (branches/0.11-stable/trac/util/datefmt.py#L233):
if not hasattr(tzinfo, 'localize'): # This is a tzinfo define by trac which don't have to deal with dst dt = datetime(*(tm[0:6] + (0, tzinfo))) else: # We need to detect daylight saving correctly - see #... dt = tzinfo.localize(datetime(*tm[0:6]))
Attachments (1)
Change History (12)
comment:1 by , 16 years ago
Milestone: | → 0.11.5 |
---|---|
Owner: | set to |
comment:2 by , 16 years ago
I think this should be a fairly simply change if we assume that all datetime instances - once built - contain the correct timezone. So I think the only functions to change are to_datetime and parse_date in datefmt.py.
comment:3 by , 15 years ago
Cc: | added |
---|
comment:4 by , 15 years ago
Cc: | added |
---|
follow-up: 6 comment:5 by , 15 years ago
Cc: | added |
---|
I think this should be a fairly simply change if we assume that all datetime instances - once built - contain the correct timezone. So I think the only functions to change are to_datetime and parse_date in datefmt.py.
Is there an indication to if and when such fix will be made?
comment:6 by , 15 years ago
Replying to Jappie <jappie@…>:
Is there an indication to if and when such fix will be made?
Sorry for not giving additional information!
trac: 0.11.5
python: 2.6.2-r1
pytz: 2009j
timezone: Europe/Amsterdam
I have the same problem as reported by Felix Schwarz. The patch Felix Schwarz sugests works perfectly for me. I haven't encountered a related issue in Trac yet, so I cannot verify if the second solution will work.
comment:7 by , 15 years ago
The pytz docstring for tzinfo.localize()
is quite explicit about this:
Convert naive time to local time. This method should be used to construct localtimes, rather than passing a tzinfo argument to a datetime constructor.
So I think Felix' suggestion is correct: we should always construct "naive" datetime
objects (i.e. without a tzinfo
), then pass them to tzinfo.localize()
. I have a slightly more elegant solution than the proposed patch. I also have a test case that fails with the current implementation, and passes when tzinfo.localize()
is used. Patch follows.
by , 15 years ago
Attachment: | 8240-tzinfo-localize-r8816.patch added |
---|
Use tzinfo.localize()
when constructing datetime
objects.
comment:9 by , 15 years ago
Owner: | changed from | to
---|
comment:10 by , 15 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Following the consistent failures on Tim's buildbot, I have investigated this some more, and I can reproduce the issue by switching my timezone to PST. I'm working on a fix.
comment:11 by , 15 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
I was a bit too enthusiastic with my changes from [8831]. Only the calls to the datetime
constructor must be wrapped in tzinfo.localize()
, not the calls to now()
or fromtimestamp()
.
I didn't notice the failure, because the one test case that tested it was using exactly my timezone. The issue is fixed in [8857], with an additional test to avoid this special case.
Yes, supporting the DST correctly is definitely needed (see also #5895).
As the pytz documentation states:
So we should rather find a way to consistently use localize/normalize in to_datetime/to_timestamp and related places.