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

    https://github.com/apache/brooklyn-server/pull/258#discussion_r70850684
  
    --- Diff: 
core/src/main/java/org/apache/brooklyn/core/effector/ssh/SshCommandEffector.java
 ---
    @@ -65,38 +69,43 @@ public Body(Effector<?> eff, ConfigBag params) {
                 this.effector = eff;
                 this.command = 
Preconditions.checkNotNull(params.get(EFFECTOR_COMMAND), "command must be 
supplied when defining this effector");
                 this.executionDir = params.get(EFFECTOR_EXECUTION_DIR);
    -            // TODO could take a custom "env" aka effectorShellEnv
             }
     
             @Override
             public String call(ConfigBag params) {
    -            String command = this.command;
    -            
    -            command = 
SshCommandSensor.makeCommandExecutingInDirectory(command, executionDir, 
entity());
    -            
    +            String sshCommand = 
SshCommandSensor.makeCommandExecutingInDirectory(command, executionDir, 
entity());
    +
                 MutableMap<String, String> env = MutableMap.of();
    +
                 // first set all declared parameters, including default values
    -            for (ParameterType<?> param: effector.getParameters()) {
    -                env.addIfNotNull(param.getName(), Strings.toString( 
params.get(Effectors.asConfigKey(param)) ));
    +            for (ParameterType<?> param : effector.getParameters()) {
    +                env.addIfNotNull(param.getName(), 
Strings.toString(params.get(Effectors.asConfigKey(param))));
                 }
    -            
    +
                 // then set things from the entities defined shell 
environment, if applicable
    -            
env.putAll(Strings.toStringMap(entity().getConfig(BrooklynConfigKeys.SHELL_ENVIRONMENT),
 ""));
    -            
    -            // if we wanted to resolve the surrounding environment in real 
time -- see above
    -//            Map<String,Object> paramsResolved = (Map<String, Object>) 
Tasks.resolveDeepValue(effectorShellEnv, Map.class, 
entity().getExecutionContext());
    -            
    +            
env.putAll(Strings.toStringMap(entity().config().get(BrooklynConfigKeys.SHELL_ENVIRONMENT),
 ""));
    +
    +            // now add the resolved shell environment entries from our 
configuration
    +            try {
    +                Map<String, Object> effectorEnv = 
params.get(EFFECTOR_SHELL_ENVIRONMENT);
    +                if (effectorEnv != null && effectorEnv.size() > 0) {
    +                    Map<String, Object> envResolved = (Map<String, 
Object>) Tasks.resolveDeepValue(effectorEnv, Object.class, 
entity().getExecutionContext());
    +                    env.putAll(Strings.toStringMap(envResolved, ""));
    +                }
    +            } catch (InterruptedException | ExecutionException e) {
    +                Exceptions.propagateIfFatal(e);
    +            }
    +
                 // finally set the parameters we've been passed; this will 
repeat declared parameters but to no harm,
                 // it may pick up additional values (could be a flag defining 
whether this is permitted or not)
    -            env.putAll(Strings.toStringMap(params.getAllConfig()));
    -            
    -            SshEffectorTasks.SshEffectorTaskFactory<String> t = 
SshEffectorTasks.ssh(command)
    +            env.putAll(Strings.toStringMap(params.getAllConfig(), ""));
    --- End diff --
    
    We need it because it makes the syntax the same between the SSH sensor and 
effector initialisers, and it has the normal and well understood semantics of a 
config key?


---
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 [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to