brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From henev...@apache.org
Subject [28/31] git commit: code review
Date Tue, 29 Jul 2014 19:32:23 GMT
code review


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/339736db
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/339736db
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/339736db

Branch: refs/heads/master
Commit: 339736dba4b4870c1bad19641c8e9dca5e8be826
Parents: 6b4b9e5
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Authored: Tue Jul 29 15:18:44 2014 -0400
Committer: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Committed: Tue Jul 29 15:18:44 2014 -0400

----------------------------------------------------------------------
 .../brooklyn/entity/trait/StartableMethods.java |  2 +-
 .../ha/HighAvailabilityManagerImpl.java         |  1 +
 .../java/brooklyn/util/task/DynamicTasks.java   |  6 +++---
 .../java/brooklyn/util/task/ssh/SshTasks.java   | 21 ++++++++++++++++----
 .../entity/proxy/nginx/NginxSshDriver.java      |  2 +-
 5 files changed, 23 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/339736db/core/src/main/java/brooklyn/entity/trait/StartableMethods.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/trait/StartableMethods.java b/core/src/main/java/brooklyn/entity/trait/StartableMethods.java
index bab61e5..e8f1039 100644
--- a/core/src/main/java/brooklyn/entity/trait/StartableMethods.java
+++ b/core/src/main/java/brooklyn/entity/trait/StartableMethods.java
@@ -82,7 +82,7 @@ public class StartableMethods {
             }
             try {
                 TaskAdaptable<Void> task = TaskTags.markInessential(Effectors.invocation((Entity)entity,
Startable.STOP, Collections.emptyMap()));
-                DynamicTasks.queueIfPossible(task).orSubmitAsync((Entity)entity).andWaitForSuccess();
+                DynamicTasks.submit(task, (Entity)entity).getUnchecked();
             } catch (Exception e) {
                 log.warn("Error stopping "+entity+"; continuing with shutdown", e);
                 exceptions.add(e);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/339736db/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java b/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java
index 5a13d31..c205ebe 100644
--- a/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java
+++ b/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java
@@ -544,6 +544,7 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager
{
                             t.cancel(true);
                         } catch (Exception e) {
                             Exceptions.propagateIfFatal(e);
+                            LOG.debug("Error cancelling "+t+" on "+entity+" (will warn when
all tasks are cancelled): "+e, e);
                             exceptions.add(e);
                         }
                     }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/339736db/core/src/main/java/brooklyn/util/task/DynamicTasks.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/util/task/DynamicTasks.java b/core/src/main/java/brooklyn/util/task/DynamicTasks.java
