gaul commented on a change in pull request #135:
URL: https://github.com/apache/jclouds/pull/135#discussion_r820301900



##########
File path: core/src/main/java/org/jclouds/ContextBuilder.java
##########
@@ -370,7 +371,11 @@ private Properties currentStateToUnexpandedProperties() {
          defaults.setProperty(PROPERTY_CREDENTIAL, credential);
       if (overrides.isPresent())
          putAllAsString(overrides.get(), defaults);
-      
putAllAsString(propertiesPrefixedWithJcloudsApiOrProviderId(getSystemProperties(),
 apiMetadata.getId(), providerId), defaults);
+      Map<String, Object> system = 
propertiesPrefixedWithJcloudsApiOrProviderId(getSystemProperties(), 
apiMetadata.getId(), providerId);
+      if (Strings.isNullOrEmpty((String) system.get(PROPERTY_ENDPOINT))) {
+         system.remove(PROPERTY_ENDPOINT);

Review comment:
       Does it make more sense for the application _not_ to set a value when it 
doesn't have a value?  This seems like a weird quirk of an application's 
configuration (environment variables) that makes more sense to address in the 
application than in jclouds itself.
   
   Also I believe that `System.getProperties().remove()` is thread-safe but can 
mess up iteration order and anyway modifying global state is a bad idea.




-- 
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: notifications-unsubscr...@jclouds.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to