Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/757#discussion_r35395212
--- Diff:
core/src/main/java/brooklyn/util/internal/ssh/ShellAbstractTool.java ---
@@ -105,7 +105,7 @@ protected static Boolean hasVal(Map<String,?> map,
ConfigKey<?> keyC) {
public static <T> T getOptionalVal(Map<String,?> map, ConfigKey<T>
keyC) {
if (keyC==null) return null;
String key = keyC.getName();
- if (map!=null && map.containsKey(key)) {
+ if (map!=null && map.containsKey(key) && map.get(key) != null) {
--- End diff --
I'll go with this and merge it.
However, I'm slightly hesitant: in some places, it's nice to be able to
explicitly supply null so that it clears the value rather than it using the
default. For example, in the use of JcloudsLocation I want to be able to set
the privateKeyFile to none, rather than it picking the default of
"~/.ssh/id_rsa". But I could probably still do that with a blank string. So not
sure if there are any valid use-cases for explicitly setting null.
---
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.
---
