Ticket #9223 (closed defect: fixed)

Opened 2 years ago

Last modified 2 years ago

Plug-in is using wrong time values

Reported by: netjunki Assigned to: miezuit
Priority: normal Component: WikiReplacePlugin
Severity: normal Keywords: patch
Cc: Trac Release: 0.12

Description

I was getting some weird results in the page history for a page in my trac instance where the pages were changed 42 years in the past. I was trying to track this down and ended up digging around in the database and noticed this: 

select name,version,time,comment from wiki where name='History';

History|17|1316672699697882|
History|18|1316981182|Replaced "History/PastDistantTimeline/CivilWarArtifact" with "History/PastDistantTimeline/CivilWarArtifactWhereabouts".
History|19|1316981203|Replaced "History/PresentTimeline/Calendar" with "History/Present/Calendar".
History|20|1316981223|Replaced "History/PresentTimeline/Pacing" with "History/Present/Pacing".
History|21|1316987293068044|

Notice how the time field is 6 digits too short? I prepared a patch that I haven't had a chance to test yet... but I think this should do it.

Attachments

wikireplaceplugin-dateformatfix.patch (0.9 kB) - added by netjunki on 10/02/11 03:57:06.
Date format patch

Change History

10/02/11 03:57:06 changed by netjunki

  • attachment wikireplaceplugin-dateformatfix.patch added.

Date format patch

10/02/11 12:30:24 changed by osimons

Note: Microseconds was just added for 0.12, so to remain compatible with 0.11 (which uses seconds) it could use simple compat handling like I do in XmlRpcPlugin: source:xmlrpcplugin/trunk/tracrpc/util.py (currently line 51-57).

This should ensure that the plugin always calculates timestamps correctly.

10/02/11 12:38:57 changed by netjunki

Neat. I'll prepare a patch with that change once I get access to my test system again.

(follow-up: ↓ 4 ) 10/02/11 23:28:44 changed by miezuit

(In [10716]) Added compatibility with Trac 0.12 (with microseconds support). The plugin also remains compatible with 0.11. Refs #9223.

(in reply to: ↑ 3 ) 10/02/11 23:34:01 changed by anonymous

Replying to miezuit:

The plugin also remains compatible with 0.11.

No, it won't be compatible as you try to import to_utimestamp directly. It will fail for 0.11.x (ImportError). to_timestamp (0.11) != to_utimestamp (0.12)... That is what my compat util code handles.

10/02/11 23:46:40 changed by miezuit

(In [10717]) Fixed compatibility with 0.11. Refs #9223.

10/02/11 23:52:34 changed by miezuit

  • status changed from new to closed.
  • resolution set to fixed.

Hi, thanks for the tip. I didn't notice.

I tested on Trac 0.11 and 0.12 and it work fine for both now.

In the patch provided by netjunki I had to replace the use of datetime.datetime.now() with trac.util.datefmt.to_datetime() - see #8235.

The following two are equivalent: 

datetime.now(req.tz)
to_datetime(None, req.tz)

10/03/11 00:07:21 changed by osimons

You may actually be better served by using to_datetime(None, utc) for all your internal times. Trac stores all timestamps as utc, and really only uses the req.tz for formatting the time back to the user. Using now from the location of the user will most likely deviate from what you actually need.

10/03/11 01:19:59 changed by osimons

Actually, did some simple tests that verify that using non-UTC input should work too - provided you go through timestamp conversion (my local timezone is CET, using current 0.12.x): 

>>> from trac.util.datefmt import to_datetime, to_timestamp, to_utimestamp, utc, localtz
>>> to_datetime(None, utc) - to_datetime(None, localtz)
datetime.timedelta(-1, 86399, 999978)             # ~2 hours
>>> to_timestamp(to_datetime(None, utc)) - to_timestamp(to_datetime(None, localtz))
0
>>> to_utimestamp(to_datetime(None, utc)) - to_utimestamp(to_datetime(None, localtz))
-22L                                              # ~0

So, at least timestamp calculation from user input seems to do the right thing.

Add/Change #9223 (Plug-in is using wrong time values)




Change Properties
Action