Ticket #8959 (closed defect: fixed)

Opened 2 years ago

Last modified 2 years ago

[PATCH] trac 0.11 milestone dates are NOT microseconds

Reported by: bof Assigned to: ChrisNelson
Priority: high Component: TracJsGanttPlugin
Severity: normal Keywords:
Cc: bobrien@newtonrobotics.com 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

tracjsganttplugin-0.11-dateformat-fix.patch (1.0 kB) - added by bof on 07/08/11 11:51:14.
heuristically cope with milestone date in seconds and microseconds format
tracjsganttplugin-t8959-r10394.patch (1.2 kB) - added by rjollos on 07/09/11 06:25:34.

Change History

07/08/11 11:51:14 changed by bof

  • attachment tracjsganttplugin-0.11-dateformat-fix.patch added.

heuristically cope with milestone date in seconds and microseconds format

07/08/11 13:45:52 changed by ChrisNelson

  • status changed from new to assigned.

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

07/08/11 15:34:18 changed by bof

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

patch is correct :)

07/09/11 05:22:44 changed by rjollos

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

(in reply to: ↑ description ; follow-up: ↓ 7 ) 07/09/11 06:15:02 changed 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.

(follow-up: ↓ 6 ) 07/09/11 06:23:59 changed 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.

07/09/11 06:25:34 changed by rjollos

  • attachment tracjsganttplugin-t8959-r10394.patch added.

(in reply to: ↑ 5 ) 07/09/11 10:26:27 changed 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.

(in reply to: ↑ 4 ; follow-up: ↓ 9 ) 07/09/11 19:49:13 changed 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?

07/09/11 19:52:37 changed by ChrisNelson

  • cc set to bobrien@newtonrobotics.com.

(in reply to: ↑ 7 ) 07/09/11 22:06:50 changed 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.

07/10/11 00:33:09 changed 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.

07/10/11 00:34:25 changed by ChrisNelson

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

Add/Change #8959 ([PATCH] trac 0.11 milestone dates are NOT microseconds)




Change Properties
Action