Modify

Opened 3 years ago

Closed 3 years ago

#8959 closed defect (fixed)

[PATCH] trac 0.11 milestone dates are NOT microseconds

Reported by: bof Owned by: ChrisNelson
Priority: high Component: TracJsGanttPlugin
Severity: normal Keywords:
Cc: bobrien@… Trac Release: 0.11

Description

Testing this plugin under 0.11, I experienced at first strange browser-eats-all-my-memory phenomena, happening both with Firefox and Chrome. I'm talking about trying to take 1 GB of memory within 10 seconds + crashing because of that.

Some testing revealed that that happened because the JS attempted to display all days from 1/1/1970 until today, because it appeared to see milestone start/end dates of 1/1/1970.

These milestones do have end dates correctly set, i.e. not == 0

Inspecting the code, I found out that it unconditionally expects milestone dates to be in microsecond format. However, for trac 0.11, that is not the case - they are seconds-since-the-epoch. Dividing these by 1000000, gives a timestamp very near 1/1/1970......

The appended patch tries to handle the format difference heuristically, by looking at the timestamp value, and only dividing by 1000000 when it is smaller than 232-1.

Attachments (2)

tracjsganttplugin-0.11-dateformat-fix.patch (1021 bytes) - added by bof 3 years ago.
heuristically cope with milestone date in seconds and microseconds format
tracjsganttplugin-t8959-r10394.patch (1.2 KB) - added by rjollos 3 years ago.

Download all attachments as: .zip

Change History (13)

Changed 3 years ago by bof

heuristically cope with milestone date in seconds and microseconds format

comment:1 Changed 3 years ago by ChrisNelson

  • Status changed from new to assigned

Thanks for the patch! I'll try to merge it soon.

comment:2 Changed 3 years ago by bof

description should read "larger" instead of "smaller", in the last line...

patch is correct :)

comment:3 Changed 3 years ago by rjollos

This is a duplicate of #8743, but since there is a patch in this ticket I'll close the other one.

comment:4 in reply to: ↑ description ; follow-up: Changed 3 years ago by rjollos

Replying to bof:

Testing this plugin under 0.11, I experienced at first strange browser-eats-all-my-memory phenomena, happening both with Firefox and Chrome. I'm talking about trying to take 1 GB of memory within 10 seconds + crashing because of that.

I've confirmed this behavior. I'm guessing the plugin author must be developing under Trac 0.12 and this hasn't been extensively tested with 0.11.

I have a patch that makes better use of the Trac API, which I will post shortly.

comment:5 follow-up: Changed 3 years ago by rjollos

I haven't tested this patch with 0.12, but it should work fine because it makes use of the Trac API which will handle the differences in how timestamps are stored between Trac 0.11 and 0.12.

In general, we can use trac.util.datefmt to simplify the code and fix potential subtle timezone related issues that might be present. I'll open a ticket for that down the road and provide another patch.

Note: This patch includes the patch in #8859.

Changed 3 years ago by rjollos

comment:6 in reply to: ↑ 5 Changed 3 years ago by bof

Replying to rjollos:

I haven't tested this patch with 0.12, but it should work fine because it makes use of the Trac API which will handle the differences in how timestamps are stored between Trac 0.11 and 0.12.

I have tested your patch (instead of my heuristic) on both 0.11.6 and 0.12.2,
and can confirm that it fixes the milestone date handling issue for both.

Great simplification, thank you! Chris, please use that one instead of my hack.

comment:7 in reply to: ↑ 4 ; follow-up: Changed 3 years ago by ChrisNelson

Replying to rjollos:

Replying to bof:

Testing this plugin under 0.11, I experienced at first strange browser-eats-all-my-memory phenomena, happening both with Firefox and Chrome. I'm talking about trying to take 1 GB of memory within 10 seconds + crashing because of that.

I've confirmed this behavior. I'm guessing the plugin author must be developing under Trac 0.12 and this hasn't been extensively tested with 0.11.

Actually, we are on 0.11.6 but we have applied the microsecond patch so we haven't tested extensively with 0.11.6 default timestamps.

I have a patch that makes better use of the Trac API, which I will post shortly.

I looked at the patch. In

-                    finish.strftime(self.pyDateFormat)
+                    format_date(ts, self.dbDateFormat)
  1. Does format_date() handle None as today?
  2. Why is dbDateFormat the right thing to use?

comment:8 Changed 3 years ago by ChrisNelson

  • Cc bobrien@… added

comment:9 in reply to: ↑ 7 Changed 3 years ago by rjollos

Replying to ChrisNelson:

  1. Does format_date() handle None as today?

Yes, exactly.

  1. Why is dbDateFormat the right thing to use?

You want the milestones's finish date to be in the same format as the finish dates for your tickets when both are passed to _schedule_tasks and then _finish, because there you do are expecting a date in dbDateFormat (line 420): f = datetime.datetime(*time.strptime(succ[self.fields['finish']], self.dbDateFormat)[0:7]).

It works fine if pyDateFromat = dbDateFormat (which is the default behavior if date_format is not specified in trac.ini), but if they are different, then with r10394 of the code an error results.

You can reproduce the issue by setting up a test environment with date_format set to anything other than %Y-%m-%d, and setting some milestone due dates.

comment:10 Changed 3 years ago by ChrisNelson

(In [10407]) Handle milestone dates better. Refs #8959.

No longer assume that milestone dates are microseconds. Use Trac API
to do version-indenpendent conversion.

Patch thanks to rjollos.

comment:11 Changed 3 years ago by ChrisNelson

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

Add Comment

Modify Ticket

Action
as 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.