ahgittin commented on code in PR #1363: URL: https://github.com/apache/brooklyn-server/pull/1363#discussion_r999125606
########## core/src/main/java/org/apache/brooklyn/core/workflow/steps/LogWorkflowStep.java: ########## @@ -30,26 +33,22 @@ public class LogWorkflowStep extends WorkflowStepDefinition { private static final Logger LOG = LoggerFactory.getLogger(LogWorkflowStep.class); - String message; - - public String getMessage() { - return message; - } + public static final String SHORTHAND = "${message}"; + public static final ConfigKey<String> MESSAGE = ConfigKeys.newStringConfigKey("message"); @Override - public void setShorthand(String value) { - message = value; + public void populateFromShorthand(String value) { + populateFromShorthandTemplate(SHORTHAND, value); } @Override - protected Task<?> newTask(String name, WorkflowExecutionContext workflowExecutionContext) { - return Tasks.create(getDefaultTaskName(workflowExecutionContext), () -> { - - if (Strings.isBlank(message)) { - throw new IllegalArgumentException("Log message is required"); - } - - LOG.info("{}: {}", name, getMessage()); - }); + protected Object doTaskBody(WorkflowStepInstanceExecutionContext context) { + String message = context.getInput(MESSAGE); + if (Strings.isBlank(message)) { + throw new IllegalArgumentException("Log message is required"); + } + // TODO all workflow log messages should include step id as logging MDC, or message to start/end each workflow/task + LOG.info("{}", message); Review Comment: that's what i was thinking here with the TODO. however we do have all that information from the task, and we log which workflows are running in which tasks, so it doesn't feel necessary. we can add it if it is irritating not having it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@brooklyn.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org