commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Stansberry <bes_commons_...@yahoo.com>
Subject Re: [logging] detecting logging libs
Date Mon, 23 May 2005 01:38:22 GMT
--- Simon Kitching <skitching@apache.org> wrote:
> On Sun, 2005-05-22 at 21:01 +0100, robert burrell
> donkin wrote:
> > On Sun, 2005-05-22 at 10:43 +0000,
> skitching@apache.org wrote:
> > 
> > <snip>
> > 
> > >      protected boolean
> isJdk13LumberjackAvailable() {
> > >  
> > > +        // note: the algorithm here is
> different from isLog4JAvailable.
> > > +        // I think isLog4JAvailable is
> correct....see bugzilla#31597
> > 
> > +1
> > 
> > could do with going through all the isAvailable's
> and checking whether
> > their algorithms are correct. however, i suspect
> that brian's approach
> > will be needed to deal correctly with some
> circumstances. if no one
> > feels like volunteering should probably record
> this in bugzilla so we
> > don't lose track...
> 
> If by "brian's approach" you mean creating an
> instance of the logger
> class in order to test whether that logging lib is
> *really* available, I
> agree. 
> 
> Not only is it more reliable, but it's a cleaner
> solution; currently the
> LogFactoryImpl class is making *assumptions* about
> what classes the
> various logging adapters depend on. That information
> should be only in
> the logging adapter class.
> 
> The only problem with creating an instance of the
> logger is that we
> would have to pass a category string to the logger
> constructor, and
> therefore must build an assumption into
> LogFactoryImpl about what
> category names are valid for the underlying logger.
> Can we assume that
> an empty string is a valid category for all logger
> libraries? Can we
> assume that "apache" or "org.apache.commons.logging"
> are valid category
> strings? Perhaps some loggers only accept valid URLs
> as
> categories...yes, I'm playing devil's advocate a bit
> here. 

It's a good point; the "assumed" category names in the
various isXYZAvailable method is my biggest dislike
about the patch I submitted.  Actually, if an approach
like the patch were adopted, I think we should
consider marking those methods as deprecated (they're
not used in the patched LogFactoryImpl itself) and
dropping them at the next release with a point scope
that allows it.

In general, if any significant work on LogFactoryImpl
is done I think a review of its API would be good. 
This seems to me to be the kind of class that if
someone wants a different implementation, they should
write a different implementation and not count on a
lot  of behaviors exposed by a superclass.

Of course, any kind of deprecation doesn't solve the
immediate problem.

Looking into it more, I remembered that
LogFactoryImpl.getInstance(Class clazz) just calls
getInstance(clazz.getName()) and thus already creates
a requirement that Log implementations support fully
qualified class names.  So any adapter that doesn't do
this must be designed to work with a custom
LogFactory.  So, the potential list of users affected
by this issue is smaller -- only those with custom
LogFactory implementations and Log implementations
that don't support class names.

> I guess we
> could always say that the writer of the logging
> adapter is required to
> return a valid logger instance for category "", even
> if that is not
> normally a category that is valid to the underlying
> library.
> 

Half-baked idea to throw out before I run.  Add to
LogFactoryImpl:

/**
 * Returns a category that can be used to test whether
 * an instance of a <code>Log</code> can be created.
 * <p>
 * This default implementation returns the fully 
 * qualified name of this class.  Subclasses should
 * override this if they support <code>Log</code>
 * implementations that cannot handle this category.
 * </p>
 */
protected String getTestCategory() {
  return getClass().getName();
}

All the various isXYZAvailable methods would call this
method when creating a test instance.

Brian

> Regards,
> 
> Simon
> 
> 
> 
>
---------------------------------------------------------------------
> To unsubscribe, e-mail:
> commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail:
> commons-dev-help@jakarta.apache.org
> 
> 



		
__________________________________ 
Yahoo! Mail Mobile 
Take Yahoo! Mail with you! Check email on your mobile phone. 
http://mobile.yahoo.com/learn/mail 

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message