Ticket #10748 (closed enhancement: fixed)

Opened 5 months ago

Last modified 4 months ago

For optimal performance req.chrome should be evaluated only if necessary.

Reported by: olemis Assigned to: olemis
Priority: normal Component: ThemeEnginePlugin
Severity: minor Keywords: chrome css script
Cc: peter@digiverse.si, gmartin Trac Release: 1.0

Description

When the plugin is installed and active it's post_process_request() will be invoked for every request. AFAICT there is nothing in that code that would differentiate requests (API vs UI requests) so add_stylesheet() will be called for all of them which then calls add_link() that evaluates req.chrome.

mentioned in BH:comment:3:ticket:330

Attachments

Change History

01/04/13 01:12:25 changed by olemis

  • status changed from new to assigned.

01/04/13 01:20:05 changed by olemis

  • type changed from defect to enhancement.

(In [12508]) ThemeEnginePlugin : Do not access req.chrome while filtering requests if template = data = None ( refs #10748 )

(follow-up: ↓ 6 ) 01/04/13 13:44:37 changed by rjollos

  • cc changed from peter@digiverse.si to peter@digiverse.si, gmartin.

I cleaned up the reference in your commit message :)

One minor, almost irrelevant comment that I'd like to get your thoughts on. PEP8 states: Comparisons to singletons like None should always be done with is or is not, never the equality operators.

So I was thinking that would suggest the following change: 

if (template, data) != (None, None):

-> 

if (template, data) is not (None, None)

(I wish individual bullet points on PEP8 had anchors so that we could link to them individually. The section headings have anchors, but even for those the browser doesn't seem to jump to the right location [at least in Chrome 23, try: http://www.python.org/dev/peps/pep-0008/#id37, it should go Programming Recommendations section]).

(in reply to: ↑ 5 ) 01/04/13 16:37:03 changed by olemis

Replying to rjollos:

...

So I was thinking that would suggest the following change: {{{ #!python if (template, data) != (None, None): }}} -> {{{ #!python if (template, data) is not (None, None) }}}

Bad idea , mainly because of this 

>>> (None, None) is (None, None)
False

...

01/04/13 16:39:54 changed by rjollos

Ah, right, because in the latter case it will be testing if the two references are the same. Thanks for the clarification.

01/17/13 15:52:38 changed by olemis

(In [12534]) ThemeEnginePlugin: [regression] Theme struct will be None if no theme configured or set to 'default' - refs #10748 fixes #10800

01/17/13 15:59:21 changed by olemis

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

This is done. Please reopen if anything weird is detected .

01/19/13 21:50:26 changed by olemis

(In [12540]) ThemeEnginePlugin: Let theme post-process requests while rendering error pages - after [12508] refs #10748 fixes #10809

Add/Change #10748 (For optimal performance req.chrome should be evaluated only if necessary.)




Change Properties
Action