You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2014/07/29 21:32:23 UTC
[28/31] git commit: code review
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 <al...@cloudsoftcorp.com>
Authored: Tue Jul 29 15:18:44 2014 -0400
Committer: Alex Heneveld <al...@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();