[GitHub] brooklyn-server pull request: Suppress exceptions in the isRunning...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/brooklyn-server/pull/145


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request: Suppress exceptions in the isRunning...

[aledsage]

Github user aledsage commented on the pull request:

https://github.com/apache/brooklyn-server/pull/145#issuecomment-220293249
  
A couple of minor thoughts, but merging now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request: Suppress exceptions in the isRunning...

[aledsage]

Github user aledsage commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/145#discussion_r63859261
  
--- Diff: 
software/base/src/main/java/org/apache/brooklyn/entity/software/base/VanillaWindowsProcessWinRmDriver.java
 ---
@@ -80,9 +84,22 @@ public void launch() {
 
 @Override
 public boolean isRunning() {
-int exitCode = executeCommandInTask(
-
getEntity().getConfig(VanillaWindowsProcess.CHECK_RUNNING_COMMAND),
-
getEntity().getConfig(VanillaWindowsProcess.CHECK_RUNNING_POWERSHELL_COMMAND), 
"is-running-command");
+int exitCode = 1;
+try {
+exitCode = executeCommandInTask(
+
getEntity().getConfig(VanillaWindowsProcess.CHECK_RUNNING_COMMAND),
+
getEntity().getConfig(VanillaWindowsProcess.CHECK_RUNNING_POWERSHELL_COMMAND), 
"is-running-command");
+} catch (Exception e) {
+// If machine is restarting, then will get WinRM IOExceptions 
- don't propagate such exceptions
+Throwable interestingCause = 
Exceptions.getFirstThrowableOfType(e, WebServiceException.class) != null ?
+Exceptions.getFirstThrowableOfType(e, 
WebServiceException.class/*Wraps Soap exceptions*/) : 
Exceptions.getFirstThrowableOfType(e, Fault.class/*Wraps IO exceptions*/);
+if (interestingCause != null) {
+LOG.warn(getEntity() + " isRunning check failed. Executing 
WinRM command failed.", e);
--- End diff --

I wonder how many times / how often we might get this exception logged if 
the VM is rebooting for several minutes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request: Suppress exceptions in the isRunning...

[aledsage]

Github user aledsage commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/145#discussion_r63859115
  
--- Diff: 
software/base/src/main/java/org/apache/brooklyn/entity/software/base/VanillaWindowsProcessWinRmDriver.java
 ---
@@ -80,9 +84,22 @@ public void launch() {
 
 @Override
 public boolean isRunning() {
-int exitCode = executeCommandInTask(
-
getEntity().getConfig(VanillaWindowsProcess.CHECK_RUNNING_COMMAND),
-
getEntity().getConfig(VanillaWindowsProcess.CHECK_RUNNING_POWERSHELL_COMMAND), 
"is-running-command");
+int exitCode = 1;
--- End diff --

Very minor (for next time). I wouldn't bother giving this an initial value. 
It will always get a value inside the try-block, and the catch block will 
always either return or throw. So the initial value of `1` will always get 
overwritten. Better to be explicit about that by not giving it any initial 
value.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request: Suppress exceptions in the isRunning...

[nakomis]

Github user nakomis commented on the pull request:

https://github.com/apache/brooklyn-server/pull/145#issuecomment-220266139
  
A future improvement would be for 
`AbstractSoftwareProcessWinRmDriver.rebootAndWait` to set a sensor to indicate 
that the machine is rebooting, which could be checked in the catch block, but I 
don't think that's worth holding up this PR. Other than that, LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request: Suppress exceptions in the isRunning...

[aledsage]

Github user aledsage commented on the pull request:

https://github.com/apache/brooklyn-server/pull/145#issuecomment-220257668
  
Looks good. One minor comment worth adding to the code, then good to merge.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request: Suppress exceptions in the isRunning...

[bostko]

GitHub user bostko opened a pull request:

https://github.com/apache/brooklyn-server/pull/145

Suppress exceptions in the isRunning check for Windows

Some Windows Blueprints require restart.
That's why for the isRunning check,
I catched WinRM IO Exceptions which could happen because of the restart.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/bostko/brooklyn-server windows_is_running

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/brooklyn-server/pull/145.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #145


commit 2135787c92bca87a30a422553c445138a4c0e9cb
Author: Valentin Aitken 
Date:   2016-05-18T20:44:14Z

Suppress exceptions in the isRunning check for Windows

Some Windows Blueprints require restart.
That's why for the isRunning check,
I catched WinRM IO Exceptions which could happen because of the restart.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---