You are viewing a plain text version of this content. The canonical link for it is here.
Posted to scm@geronimo.apache.org by ja...@apache.org on 2008/05/29 16:59:43 UTC
svn commit: r661343 -
/geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
Author: jawarner
Date: Thu May 29 07:59:43 2008
New Revision: 661343
URL: http://svn.apache.org/viewvc?rev=661343&view=rev
Log:
GERONIMO-4087: Improve usability of gshell commands deploy/* when failing to connect to server
Modified:
geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
Modified: geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java?rev=661343&r1=661342&r2=661343&view=diff
==============================================================================
--- geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java (original)
+++ geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java Thu May 29 07:59:43 2008
@@ -178,10 +178,14 @@
}
return manager;
} catch (IOException e) {
- log.fatal("caught ", e);
+ if (log.isDebugEnabled()) {
+ log.debug("caught ", e);
+ }
DeploymentManagerCreationException deploymentManagerCreationException =
(DeploymentManagerCreationException) new DeploymentManagerCreationException(e.getMessage()).initCause(e);
- log.fatal("throwing ", deploymentManagerCreationException);
+ if (log.isDebugEnabled()) {
+ log.debug("throwing ", deploymentManagerCreationException);
+ }
throw deploymentManagerCreationException;
} catch (SecurityException e) {
if (log.isDebugEnabled()) {
Re: svn commit: r661343 - /geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
Posted by Jason Warner <ja...@apache.org>.
Thanks for the heads up.
On Fri, May 30, 2008 at 9:40 AM, Jason Dillon <ja...@planet57.com> wrote:
> There is not string concatenation going on here, so this call
> if (log.isDebugEnabled()) {
> log.debug("caught ", e);
> }
>
> Actually wastes a call to log.isDebugEnabled() as log.debug() will invoke
> this method prior to processing the log message anyways. You only need to
> guard log messages which contain string concatenation.
>
> --jason
>
>
> On May 30, 2008, at 8:30 PM, Jason Warner wrote:
>
> How so?
>
> On Fri, May 30, 2008 at 12:22 AM, Jason Dillon <ja...@planet57.com> wrote:
>
>> Its unnecessary to guard these log statements.
>>
>> --jason
>>
>>
>>
>> On May 29, 2008, at 9:59 PM, jawarner@apache.org wrote:
>>
>> Author: jawarner
>>> Date: Thu May 29 07:59:43 2008
>>> New Revision: 661343
>>>
>>> URL: http://svn.apache.org/viewvc?rev=661343&view=rev
>>> Log:
>>> GERONIMO-4087: Improve usability of gshell commands deploy/* when failing
>>> to connect to server
>>>
>>> Modified:
>>>
>>> geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
>>>
>>> Modified:
>>> geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
>>> URL:
>>> http://svn.apache.org/viewvc/geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java?rev=661343&r1=661342&r2=661343&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
>>> (original)
>>> +++
>>> geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
>>> Thu May 29 07:59:43 2008
>>> @@ -178,10 +178,14 @@
>>> }
>>> return manager;
>>> } catch (IOException e) {
>>> - log.fatal("caught ", e);
>>> + if (log.isDebugEnabled()) {
>>> + log.debug("caught ", e);
>>> + }
>>> DeploymentManagerCreationException
>>> deploymentManagerCreationException =
>>> (DeploymentManagerCreationException) new
>>> DeploymentManagerCreationException(e.getMessage()).initCause(e);
>>> - log.fatal("throwing ", deploymentManagerCreationException);
>>> + if (log.isDebugEnabled()) {
>>> + log.debug("throwing ",
>>> deploymentManagerCreationException);
>>> + }
>>> throw deploymentManagerCreationException;
>>> } catch (SecurityException e) {
>>> if (log.isDebugEnabled()) {
>>>
>>>
>>>
>>
>
>
> --
> ~Jason Warner
>
>
>
--
~Jason Warner
Re: svn commit: r661343 - /geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
Posted by Jason Dillon <ja...@planet57.com>.
There is not string concatenation going on here, so this call
if (log.isDebugEnabled()) {
log.debug("caught ", e);
}
Actually wastes a call to log.isDebugEnabled() as log.debug() will
invoke this method prior to processing the log message anyways. You
only need to guard log messages which contain string concatenation.
--jason
On May 30, 2008, at 8:30 PM, Jason Warner wrote:
> How so?
>
> On Fri, May 30, 2008 at 12:22 AM, Jason Dillon <ja...@planet57.com>
> wrote:
> Its unnecessary to guard these log statements.
>
> --jason
>
>
>
> On May 29, 2008, at 9:59 PM, jawarner@apache.org wrote:
>
> Author: jawarner
> Date: Thu May 29 07:59:43 2008
> New Revision: 661343
>
> URL: http://svn.apache.org/viewvc?rev=661343&view=rev
> Log:
> GERONIMO-4087: Improve usability of gshell commands deploy/* when
> failing to connect to server
>
> Modified:
> geronimo/server/branches/2.1/framework/modules/geronimo-deploy-
> jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/
> BaseDeploymentFactory.java
>
> Modified: geronimo/server/branches/2.1/framework/modules/geronimo-
> deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/
> factories/BaseDeploymentFactory.java
> URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java?rev=661343&r1=661342&r2=661343&view=diff
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- geronimo/server/branches/2.1/framework/modules/geronimo-deploy-
> jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/
> BaseDeploymentFactory.java (original)
> +++ geronimo/server/branches/2.1/framework/modules/geronimo-deploy-
> jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/
> BaseDeploymentFactory.java Thu May 29 07:59:43 2008
> @@ -178,10 +178,14 @@
> }
> return manager;
> } catch (IOException e) {
> - log.fatal("caught ", e);
> + if (log.isDebugEnabled()) {
> + log.debug("caught ", e);
> + }
> DeploymentManagerCreationException
> deploymentManagerCreationException =
> (DeploymentManagerCreationException) new
> DeploymentManagerCreationException(e.getMessage()).initCause(e);
> - log.fatal("throwing ",
> deploymentManagerCreationException);
> + if (log.isDebugEnabled()) {
> + log.debug("throwing ",
> deploymentManagerCreationException);
> + }
> throw deploymentManagerCreationException;
> } catch (SecurityException e) {
> if (log.isDebugEnabled()) {
>
>
>
>
>
>
> --
> ~Jason Warner
Re: svn commit: r661343 - /geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
Posted by Jason Warner <ja...@apache.org>.
How so?
On Fri, May 30, 2008 at 12:22 AM, Jason Dillon <ja...@planet57.com> wrote:
> Its unnecessary to guard these log statements.
>
> --jason
>
>
>
> On May 29, 2008, at 9:59 PM, jawarner@apache.org wrote:
>
> Author: jawarner
>> Date: Thu May 29 07:59:43 2008
>> New Revision: 661343
>>
>> URL: http://svn.apache.org/viewvc?rev=661343&view=rev
>> Log:
>> GERONIMO-4087: Improve usability of gshell commands deploy/* when failing
>> to connect to server
>>
>> Modified:
>>
>> geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
>>
>> Modified:
>> geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
>> URL:
>> http://svn.apache.org/viewvc/geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java?rev=661343&r1=661342&r2=661343&view=diff
>>
>> ==============================================================================
>> ---
>> geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
>> (original)
>> +++
>> geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
>> Thu May 29 07:59:43 2008
>> @@ -178,10 +178,14 @@
>> }
>> return manager;
>> } catch (IOException e) {
>> - log.fatal("caught ", e);
>> + if (log.isDebugEnabled()) {
>> + log.debug("caught ", e);
>> + }
>> DeploymentManagerCreationException
>> deploymentManagerCreationException =
>> (DeploymentManagerCreationException) new
>> DeploymentManagerCreationException(e.getMessage()).initCause(e);
>> - log.fatal("throwing ", deploymentManagerCreationException);
>> + if (log.isDebugEnabled()) {
>> + log.debug("throwing ",
>> deploymentManagerCreationException);
>> + }
>> throw deploymentManagerCreationException;
>> } catch (SecurityException e) {
>> if (log.isDebugEnabled()) {
>>
>>
>>
>
--
~Jason Warner
Re: svn commit: r661343 - /geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
Posted by Jason Dillon <ja...@planet57.com>.
Its unnecessary to guard these log statements.
--jason
On May 29, 2008, at 9:59 PM, jawarner@apache.org wrote:
> Author: jawarner
> Date: Thu May 29 07:59:43 2008
> New Revision: 661343
>
> URL: http://svn.apache.org/viewvc?rev=661343&view=rev
> Log:
> GERONIMO-4087: Improve usability of gshell commands deploy/* when
> failing to connect to server
>
> Modified:
> geronimo/server/branches/2.1/framework/modules/geronimo-deploy-
> jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/
> BaseDeploymentFactory.java
>
> Modified: geronimo/server/branches/2.1/framework/modules/geronimo-
> deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/
> factories/BaseDeploymentFactory.java
> URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java?rev=661343&r1=661342&r2=661343&view=diff
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- geronimo/server/branches/2.1/framework/modules/geronimo-deploy-
> jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/
> BaseDeploymentFactory.java (original)
> +++ geronimo/server/branches/2.1/framework/modules/geronimo-deploy-
> jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/
> BaseDeploymentFactory.java Thu May 29 07:59:43 2008
> @@ -178,10 +178,14 @@
> }
> return manager;
> } catch (IOException e) {
> - log.fatal("caught ", e);
> + if (log.isDebugEnabled()) {
> + log.debug("caught ", e);
> + }
> DeploymentManagerCreationException
> deploymentManagerCreationException =
> (DeploymentManagerCreationException) new
> DeploymentManagerCreationException(e.getMessage()).initCause(e);
> - log.fatal("throwing ",
> deploymentManagerCreationException);
> + if (log.isDebugEnabled()) {
> + log.debug("throwing ",
> deploymentManagerCreationException);
> + }
> throw deploymentManagerCreationException;
> } catch (SecurityException e) {
> if (log.isDebugEnabled()) {
>
>
Re: svn commit: r661343 - /geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
Posted by Jason Warner <ja...@apache.org>.
That's a good point, Jarek. I guess I wasn't really thinking when I made
the change. As is, the logging is redundant. I'll make your suggested
change shortly.
Thanks!
On Thu, May 29, 2008 at 11:09 AM, Jarek Gawor <jg...@gmail.com> wrote:
> Since the DeploymentManagerCreationException (or
> AuthenticationFailedException) is initialized with .initCause(e) of
> the root exception why log the root exception and the new exception?
> Logging of DeploymentManagerCreationException or
> AuthenticationFailedException should be enough (which will include the
> root exception).
>
> Jarek
>
> On Thu, May 29, 2008 at 10:59 AM, <ja...@apache.org> wrote:
> > Author: jawarner
> > Date: Thu May 29 07:59:43 2008
> > New Revision: 661343
> >
> > URL: http://svn.apache.org/viewvc?rev=661343&view=rev
> > Log:
> > GERONIMO-4087: Improve usability of gshell commands deploy/* when failing
> to connect to server
> >
> > Modified:
> >
> geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
> >
> > Modified:
> geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
> > URL:
> http://svn.apache.org/viewvc/geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java?rev=661343&r1=661342&r2=661343&view=diff
> >
> ==============================================================================
> > ---
> geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
> (original)
> > +++
> geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
> Thu May 29 07:59:43 2008
> > @@ -178,10 +178,14 @@
> > }
> > return manager;
> > } catch (IOException e) {
> > - log.fatal("caught ", e);
> > + if (log.isDebugEnabled()) {
> > + log.debug("caught ", e);
> > + }
> > DeploymentManagerCreationException
> deploymentManagerCreationException =
> > (DeploymentManagerCreationException) new
> DeploymentManagerCreationException(e.getMessage()).initCause(e);
> > - log.fatal("throwing ", deploymentManagerCreationException);
> > + if (log.isDebugEnabled()) {
> > + log.debug("throwing ",
> deploymentManagerCreationException);
> > + }
> > throw deploymentManagerCreationException;
> > } catch (SecurityException e) {
> > if (log.isDebugEnabled()) {
> >
> >
> >
>
--
~Jason Warner
Re: svn commit: r661343 - /geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
Posted by Jarek Gawor <jg...@gmail.com>.
Since the DeploymentManagerCreationException (or
AuthenticationFailedException) is initialized with .initCause(e) of
the root exception why log the root exception and the new exception?
Logging of DeploymentManagerCreationException or
AuthenticationFailedException should be enough (which will include the
root exception).
Jarek
On Thu, May 29, 2008 at 10:59 AM, <ja...@apache.org> wrote:
> Author: jawarner
> Date: Thu May 29 07:59:43 2008
> New Revision: 661343
>
> URL: http://svn.apache.org/viewvc?rev=661343&view=rev
> Log:
> GERONIMO-4087: Improve usability of gshell commands deploy/* when failing to connect to server
>
> Modified:
> geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
>
> Modified: geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java
> URL: http://svn.apache.org/viewvc/geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java?rev=661343&r1=661342&r2=661343&view=diff
> ==============================================================================
> --- geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java (original)
> +++ geronimo/server/branches/2.1/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/factories/BaseDeploymentFactory.java Thu May 29 07:59:43 2008
> @@ -178,10 +178,14 @@
> }
> return manager;
> } catch (IOException e) {
> - log.fatal("caught ", e);
> + if (log.isDebugEnabled()) {
> + log.debug("caught ", e);
> + }
> DeploymentManagerCreationException deploymentManagerCreationException =
> (DeploymentManagerCreationException) new DeploymentManagerCreationException(e.getMessage()).initCause(e);
> - log.fatal("throwing ", deploymentManagerCreationException);
> + if (log.isDebugEnabled()) {
> + log.debug("throwing ", deploymentManagerCreationException);
> + }
> throw deploymentManagerCreationException;
> } catch (SecurityException e) {
> if (log.isDebugEnabled()) {
>
>
>