incubator-bloodhound-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Olemis Lang <>
Subject Re: [Whitelabeling] Quick assessment of (current) whitelabelling implementation .
Date Wed, 19 Dec 2012 17:04:29 GMT
On 12/19/12, Peter Koželj <> wrote:
>> If my comment above is accurate : no need to do nothing of this . This
>> is happening every day , every where in Trac
>> Maybe I didn't understand the initial statement about option defaults .
> We were not on the same page here :( as I misunderstood  your initial
> observation.
> I was considering env.config.options to automatically enumerate labels

all labels at once ? well if that's the case you just cannot do that .
I was thinking of options one-by-one

> but
> then I had nowhere to put the defaults.

now it seems I understand .

> Anyway this has nothing to do with your observation and I apologize  for
> confusion.

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 .

>> Please review the implementation of
>> method and request callbacks . Using data may lead to clashes with
>> other active request handlers
> Reviewed, it was an useful exercise :)

... and painful ... I know . the issue fixed in r

> 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 ;)

> - 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
>, 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 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 ?



Blog ES:
Blog EN:

Featured article:

View raw message