karaf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Glen Mazza <glen.ma...@gmail.com>
Subject Re: svn commit: r1176338
Date Tue, 27 Sep 2011 15:27:25 GMT
Hi JB, just some suggestions:

On 09/27/2011 07:49 AM, jbonofre@apache.org wrote:
> ==============================================================================
> --- karaf/branches/karaf-2.2.x/shell/osgi/src/main/java/org/apache/karaf/shell/osgi/RestartBundle.java
(original)
> +++ karaf/branches/karaf-2.2.x/shell/osgi/src/main/java/org/apache/karaf/shell/osgi/RestartBundle.java
Tue Sep 27 11:49:12 2011
> @@ -26,10 +26,13 @@ public class RestartBundle extends Bundl
>
>       protected void doExecute(List<Bundle>  bundles) throws Exception {
>           for (Bundle bundle : bundles) {
> -            bundle.stop();
> -        }
> -        for (Bundle bundle : bundles) {
> -            bundle.start();
> +            try {
> +                bundle.stop();
> +                bundle.start();
> +            } catch (Exception e) {
> +                System.err.println("Bundle " + bundle.getBundleId() + " didn't restart
correctly");
> +                e.printStackTrace();
> +            }
>           }
>       }
>
>

For the above, you might wish to, for future troubleshooting, split out 
the .stop() and .start() into separate try-catch blocks with different 
error messages, so it is easier to know which half failed just by 
viewing the exception message.


> Modified: karaf/branches/karaf-2.2.x/shell/osgi/src/main/java/org/apache/karaf/shell/osgi/StartBundle.java
> URL: http://svn.apache.org/viewvc/karaf/branches/karaf-2.2.x/shell/osgi/src/main/java/org/apache/karaf/shell/osgi/StartBundle.java?rev=1176338&r1=1176337&r2=1176338&view=diff
> ==============================================================================
> --- karaf/branches/karaf-2.2.x/shell/osgi/src/main/java/org/apache/karaf/shell/osgi/StartBundle.java
(original)
> +++ karaf/branches/karaf-2.2.x/shell/osgi/src/main/java/org/apache/karaf/shell/osgi/StartBundle.java
Tue Sep 27 11:49:12 2011
> @@ -26,7 +26,12 @@ public class StartBundle extends Bundles
>
>       protected void doExecute(List<Bundle>  bundles) throws Exception {
>           for (Bundle bundle : bundles) {
> -            bundle.start();
> +            try {
> +                bundle.start();
> +            } catch (Exception e) {
> +                System.err.println("Bundle " + bundle.getBundleId() + " didn't start
correctly");
> +                e.printStackTrace();
> +            }
>           }
>       }

If you're comfortable making the statement, I think 
System.err.println("Bundle " + bundle.getBundleId() + " failed to 
start."); is better, as it gives confidence to the user that there's no 
rogue bundles started in an invalid state, that Karaf either tries to 
start a bundle or fails cleanly.  (And if it *did* start in an invalid 
state, that would be a Karaf internal bug to be fixed/patched so that 
wouldn't happen.)

>
>
> Modified: karaf/branches/karaf-2.2.x/shell/osgi/src/main/java/org/apache/karaf/shell/osgi/StopBundle.java
> URL: http://svn.apache.org/viewvc/karaf/branches/karaf-2.2.x/shell/osgi/src/main/java/org/apache/karaf/shell/osgi/StopBundle.java?rev=1176338&r1=1176337&r2=1176338&view=diff
> ==============================================================================
> --- karaf/branches/karaf-2.2.x/shell/osgi/src/main/java/org/apache/karaf/shell/osgi/StopBundle.java
(original)
> +++ karaf/branches/karaf-2.2.x/shell/osgi/src/main/java/org/apache/karaf/shell/osgi/StopBundle.java
Tue Sep 27 11:49:12 2011
> @@ -26,7 +26,12 @@ public class StopBundle extends BundlesC
>   	
>   	protected void doExecute(List<Bundle>  bundles) throws Exception {
>           for (Bundle bundle : bundles) {
> -            bundle.stop();
> +            try {
> +                bundle.stop();
> +            } catch (Exception e) {
> +                System.err.println("Bundle " + bundle.getBundleId() + " didn't stop
correctly");
> +                e.printStackTrace();
> +            }
>           }
>       }
>
>

Unsure, but perhaps System.err.println("Bundle " + bundle.getBundleId() + " didn't stop correctly;
if still active may be in an invalid state.");  would be better because it warns the user
not to rely on that supposedly still active bundle--that its state might be damaged.



> Modified: karaf/branches/karaf-2.2.x/shell/osgi/src/main/java/org/apache/karaf/shell/osgi/UninstallBundle.java
> URL: http://svn.apache.org/viewvc/karaf/branches/karaf-2.2.x/shell/osgi/src/main/java/org/apache/karaf/shell/osgi/UninstallBundle.java?rev=1176338&r1=1176337&r2=1176338&view=diff
> ==============================================================================
> --- karaf/branches/karaf-2.2.x/shell/osgi/src/main/java/org/apache/karaf/shell/osgi/UninstallBundle.java
(original)
> +++ karaf/branches/karaf-2.2.x/shell/osgi/src/main/java/org/apache/karaf/shell/osgi/UninstallBundle.java
Tue Sep 27 11:49:12 2011
> @@ -26,7 +26,12 @@ public class UninstallBundle extends Bun
>
>       protected void doExecute(List<Bundle>  bundles) throws Exception {
>           for (Bundle bundle : bundles) {
> -            bundle.uninstall();
> +            try {
> +                bundle.uninstall();
> +            } catch (Exception e) {
> +                System.err.println("Bundle " + bundle.getBundleId() + " didn't uninstall
correctly");
> +                e.printStackTrace();
> +            }
>           }
>       }

Likewise, perhaps System.err.println("Bundle " + bundle.getBundleId() + " failed to properly
uninstall; if still present may be in an invalid state.");  gives a clearer warning to the
user not to rely on that bundle.

Regards,
Glen





>
>





-- 
Glen Mazza
Talend - http://www.talend.com/ai
Blog - http://www.jroller.com/gmazza
Twitter - glenmazza


Mime
View raw message