Modify

Opened 6 years ago

Closed 6 years ago

#8959 closed defect (fixed)

[PATCH] trac 0.11 milestone dates are NOT microseconds

Reported by: Patrick Schaaf Owned by: Chris Nelson
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 Patrick Schaaf 6 years ago.
heuristically cope with milestone date in seconds and microseconds format
tracjsganttplugin-t8959-r10394.patch (1.2 KB) - added by Ryan J Ollos 6 years ago.

Download all attachments as: .zip

Change History (13)

Changed 6 years ago by Patrick Schaaf

heuristically cope with milestone date in seconds and microseconds format

comment:1 Changed 6 years ago by Chris Nelson

Status: newassigned

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

comment:2 Changed 6 years ago by Patrick Schaaf

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

patch is correct :)

comment:3 Changed 6 years ago by Ryan J Ollos

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 ; Changed 6 years ago by Ryan J Ollos

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 Changed 6 years ago by Ryan J Ollos

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 6 years ago by Ryan J Ollos

comment:6 in reply to:  5 Changed 6 years ago by Patrick Schaaf

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 ; Changed 6 years ago by Chris Nelson

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 6 years ago by Chris Nelson

Cc: bobrien@… added; anonymous removed

comment:9 in reply to:  7 Changed 6 years ago by Ryan J Ollos

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 6 years ago by Chris Nelson

(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 6 years ago by Chris Nelson

Resolution: fixed
Status: assignedclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Chris Nelson.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


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

 
Note: See TracTickets for help on using tickets.