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