Tom, Andrei,
Do you have any comments, or should I incorporate this last change
and check in the code? Will anybody want to see the final code, if
this is the only change?
thanks,
-marina
Marina Vatkina wrote:
> Hi Wonseok,
>
> Thanks for looking into this. Yeah, I was also thinking about it.
> It might even remove the need for the 'init' param - it's only
> used to bypass updateSessionName() call. So it'll be like
> updateLoggers(m, serverPlatformChanged, false);
> and
> updateLoggers(m, serverPlatformChanged, sessionNameChanged);
>
> I'll wait for Tom and Andrei to see if they have any other comments,
> before making further changes.
>
> thanks,
> -marina
>
> Wonseok Kim wrote:
>
>> Marina,
>> Thanks for solving the puzzle of the logger class change!
>>
>> It looks good except for one thing.
>>
>> Now updateServerPlatform() and updateSessionName() are done in
>> updateLoggers(), but doesn't it look a little bit awkward?
>> updateLoggers() just need to know whether ServerPlatform and session
>> name change. It would be easy to understand the code intuitively if
>> updateServerPlatform() and updateSessionName() are called in
>> predeploy() and updateServerSession() like below.
>>
>> protected void updateServerSession(Map m) {
>> ...
>> // In deploy Session name and ServerPlatform could've changed
>> which will affect the loggers.
>> boolean serverPlatformChanged = updateServerPlatform(m);
>> boolean sessionNameChanged = updateSessionName(m);
>> updateLoggers(m, false, serverPlatformChanged,
>> sessionNameChanged);
>>
>> Cheers,
>> -Wonseok
>>
>> On 11/3/06, * Marina Vatkina* <Marina.Vatkina_at_sun.com
>> <mailto:Marina.Vatkina_at_sun.com>> wrote:
>>
>> Team,
>>
>> One more attempt to solve this puzzle is attached. Again, tested
>> in the
>> container and via e-p-t tests (i.e. no special testing for SE
>> changes of
>> the platform, session name, and logging).
>>
>> Please review *very carefully* ;).
>>
>> thanks,
>> -marina
>>
>> Marina Vatkina wrote:
>> > Hi Wonseok,
>> >
>> > Wonseok Kim wrote:
>> >
>> >> Hello Marina,
>> >>
>> >> I realized today that logger property(toplink.logging.logger )
>> can
>> >> change also in deploy phase like other properties.
>> >> If it is different, loggers should be updated also. Do you want
>> to fix
>> >> it also or file it as another issue?
>> >
>> >
>> > Probably. Let me think about it.
>> >
>> >>
>> >> I found also that your modification is not using member variable
>> >> "serverPlatform" anymore which you introduced before.
>> >
>> >
>> > Thanks.
>> >
>> >> As to session.setSessionLog(), just do as you wish, I think it's
>> not a
>> >> big deal.
>> >
>> >
>> > OK.
>> >
>> > thanks,
>> > -marina
>> >
>> >>
>> >> Cheers,
>> >> -Wonseok
>> >>
>>
>>