Opened 15 years ago

Closed 11 years ago

#2386 enhancement closed fixed (fixed)

Change log timestamp callLater thing to be attached to factory instead of module

Reported by: itamarst Owned by: lvh
Priority: normal Milestone:
Component: web Keywords: easy
Cc: ghazel, lvh Branch: branches/remove-global-log-2386-2
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun, lvh

Description

The twisted.web speed optimization for timestamps could be made more trial-friendly by making it run by server.Site, rather than on import.

Attachments (3)

issue.2386.fix.diff (6.4 KB) - added by Zach 11 years ago.
Proposed fix
issue.2386.fix.2.diff (6.5 KB) - added by Zach 11 years ago.
Updated some documentation, definitely needs review
issue.2386.fix.3.diff (6.9 KB) - added by Zach 11 years ago.
New diff

Download all attachments as: .zip

Change History (23)

comment:1 Changed 15 years ago by Jean-Paul Calderone

Cc: ghazel added

#2447 is a duplicate of this, and includes a test case.

comment:2 Changed 15 years ago by Jean-Paul Calderone

Nevermind, #2447 is about DelayedCalls in web code, but not this DelayedCall.

comment:3 Changed 11 years ago by <automation>

Owner: jknight deleted

comment:4 Changed 11 years ago by Jean-Paul Calderone

Keywords: easy added

The timer can be started in the factory's startFactory and it can be cleaned up in stopFactory.

comment:5 Changed 11 years ago by Zach

Owner: set to Zach

Changed 11 years ago by Zach

Attachment: issue.2386.fix.diff added

Proposed fix

comment:6 Changed 11 years ago by Zach

Keywords: review added

comment:7 Changed 11 years ago by lvh

Owner: changed from Zach to lvh
Status: newassigned

comment:8 Changed 11 years ago by lvh

Author: lvh
Branch: branches/remove-global-log-2386

(In [31034]) Branching to 'remove-global-log-2386'

comment:10 Changed 11 years ago by lvh

Keywords: review removed
Status: assignednew

Okay, looks good to me, happy buildbots incoming and as soon as all of them are in I'll merge it :)

comment:11 Changed 11 years ago by lvh

Owner: changed from lvh to Zach

Err, except not really. Could you add a docstring explaining the new (private) attributes?

Changed 11 years ago by Zach

Attachment: issue.2386.fix.2.diff added

Updated some documentation, definitely needs review

comment:12 Changed 11 years ago by Zach

Keywords: review added
Owner: Zach deleted

comment:13 Changed 11 years ago by lvh

Cc: lvh added
Keywords: review removed
Owner: set to Zach

Okay on the method docstring but missing attribute docstrings.

Changed 11 years ago by Zach

Attachment: issue.2386.fix.3.diff added

New diff

comment:14 Changed 11 years ago by Zach

Keywords: review added
Owner: Zach deleted

comment:15 Changed 11 years ago by lvh

Keywords: review removed
Owner: set to lvh
Status: newassigned

Looking good! In the future please use more complete sentences, but I'll do it for you this time since otherwise I'm earning gajillions of points off of you ;-)

comment:16 Changed 11 years ago by Jean-Paul Calderone

Author: lvhexarkun, lvh
Branch: branches/remove-global-log-2386branches/remove-global-log-2386-2

(In [31236]) Branching to 'remove-global-log-2386-2'

comment:17 Changed 11 years ago by Jean-Paul Calderone

(In [31237]) Apply issue.2386.fix.3.diff

refs #2386

comment:18 Changed 11 years ago by Jean-Paul Calderone

(In [31238]) Rename _logDateTimeCall{back,}; insert some whitespace and do some docstring stuff

refs #2386

comment:19 Changed 11 years ago by Jean-Paul Calderone

I made a few more tweaks, just docstrings and whitespace and one attribute name. Build results look good, so I'll merge this now. Thanks steiza and lvh!

comment:20 Changed 11 years ago by Jean-Paul Calderone

Resolution: fixed
Status: assignedclosed

(In [31240]) Merge remove-global-log-2386-2

Author: steiza Reviewer: lvh, exarkun Fixes: #2386

Move the global timestamp cache from twisted.web.http into per-instance state on HTTPFactory.

Note: See TracTickets for help on using tickets.