This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit 15374e4f5b551e0ca95f4f52e3d472a745480b6f Author: Alex Heneveld <[email protected]> AuthorDate: Sun Jun 18 01:03:59 2023 +0100 workflow tidies better names for tasks, better rest api error reporting, remove value set other metadata as now in workflowScratchUpdates --- .../apache/brooklyn/core/workflow/WorkflowEffector.java | 2 +- .../core/workflow/WorkflowExecutionContext.java | 17 ++++++++++++++--- .../apache/brooklyn/core/workflow/WorkflowPolicy.java | 2 +- .../workflow/WorkflowStepInstanceExecutionContext.java | 5 +++++ .../workflow/steps/appmodel/SetConfigWorkflowStep.java | 2 +- .../steps/variables/SetVariableWorkflowStep.java | 5 +++-- .../steps/variables/TransformVariableWorkflowStep.java | 5 +++-- .../core/workflow/WorkflowPersistReplayErrorsTest.java | 2 +- .../launcher/blueprints/SimpleBlueprintTest.java | 11 +++++++++++ 9 files changed, 40 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowEffector.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowEffector.java index 5844ac02e1..ca85db81e7 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowEffector.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowEffector.java @@ -104,7 +104,7 @@ public class WorkflowEffector extends AddEffectorInitializerAbstract implements WorkflowExecutionContext.WorkflowContextType.EFFECTOR, effector.getName() + " (workflow effector)", ConfigBag.newInstance(this.definitionParams), effector.getParameters().stream().map(Effectors::asConfigKey).collect(Collectors.toSet()), invocationParams, - getFlagsForTaskInvocationAt(entity, effector, invocationParams)); + getFlagsForTaskInvocationAt(entity, effector, invocationParams), effector.getName()); Task<Object> task = w.getTask(true).get(); if (parentInitializer!=null) { // allow the parent to record the child workflow _before_ the child workflow gets persisted diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java index ec05fcb380..015d9e8e10 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java @@ -261,6 +261,12 @@ public class WorkflowExecutionContext { public static WorkflowExecutionContext newInstanceUnpersistedWithParent(BrooklynObject entityOrAdjunctWhereRunning, WorkflowExecutionContext parent, WorkflowContextType wcType, String name, ConfigBag paramsDefiningWorkflow, Collection<ConfigKey<?>> extraConfigKeys, ConfigBag extraInputs, Map<String, Object> optionalTaskFlags) { + return newInstanceUnpersistedWithParent(entityOrAdjunctWhereRunning, parent, wcType, name, paramsDefiningWorkflow, extraConfigKeys, extraInputs, optionalTaskFlags, null); + } + + public static WorkflowExecutionContext newInstanceUnpersistedWithParent(BrooklynObject entityOrAdjunctWhereRunning, WorkflowExecutionContext parent, + WorkflowContextType wcType, String name, ConfigBag paramsDefiningWorkflow, + Collection<ConfigKey<?>> extraConfigKeys, ConfigBag extraInputs, Map<String, Object> optionalTaskFlags, String optionalTaskName) { // parameter defs Map<String,ConfigKey<?>> parameters = MutableMap.of(); @@ -289,7 +295,7 @@ public class WorkflowExecutionContext { input, paramsDefiningWorkflow.get(WorkflowCommonConfig.OUTPUT), WorkflowReplayUtils.updaterForReplayableAtWorkflow(paramsDefiningWorkflow, wcType == WorkflowContextType.NESTED_WORKFLOW), - optionalTaskFlags); + optionalTaskFlags, optionalTaskName); w.getStepsResolved(); // ensure steps resolve at this point; should be true even if condition doesn't apply (though input might not be valid without condition) @@ -311,6 +317,11 @@ public class WorkflowExecutionContext { protected WorkflowExecutionContext(BrooklynObject entityOrAdjunctWhereRunning, WorkflowExecutionContext parent, String name, List<Object> stepsDefinition, Map<String,Object> input, Object output, Consumer<WorkflowExecutionContext> replayableInitializer, Map<String, Object> optionalTaskFlags) { + this(entityOrAdjunctWhereRunning, parent, name, stepsDefinition, input, output, replayableInitializer, optionalTaskFlags, null); + } + protected WorkflowExecutionContext(BrooklynObject entityOrAdjunctWhereRunning, WorkflowExecutionContext parent, String name, + List<Object> stepsDefinition, Map<String,Object> input, Object output, + Consumer<WorkflowExecutionContext> replayableInitializer, Map<String, Object> optionalTaskFlags, String optionalTaskName) { initParent(parent); this.name = name; this.adjunct = entityOrAdjunctWhereRunning instanceof Entity ? null : entityOrAdjunctWhereRunning; @@ -321,7 +332,7 @@ public class WorkflowExecutionContext { this.outputDefinition = output; if (replayableInitializer!=null) replayableInitializer.accept(this); - TaskBuilder<Object> tb = Tasks.builder().dynamic(true); + TaskBuilder<Object> tb = Tasks.builder().displayName(optionalTaskName).dynamic(true); if (optionalTaskFlags!=null) tb.flags(optionalTaskFlags); if (Strings.isBlank(tb.getDisplayName())) tb.displayName(name); task = tb.body(new Body()).build(); @@ -449,7 +460,7 @@ public class WorkflowExecutionContext { } if (task==null) { - if (taskId !=null) { + if (taskId!=null) { task = (Task<Object>) getManagementContext().getExecutionManager().getTask(taskId); } if (task==null) { diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowPolicy.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowPolicy.java index 88324b3d9b..e54b4b0b89 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowPolicy.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowPolicy.java @@ -145,7 +145,7 @@ public class WorkflowPolicy<T> extends AbstractPolicy { Set<PollConfig> pollConfigs = MutableSet.of(pc); poller.schedulePoll(this, pollConfigs, new WorkflowSensor.WorkflowPollCallable( - getDisplayName() + " (workflow)", config().getBag(), this), new PolicyNoOpPollHandler()); + getDisplayName() + " (policy)", config().getBag(), this), new PolicyNoOpPollHandler()); if (!isSuspended()) resume(); } diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java index 94c2da19ee..2e421cddcf 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java @@ -18,8 +18,10 @@ */ package org.apache.brooklyn.core.workflow; +import com.fasterxml.jackson.annotation.JsonGetter; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonSetter; import com.google.common.reflect.TypeToken; import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.mgmt.ManagementContext; @@ -28,6 +30,7 @@ import org.apache.brooklyn.core.entity.internal.ConfigUtilsInternal; import org.apache.brooklyn.core.mgmt.BrooklynTaskTags; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.collections.MutableSet; +import org.apache.brooklyn.util.exceptions.Exceptions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -71,6 +74,8 @@ public class WorkflowStepInstanceExecutionContext { /** set if the step is in an error handler context, containing the error being handled, * or if the step had an error that was not handled */ Throwable error; + @JsonGetter("error") String getErrorForJson() { return Exceptions.collapseText(error); } + @JsonSetter("error") void setErrorFromJaon(String error) { this.error = new RuntimeException(error); } /** set if there was an error handled locally */ String errorHandlerTaskId; diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/SetConfigWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/SetConfigWorkflowStep.java index b17757f41a..958f94adb4 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/SetConfigWorkflowStep.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/SetConfigWorkflowStep.java @@ -45,13 +45,13 @@ public class SetConfigWorkflowStep extends WorkflowStepDefinition { if (config ==null) throw new IllegalArgumentException("Config key name is required"); String configName = context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT, config.name, String.class); if (Strings.isBlank(configName)) throw new IllegalArgumentException("Config key name is required"); + // see note on type in SetSensorWorkflowStep TypeToken<?> type = context.lookupType(config.type, () -> TypeToken.of(Object.class)); Object resolvedValue = context.getInput(VALUE.getName(), type); Entity entity = config.entity; if (entity==null) entity = context.getEntity(); Object oldValue = entity.config().set((ConfigKey<Object>) ConfigKeys.newConfigKey(type, configName), resolvedValue); - // see note on type in SetSensorWorkflowStep context.noteOtherMetadata("Value set", resolvedValue); if (oldValue!=null) context.noteOtherMetadata("Previous value", oldValue); diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/SetVariableWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/SetVariableWorkflowStep.java index 63f2a893d0..a9b112a520 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/SetVariableWorkflowStep.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/SetVariableWorkflowStep.java @@ -104,8 +104,9 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition { Object resolvedValue = new ConfigurableInterpolationEvaluation(context, type, unresolvedValue, context.getInputOrDefault(INTERPOLATION_MODE), context.getInputOrDefault(INTERPOLATION_ERRORS)).evaluate(); Object oldValue = setWorkflowScratchVariableDotSeparated(context, name, resolvedValue); - context.noteOtherMetadata("Value set", resolvedValue); - if (oldValue!=null) context.noteOtherMetadata("Previous value", oldValue); + // these are easily inferred from workflow vars +// context.noteOtherMetadata("Value set", resolvedValue); +// if (oldValue!=null) context.noteOtherMetadata("Previous value", oldValue); return context.getPreviousStepOutput(); } diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformVariableWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformVariableWorkflowStep.java index 390dbcae96..e014bb661c 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformVariableWorkflowStep.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformVariableWorkflowStep.java @@ -147,8 +147,9 @@ public class TransformVariableWorkflowStep extends WorkflowStepDefinition { if (name!=null) { Object oldValue = setWorkflowScratchVariableDotSeparated(context, name, v); - context.noteOtherMetadata("Value set", v); - if (oldValue != null) context.noteOtherMetadata("Previous value", oldValue); + // these are easily inferred from workflow vars +// context.noteOtherMetadata("Value set", v); +// if (oldValue != null) context.noteOtherMetadata("Previous value", oldValue); if (context.getOutput()!=null) throw new IllegalStateException("Transform that produces output results cannot be used when setting a variable"); if (STEP_TARGET_NAME_FOR_END.equals(context.next)) throw new IllegalStateException("Return transform cannot be used when setting a variable"); diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java index 8afc16f79d..7f3843b79c 100644 --- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java @@ -634,7 +634,7 @@ public class WorkflowPersistReplayErrorsTest extends RebindTestFixture<BasicAppl m -> m.matches("Starting step .*-1 in task .*"), m -> m.matches("Error in step '1 - invoke-effector does-not-exist'; rethrowing: No effector matching 'does-not-exist'"), m -> m.matches("Error in workflow 'myWorkflow .workflow effector.' around step .*-1, running error handler"), - m -> m.matches("Encountered error in workflow .*/.* 'myWorkflow .workflow effector.' .handler present.: No effector matching 'does-not-exist'"), + m -> m.matches("Encountered error in workflow .*/.* 'myWorkflow' .handler present.: No effector matching 'does-not-exist'"), m -> m.matches("Starting .*-error-handler with 1 step in task .*"), m -> m.matches("Starting .*-error-handler-1 in task .*"), m -> m.matches("Completed handler .*-error-handler; no next step indicated so proceeding to default next step"), diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/blueprints/SimpleBlueprintTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/blueprints/SimpleBlueprintTest.java index 7adcc91d5b..54ce84e739 100644 --- a/launcher/src/test/java/org/apache/brooklyn/launcher/blueprints/SimpleBlueprintTest.java +++ b/launcher/src/test/java/org/apache/brooklyn/launcher/blueprints/SimpleBlueprintTest.java @@ -19,13 +19,24 @@ package org.apache.brooklyn.launcher.blueprints; import org.apache.brooklyn.api.entity.Application; +import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.core.entity.Dumper; +import org.apache.brooklyn.core.workflow.WorkflowBasicTest; import org.apache.brooklyn.entity.stock.BasicEntity; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; /** This does rebind. See SimpleBlueprintNonRebindTest for an example with rebind disabled. */ public class SimpleBlueprintTest extends AbstractBlueprintTest { + @Override + protected ManagementContext decorateManagementContext(ManagementContext mgmt) { + mgmt = super.decorateManagementContext(mgmt); + // make workflow step types available + WorkflowBasicTest.addWorkflowStepTypes(mgmt); + return mgmt; + } + @Override protected boolean isViewerEnabled() { return true;