index 3957b55..c5c17c9 100644
--- a/core/src/main/java/brooklyn/util/task/DynamicTasks.java
+++ b/core/src/main/java/brooklyn/util/task/DynamicTasks.java
@@ -156,8 +156,8 @@ public class DynamicTasks {
          * <p>
          * throws if there are any errors
          */
-        public void andWaitForSuccess() {
-            task.getUnchecked();
+        public T andWaitForSuccess() {
+            return task.getUnchecked();
         }
         public void orCancel() {
             if (!wasQueued()) {
@@ -332,5 +332,5 @@ public class DynamicTasks {
     public static <T> Task<T> submit(TaskAdaptable<T> task, Entity entity)
{
         return queueIfPossible(task).orSubmitAsync(entity).asTask();
     }
-    
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/339736db/core/src/main/java/brooklyn/util/task/ssh/SshTasks.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/util/task/ssh/SshTasks.java b/core/src/main/java/brooklyn/util/task/ssh/SshTasks.java
index 07e0a8a..407d328 100644
--- a/core/src/main/java/brooklyn/util/task/ssh/SshTasks.java
+++ b/core/src/main/java/brooklyn/util/task/ssh/SshTasks.java
@@ -38,6 +38,7 @@ import brooklyn.management.ManagementContext;
 import brooklyn.management.Task;
 import brooklyn.management.TaskAdaptable;
 import brooklyn.management.TaskFactory;
+import brooklyn.management.TaskQueueingContext;
 import brooklyn.util.ResourceUtils;
 import brooklyn.util.config.ConfigBag;
 import brooklyn.util.internal.ssh.SshTool;
@@ -132,7 +133,19 @@ public class SshTasks {
     }
 
     @Beta
-    public static enum OnFailingTask { FAIL, WARN_OR_FAIL_INESSENTIAL_IF_DYNAMIC, WARN_IN_LOG_ONLY,
IGNORE }
+    public static enum OnFailingTask { 
+        FAIL,
+        /** issues a warning, sometimes implemented as marking the task inessential and failing
it if it appears
+         * we are in a dynamic {@link TaskQueueingContext};
+         * useful because this way the warning appears to the user;
+         * but note that the check is done against the calling thread so use with some care
+         * (and thus this enum is currently here rather then elsewhere) */
+        WARN_OR_IF_DYNAMIC_FAIL_MARKING_INESSENTIAL,
+        /** issues a warning in the log if the task fails, otherwise swallows it */
+        WARN_IN_LOG_ONLY, 
+        /** not even a warning if the task fails (the caller is expected to handle it as
appropriate) */
+        IGNORE }
+    
     public static ProcessTaskFactory<Boolean> dontRequireTtyForSudo(SshMachineLocation
machine, final boolean failIfCantSudo) {
         return dontRequireTtyForSudo(machine, failIfCantSudo ? OnFailingTask.FAIL : OnFailingTask.WARN_IN_LOG_ONLY);
     }
@@ -140,7 +153,7 @@ public class SshTasks {
      * also gives nice warnings if sudo is not permitted */
     public static ProcessTaskFactory<Boolean> dontRequireTtyForSudo(SshMachineLocation
machine, OnFailingTask onFailingTaskRequested) {
         final OnFailingTask onFailingTask;
-        if (onFailingTaskRequested==OnFailingTask.WARN_OR_FAIL_INESSENTIAL_IF_DYNAMIC) {
+        if (onFailingTaskRequested==OnFailingTask.WARN_OR_IF_DYNAMIC_FAIL_MARKING_INESSENTIAL)
{
             if (DynamicTasks.getTaskQueuingContext()!=null)
                 onFailingTask = onFailingTaskRequested;
             else
@@ -171,10 +184,10 @@ public class SshTasks {
                 }
                 Streams.logStreamTail(log, "STDERR of sudo setup problem", Streams.byteArrayOfString(task.getStderr()),
1024);
                 
-                if (onFailingTask==OnFailingTask.WARN_OR_FAIL_INESSENTIAL_IF_DYNAMIC) {
+                if (onFailingTask==OnFailingTask.WARN_OR_IF_DYNAMIC_FAIL_MARKING_INESSENTIAL)
{
                     Tasks.markInessential();
                 }
-                if (onFailingTask==OnFailingTask.FAIL || onFailingTask==OnFailingTask.WARN_OR_FAIL_INESSENTIAL_IF_DYNAMIC)
{
+                if (onFailingTask==OnFailingTask.FAIL || onFailingTask==OnFailingTask.WARN_OR_IF_DYNAMIC_FAIL_MARKING_INESSENTIAL)
{
                     throw new IllegalStateException("Passwordless sudo is required for "+task.getMachine().getUser()+"@"+task.getMachine().getAddress().getHostName()+
                             (entity!=null ? " ("+entity+")" : ""));
                 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/339736db/software/webapp/src/main/java/brooklyn/entity/proxy/nginx/NginxSshDriver.java
----------------------------------------------------------------------
diff --git a/software/webapp/src/main/java/brooklyn/entity/proxy/nginx/NginxSshDriver.java
b/software/webapp/src/main/java/brooklyn/entity/proxy/nginx/NginxSshDriver.java
index df261c0..81e74fd 100644
--- a/software/webapp/src/main/java/brooklyn/entity/proxy/nginx/NginxSshDriver.java
+++ b/software/webapp/src/main/java/brooklyn/entity/proxy/nginx/NginxSshDriver.java
@@ -116,7 +116,7 @@ public class NginxSshDriver extends AbstractSoftwareProcessSshDriver implements
     @Override
     public void install() {
         // inessential here, installation will fail later if it needs to sudo (eg if using
port 80)
-        DynamicTasks.queueIfPossible(SshTasks.dontRequireTtyForSudo(getMachine(), OnFailingTask.WARN_OR_FAIL_INESSENTIAL_IF_DYNAMIC)).orSubmitAndBlock();
+        DynamicTasks.queueIfPossible(SshTasks.dontRequireTtyForSudo(getMachine(), OnFailingTask.WARN_OR_IF_DYNAMIC_FAIL_MARKING_INESSENTIAL)).orSubmitAndBlock();
 
         DownloadResolver nginxResolver = mgmt().getEntityDownloadsManager().newDownloader(this);
         List<String> nginxUrls = nginxResolver.getTargets();


Mime
View raw message