You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geronimo.apache.org by Kevan Miller <ke...@gmail.com> on 2006/11/16 18:01:30 UTC
Re: svn commit: r475349 - /geronimo/server/branches/1.1/modules/kernel/src/java/org/apache/geronimo/gbean/runtime/GBeanSingleReference.java
Vamsi,
I know you were following the existing code. However, the fact that
these log.debug calls are not guarded by "if (log.isDebugEnabled)"
checks is not good...
Hmm. There's not a single is*Enabled call in the kernel... That's bad
practice, IMO...
Also, I see that GBeanSingleReference log level is DEBUG (i didn't
realize this...). IMO we should not be logging "delayed" or "started"
information by default. I think we should change the log level to
INFO... Hmmph. i changed that log level, now we don't log anything
during startup. That's not good either... :-(
--kevan
On Nov 15, 2006, at 1:37 PM, vamsic007@apache.org wrote:
> Author: vamsic007
> Date: Wed Nov 15 10:37:40 2006
> New Revision: 475349
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=475349
> Log:
> GERONIMO-2386 Cleanup debug log entries created during server startup
>
> Modified:
> geronimo/server/branches/1.1/modules/kernel/src/java/org/apache/
> geronimo/gbean/runtime/GBeanSingleReference.java
>
> Modified: geronimo/server/branches/1.1/modules/kernel/src/java/org/
> apache/geronimo/gbean/runtime/GBeanSingleReference.java
> URL: http://svn.apache.org/viewvc/geronimo/server/branches/1.1/
> modules/kernel/src/java/org/apache/geronimo/gbean/runtime/
> GBeanSingleReference.java?view=diff&rev=475349&r1=475348&r2=475349
> ======================================================================
> ========
> --- geronimo/server/branches/1.1/modules/kernel/src/java/org/apache/
> geronimo/gbean/runtime/GBeanSingleReference.java (original)
> +++ geronimo/server/branches/1.1/modules/kernel/src/java/org/apache/
> geronimo/gbean/runtime/GBeanSingleReference.java Wed Nov 15
> 10:37:40 2006
> @@ -27,7 +27,7 @@
> import org.apache.geronimo.kernel.GBeanNotFoundException;
>
> /**
> - * @version $Rev: 384141 $ $Date$
> + * @version $Rev$ $Date$
> */
> public class GBeanSingleReference extends AbstractGBeanReference {
> private static final Log log = LogFactory.getLog
> (GBeanSingleReference.class);
> @@ -77,11 +77,13 @@
> } catch (GBeanNotFoundException e) {
> // gbean disappeard on us
> log.debug("Waiting to start " + abstractName + "
> because no targets are running for reference " + getName() +"
> matching the patterns " + targetName);
> + return false;
> }
> } else {
> setProxy(getKernel().getProxyManager().createProxy
> (targetName, getReferenceType()));
> }
>
> + log.debug("Started " + abstractName);
> return true;
> }
>
>
>
Re: svn commit: r475349 - /geronimo/server/branches/1.1/modules/kernel/src/java/org/apache/geronimo/gbean/runtime/GBeanSingleReference.java
Posted by Vamsavardhana Reddy <c1...@gmail.com>.
Kevan,
Is it necessary to have if(log.isDebugEnabled) check for all the log.debug()
calls? Won't the logger take care of whether to log the message or not
depending on the log level set?
--vamsi
On 11/16/06, Kevan Miller <ke...@gmail.com> wrote:
>
> Vamsi,
> I know you were following the existing code. However, the fact that
> these log.debug calls are not guarded by "if (log.isDebugEnabled)"
> checks is not good...
>
> Hmm. There's not a single is*Enabled call in the kernel... That's bad
> practice, IMO...
>
> Also, I see that GBeanSingleReference log level is DEBUG (i didn't
> realize this...). IMO we should not be logging "delayed" or "started"
> information by default. I think we should change the log level to
> INFO... Hmmph. i changed that log level, now we don't log anything
> during startup. That's not good either... :-(
>
> --kevan
>
> On Nov 15, 2006, at 1:37 PM, vamsic007@apache.org wrote:
>
> > Author: vamsic007
> > Date: Wed Nov 15 10:37:40 2006
> > New Revision: 475349
> >
> > URL: http://svn.apache.org/viewvc?view=rev&rev=475349
> > Log:
> > GERONIMO-2386 Cleanup debug log entries created during server startup
> >
> > Modified:
> > geronimo/server/branches/1.1/modules/kernel/src/java/org/apache/
> > geronimo/gbean/runtime/GBeanSingleReference.java
> >
> > Modified: geronimo/server/branches/1.1/modules/kernel/src/java/org/
> > apache/geronimo/gbean/runtime/GBeanSingleReference.java
> > URL: http://svn.apache.org/viewvc/geronimo/server/branches/1.1/
> > modules/kernel/src/java/org/apache/geronimo/gbean/runtime/
> > GBeanSingleReference.java?view=diff&rev=475349&r1=475348&r2=475349
> > ======================================================================
> > ========
> > --- geronimo/server/branches/1.1/modules/kernel/src/java/org/apache/
> > geronimo/gbean/runtime/GBeanSingleReference.java (original)
> > +++ geronimo/server/branches/1.1/modules/kernel/src/java/org/apache/
> > geronimo/gbean/runtime/GBeanSingleReference.java Wed Nov 15
> > 10:37:40 2006
> > @@ -27,7 +27,7 @@
> > import org.apache.geronimo.kernel.GBeanNotFoundException;
> >
> > /**
> > - * @version $Rev: 384141 $ $Date$
> > + * @version $Rev$ $Date$
> > */
> > public class GBeanSingleReference extends AbstractGBeanReference {
> > private static final Log log = LogFactory.getLog
> > (GBeanSingleReference.class);
> > @@ -77,11 +77,13 @@
> > } catch (GBeanNotFoundException e) {
> > // gbean disappeard on us
> > log.debug("Waiting to start " + abstractName + "
> > because no targets are running for reference " + getName() +"
> > matching the patterns " + targetName);
> > + return false;
> > }
> > } else {
> > setProxy(getKernel().getProxyManager().createProxy
> > (targetName, getReferenceType()));
> > }
> >
> > + log.debug("Started " + abstractName);
> > return true;
> > }
> >
> >
> >
>
>