incubator-bloodhound-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Koželj <pe...@digiverse.si>
Subject Re: [Whitelabeling] Quick assessment of (current) whitelabelling implementation .
Date Fri, 21 Dec 2012 10:02:26 GMT
On 19 December 2012 18:04, Olemis Lang <olemis@gmail.com> wrote:

> On 12/19/12, Peter Koželj <peter@digiverse.si> wrote:
> nevermind :)
>
> > I can use trac.config.Option instead of self.env.config.get if this is
> the
> > preferred way.
> > So is trac.config.Option the preferred way? Is if so is that just a
> > convention or are there deeper technical reasons to prefer one over
> > another?
> >
>
> beyond options docs and defaults , the major benefit I can see is that
> these become instance methods too => more readable code .
>

Fixed.


>
> > My conclusions:
> > - data needs to be dict, but of corse that does not mean it is an
> > appropriate holder for the labels by itself
>
> ... and in post_process_request it may be None too ;)
>
> > - req.chrome is always evaluated by Chrome.render_template, but I agree
> > that the same should be avoided by request filter methods
> >
>
> yes , but sometimes no template is rendered at all while processing a
> request e.g. RPC requests do not use chrome at ... why bother
> instantiating it in post process request ?
>
> ;)
>
> > So if we don't want to add it to data or force lazy evaluation of
> > req.chrome, the only thing that remains is a new property req.labels as:
> >
> > {{{
> >
> > def post_process_request(self, req, template, data, content_type):
> >     ...
> >     req.callbacks['labels'] = lambda req: self._get_whitelabelling()
> >     ...
> >
> > }}}
> >
> > This works but doing req.labels['whatever'] does leave a bit strange
> taste
> > in my mouth as it seems out of place.
> >
>
> Yes , I confess the same happens to me . If req.chrome wasn't such a
> problematic feature it would be the right place to include 'labels' .
>
> Question : Instead of post-processing request by injecting white
> labeling data somewhere is it possible to change this at stream
> filters level instead ?
>
> If so neither chrome nor data (and related complications) would be
> needed . Instance methods would be more than enough . What do you
> think ?
>

Actually adding labels to req.chrome in lazy manner is a bit messy but not
that difficult at all. What I tried and works is:

{{{

         if 'chrome' in req.__dict__:

            req.chrome['labels'] = self._get_whitelabelling()

        else:

            original_chrome_callback = req.callbacks['chrome']

            def chrome_with_labels_callback(req):

                chrome = original_chrome_callback(req)

                chrome['labels'] = self._get_whitelabelling()

                return chrome

            req.callbacks['chrome'] = chrome_with_labels_callback

}}}

Unfortunately this doesn't make any difference at all as req.chrome is
evaluated at a few other places like:
 - multiple places in bloodhound_theme and (this should be easily fixable
if we know how to distinguish RPC from UI templates calls)
 - multiple places in TracThemeEngine (this is not fixable without changes
to the plugin or maybe Trac or even both)

Apparently the scope of this issue is bigger then whitelabeling and should
be approached with all aspects of request handling in mind.
Optimally the RPC requests should probably not include BloodhoundTheme or
TracThemeEngine at all so I am leaving the whitelabeling solution as is.

Lp,
Peter

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message