Modify

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#9223 closed defect (fixed)

Plug-in is using wrong time values

Reported by: netjunki Owned by: 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 (1)

wikireplaceplugin-dateformatfix.patch (921 bytes) - added by netjunki 3 years ago.
Date format patch

Download all attachments as: .zip

Change History (9)

Changed 3 years ago by netjunki

Date format patch

comment:1 Changed 3 years ago 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.

comment:2 Changed 3 years ago by netjunki

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

comment:3 follow-up: Changed 3 years ago by miezuit

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

comment:4 in reply to: ↑ 3 Changed 3 years ago 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.

comment:5 Changed 3 years ago by miezuit

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

comment:6 Changed 3 years ago by miezuit

  • Resolution set to fixed
  • Status changed from new to closed

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)

comment:7 Changed 3 years ago 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.

comment:8 Changed 3 years ago 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 Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from miezuit. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.