Shouldn't we supply old and new values?
Translating values introduces a new subtle issue: the real value
could remain unchanged, but the token could be different eg changing
from ${FOO} to ${BAR} with both FOO and BAR equal to the same value.
Lloyd
On Oct 8, 2009, at 9:15 PM, 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.
>>>
>>> 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.
>>>
>>> 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
>
Lloyd Chambers
lloyd.chambers_at_sun.com
GlassFish Team