Two notes below.
Jerome Dochez wrote:
> kedar wrote:
>>
>>
>> Jerome Dochez wrote:
>>> This is something that would be pretty easy to change in the config
>>> layer but I am a little scared about possible regressions by
>>> changing this now. I am surprised we did not have that raised before !
>>>
>>> Kedar, I know everything can be potentially tokenized but
>>> realistically how much do we have tokenized right now (i.e. who will
>>> be potentially impacted by a change a behaviour).
>>
>> I agree. But the problem right now is that listeners do something like:
>> listener.getPort() and expect it to be a number. So, all listeners
>> should be changed to call the service to translate the raw values,
>> right?
>>
>> Besides, there is a correctness issue here. When I have:
>>
>> <system-property name="PORT" value="1234"/>
>> ....
>> <network-listener port="${PORT}"/>,
>>
>> when we restart, listener.getPort() returns 1234 correctly, but in case
>> of dynamic reconfig the same call returns "${PORT}". Is that ok?
>>
> I understand perfectly that it is the problem. I suppose we should
> look at all ConfigListener implementations to ensure that they are not
> already doing translation of raw values because if we fix it, we will
> break those.
If the event's old and new values are changed to be translated instead
of raw (as they are now), how would that break existing listeners that
also translate placeholders? Those listeners would need to be looking
for placeholders to replace and they would find none if the old and new
values suddenly become translated, right?
>>>
>>> I think the safest of course is to keep the current behaviour and
>>> change it post release if we can. There is a service that be can be
>>> used by config listeners to translate raw values.
And what service is that?? :-)
- Tim
>>>
>>> jerome
>>>
>>> Tim Quinn wrote:
>>>> Kedar Mhaswade wrote:
>>>>> Tim Quinn wrote:
>>>>>> If the user changes a config value (I tried using the admin
>>>>>> console) that contains a token (such as
>>>>>> ${com.sun.aas.instanceRoot}) then the event delivered to the
>>>>>> "change" method returns the untranslated old and new values,
>>>>>> still containing the tokens.
>>>>>>
>>>>>> Maybe this has been discussed before and I've forgotten... Is
>>>>>> there something the "change" method can do to translate the
>>>>>> values? It seems that the injected configuration object has
>>>>>> already been updated with the translated new value. Is that
>>>>>> expected and reliable?
>>>>>> Thanks.
>>>>>
>>>>> Tim,
>>>>>
>>>>> Incidentally, I had some conversation with Jerome on this just
>>>>> yesterday
>>>>> and I think it is an issue.
>>>>>
>>>>> IMO, The config layer should already resolve (expand) the variable
>>>>> in the configuration object given to the changed method in
>>>>> ConfigListener
>>>>> so that the runtime code does not have to do anything there.
>>>> That's how it is now...at least that's what I saw.
>>>>>
>>>>> Alternatively, the variable expansion is just doing a replacement
>>>>> from
>>>>> system properties. So in the changed method, if a particular
>>>>> attribute
>>>>> contains {xxx} you could replace it with System.property(xxx). I have
>>>>> tried it and it seems to work.
>>>> I know that the changed method accepts a
>>>> java.beans.PropertyChangeEvent argument. If only it were an
>>>> org.jvnet.hk2.config.PropertyChangeEvent we could have exposed
>>>> getRaw[Old | New]Value methods, for example. Maybe there could be
>>>> a utility method so this could be implemented once instead of in
>>>> each ConfigListener?
>>>> This would be especially helpful if, as Lloyd said, the translation
>>>> should include the <system-property> settings - or are those
>>>> already set so they are accessible via System.getProperty()?
>>>>>
>>>>> I am going to spend some time on making the injected configuration
>>>>> object have the variable-expanded values though.
>>>> But it seems to do that already. Am I missing something?
>>>>
>>>> - Tim
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
>>> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
>> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>