Quoting Ceki G�lc� <[EMAIL PROTECTED]>: > At 07:08 AM 12/1/2004, Jacob Kjome wrote: > >Hi Ceki, > > > >Unfortunately, I think you are still missing my point. The changes to the > >RepositorySelector interface are good. The change to the LogManager is a > >bit mysterious. I guess it is trying to protect against overriding the > >default logger repository of custom selectors which initiate a default > >repository in their constructor? > > Your conclusion is correct as long as it is limited to the scope of > LogManager's static initializer. >
The thing is, I never even had this in mind and I see it as far less of a problem (by leaps and bounds) then the original issue I was addressing. I think we'd be pretty safe if this check didn't exist. The fact that it does exist is fine since it doesn't hurt anything, but I don't see it as particularily important either. I'd put a bet on the fact that no one would ever run into this issue if we didn't do the check. > >Because that's the only way the default repository would actually exist in > >selectors instantiated within the static initialization code of LogManager. > > Absolutely. > > > I guess that might be a legitimate check? Can't hurt either > > way. However, it doesn't address my original point at all, though I can > > use the same code change from the static initialization code within the > > LogManager.setRepositorySelector() for the same effect there, which is > > what I was trying to point out in the first place. To make it abundantly > > clear what I am talking about, look at the change I just committed. I'm > > positive this is correct and it fixes all the null pointers which, BTW, > > cause all apps in Tomcat to cease to work. It is clearly the right thing > > to do. If you really disagree, you can always back it out, but I can't > > see why you would want to. > > It fixes the null pointer problem, for sure. However, introducing side > effects like you have done eventually become part of the expected behavior > of your API. You can't change it without breaking code downstream. > Forgetting to set the default repository will result in a fairly immediate > crash of the application. So anyone travelling on the clue train will > figure our that you have to set the default repository in a custom > selector. It's not rocket science. > No, it isn't, but how is a sensible default a "side effect"? In my initializer code, and I'd bet 99% of everyone else's initializer code, I would do... RepositorySelector selector = new ContextJNDISelector(); selector.setDefaultLoggerRepository(LogManager.getDefaultLoggerRepository()); LogManager.setRepositorySelector(selector, new Object()); How is a sensible default not simply... LogManager.setRepositorySelector(new ContextJNDISelector(), new Object()); With the assumption that LogManager would continue with the current selector's default logger repository? I say "assumption" and I'm guessing your are cringing at the word, but we make lots of assumptions in Log4j just like in other software products. As far as assumptions go, this is what for the "duh file". I cannot imagine that a default of a NullPointerException bringing down every app in the server is better than this sensible "side effect". Really, I'm at a loss! > >Bottom line is that external initializing code should never ever be able > >to bring down every app in the server by simply forgetting to set a > >default logger repository on the custom selector. We shouldn't override a > >default logger repository that has been set, but we should definitely make > >sure it doesn't stay null if it wasn't set. > > It's not our problem to fix. Unmaintainable code is born trying to > compensate for other people's mistakes. Keeping the hocus-focus level low > eventually makes a big difference with regards to code maintainability. > It is plainly unnacceptable to allow for a default (without documentation to the fact, I might add) that brings down the entire appserver with one line of seemingly inocuous logging initialization code. Talk about side effects. This is the mother of them all. And I'm not sure why this makes code "unmaintainable"? The developer now types fewer lines and consciously (by not manually setting the default when it is obvious in the API that they can if they want) accepts Log4j's sensible (and predictable) default of no change to the current default logger repository. Am I making any sense here or am I just living in Bizarro world, because it is sure clear to me. > > And the default repository of choice is the same one that was on the > > previous selector. That way, the transition is seamless. Anyway, take a > > look, and try this under Tomcat-5.5.4, not Tomcat-5.0.xx. Also, the > > problem would never show itself if you started up by passing the selector > > name in via the -D parameter on the command line. It would only occur in > > the case where the default selector was being used and then some > > initializer code called LogManager.setRepositorySelector(). > > That is correct but probably not relevant. I am repeating myself but the > RepositorySelector API is designed to be used by developers of Application > Servers or 1st class passengers on the clue train. They are expected to > look at the source code or read the documentation. > "clue train"? What documentation? And why shouldn't a developer only need to look at the API? Why should we expect people on the "clue train" to look at every inch of code in a logging API. Ceki, I can see your point to an extent, but sensible and predictable defaults are not, IMO, "side effects". Log4j should be easy to use and shouldn't be able to be used for evil (taking down entire application servers) whether the developer using the API is clueless or a prodogy (doing it with full knowledge out of spite). I strongly disagree with your conclusions here and hope you change your mind. At a minimum, I'm satisfied that I've vigorously tried to defend my position. If it doesn't prevail, then I'll drink the coolaide and modify my initialization code to do something that Log4j should do for me in the first place, but that's life and open source. Jake --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
