Hi Mark,
This is pretty good stuff and constitutes significant progress compared to existing Watchdog code, in particular: 1) support for multiple input types to watch for 2) separation of watchdog functionality from actual log4j configuration. (The configureAndWatch methods are looking more and more like legacy code. We should soon move to deprecate them.) Here are a few suggestions that you might wish to consider. Your Watchdog class contains some static methods and fields to keep track of and manage watchdog sub-classes. These static methods should perhaps be in their own static class, say WatchdogManager (your choice of name is as good as mine). I would also suggest making Watchdog an interface, and have WatchdogSkeleton (an abstact class) which implements the run, configure, startWatching, stopWatching and setter/getter methods. The implementation of the abstract getModificationTime and getConfigurationStream would be delegated to concrete sub-classes. Watchdogs should also be named components allowing their identification in configuration files or management consoles. If that were the case, we could for example write: log4j.watchdog.W1=org.apache.log4j.varia.FileWatchdog log4j.watchdog.W1.interval=30 log4j.watchdog.W1.resource=this log4j.watchdog.W1.operation=start # other configuration directives: log4j.rootCategory=, A1 log4j.appender.A1=org.apache.log4j.ConsoleAppender log4j.appender.A1.layout=org.apache.log4j.PatternLayout log4j.appender.A1.layout.ConversionPattern=%-4r %-5p [%t] %37c %3x - %m%n I have a number of other comments but I'll stop here to let you reply first. By the way, thanks to your code I now have a much clearer understanding of the watchdog functionality that we might want. More on this later. Cheers, Ceki ps: You should add yourself as an author, not just as a contributor. At 15:43 08.01.2002 -0800, Mark Womack wrote: >Sometime back I submitted changes to the existing Watchdog and FileWatchdog >classes. The main goal was to make it easy to create other Watchdog >subclasses and to build more functionality into the Watchdog class. >Enclosed is a second pass at the changes. I think this new version is much >better than my previous attempt. I am submitting them for possible >inclusion into a future version of Log4J, and would appreciate any feedback >anyone might have regarding the design and implementation. Even if you >think it is all crap, I would like to hear why. > >I have enclosed the full java files instead of patches. If folks feel that >these changes should be included, I will generate universal cvs patch files >against the current cvs versions. > >Descriptions: > >org.apache.log4j.helpers.Watchdog.java - Just about a complete rewrite of >this class. I felt that so much was changing, it would be better to start >fresh. It now extends Runnable instead of subclassing Thread. A Watchdog >can be started, stopped, and re-started. The static portion of the class >maintains a list of active Watchdog's, and there is a static method, >stopAllWatchdogs(), which can be used to stop all active watchdogs. This >class is now configuration specific with a method of reconfigure. Since >that is all watchdogs are currently used for, I thought this was ok, but I >could rework the design to move the configuration related code to a subclass >of ConfigurationWatchdog. A watchdog still has the check interval (aka >delay) property. In addition, you can now set the test modification time >(the initial time it tests against, defaults to 0) or the configuration >class (the configurator to use for reconfiguration, defaults to >PropertyConfigurator). Subclasses of Watchdog must implement two methods: > > getModificationTime() - Returns the modifcation time of the entity being >watched. > getConfigurationStream() - Returns an InputStream that contains the >reconfiguration data. > >Also, it should be noted that the Watchdog class makes absolutely no >assumptions about the entity being watched or that the reconfiguration data >even comes from the watched entity. It can be a file, multiple files, a >url, whatever. It just requires a modification time to test against and an >InputStream containing the configuration data. All of this is provided by >the Watchdog subclass. > >The upside to the new design, in my opinion, is that you can now create a >Watchdog, set it up to use a specific configurator, and start it. You never >have to call a configureAndWatch() method on the configurator or have the >author of the configurator class implement that method. The only >requirement is that the configurator has to support a doConfigure() method >that uses an InputStream. And you are not limited to the type of Watchdog >used in the configureAndWatch() method, which right now is only >FileWatchdog, it can be any watchdog class that anyone has developed. > >org.apache.log4j.helpers.FileWatchdog.java - This is a new version of the >FileWatchdog implemented to conform to the Watchdog design. It implements >the getModificationTime() and getConfigurationStream() methods. Nothing new >here; i basically used Ceki's original implementation. > >org.apache.log4j.PropertyConfigurator.java - Changed configureAndWatch() to >use the new FileWatchdog class. Also had to add a new >doConfigure(InputStream, LoggingRespository) method so that it could be used >with the new Watchdog design. This was trivial. > >org.apache.log4j.xml.DOMConfigurator.java - Changed configureAndWatch() to >use the new FileWatchdog class. > >org.apache.log4j.helpers.HTTPWatchdog.java - This is a new class implemented >using the Watchdog design. It can be configured to watch a url, and when >the Last-Modified response header value changes, the url is used as a source >for the new configuration data. This would allow you to put configuration >information for multiple machines onto a web server. Any change to the file >on the web server would be picked up on any machine that has set up to use >this watchdog to watch the url. > >Again, I would appreciate any feedback anyone has regarding these changes, >design, and implementation. > >Thanks for your time, >-Mark > > > > > > >-- >To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> >For additional commands, e-mail: <mailto:[EMAIL PROTECTED]> -- Ceki Gülcü - http://qos.ch -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
