LIVY-287. Add Deprecation to Livy Configurations and update naming. (a…

…pache#300)

Updates to Livy configurations

- Added config deprecation with alternatives to ClientConf, HTTPConf, RSCConf, and LivyConf.
- Added framework for deprecation without alternatives when the need arises.
- Updated naming conventions in code and templates to use - instead of _ or camelCase and deprecated previous configs.
- Updated TestClientConf and added a new test.

client-common/pom.xml

@@ -45,5 +45,10 @@
       <artifactId>jackson-databind</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-api</artifactId>
+      <scope>provided</scope>
+    </dependency>
   </dependencies>
 </project>

client-common/src/main/java/com/cloudera/livy/client/common/ClientConf.java

@@ -28,6 +28,9 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.cloudera.livy.annotations.Private;
 
 /**
@@ -37,6 +40,8 @@
 public abstract class ClientConf<T extends ClientConf>
   implements Iterable<Map.Entry<String, String>> {
 
+  protected Logger LOG = LoggerFactory.getLogger(getClass());
+
   public static interface ConfEntry {
 
     /** The key in the configuration file. */
@@ -71,24 +76,37 @@ protected ClientConf(Properties config) {
     this.config = new ConcurrentHashMap<>();
     if (config != null) {
       for (String key : config.stringPropertyNames()) {
+        logDeprecationWarning(key);
         this.config.put(key, config.getProperty(key));
       }
     }
   }
 
   public String get(String key) {
-    return config.get(key);
+    String val = config.get(key);
+    if (val != null) {
+      return val;
+    }
+    DeprecatedConf depConf = getConfigsWithAlternatives().get(key);
+    if (depConf != null) {
+      return config.get(depConf.key());
+    } else {
+      return val;
+    }
   }
 
   @SuppressWarnings("unchecked")
   public T set(String key, String value) {
+    logDeprecationWarning(key);
     config.put(key, value);
     return (T) this;
   }
 
   @SuppressWarnings("unchecked")
   public T setIfMissing(String key, String value) {
-    config.putIfAbsent(key, value);
+    if (config.putIfAbsent(key, value) == null) {
+      logDeprecationWarning(key);
+    }
     return (T) this;
   }
 
@@ -163,6 +181,7 @@ public T set(ConfEntry e, Object value) {
     if (value == null) {
       config.remove(e.key());
     } else {
+      logDeprecationWarning(e.key());
       config.put(e.key(), value.toString());
     }
     return (T) this;
@@ -176,7 +195,7 @@ public Iterator<Map.Entry<String, String>> iterator() {
   private String get(ConfEntry e, Class<?> requestedType) {
     check(getType(e.dflt()).equals(requestedType), "Invalid type conversion requested for %s.",
       e.key());
-    return config.get(e.key());
+    return this.get(e.key());
   }
 
   private boolean typesMatch(Object test, Object expected) {
@@ -193,4 +212,40 @@ private void check(boolean test, String message, Object... args) {
     }
   }
 
+  /** Logs a warning message if the given config key is deprecated. */
+  private void logDeprecationWarning(String key) {
+    DeprecatedConf altConfs = getConfigsWithAlternatives().get(key);
+    if (altConfs != null) {
+      LOG.warn("The configuration key " + altConfs.key() + " has been deprecated as of Livy "
+        + altConfs.version() + " and may be removed in the future. Please use the new key "
+        + key + " instead.");
+      return;
+    }
+
+    DeprecatedConf depConfs = getDeprecatedConfigs().get(key);
+    if (depConfs != null) {
+      LOG.warn("The configuration key " + depConfs.key() + " has been deprecated as of Livy "
+        + depConfs.version() + " and may be removed in the future. "
+        + depConfs.deprecationMessage());
+    }
+  }
+
+  /** Maps valid key to DeprecatedConf with the deprecated key. */
+  protected abstract Map<String, DeprecatedConf> getConfigsWithAlternatives();
+
+  /** Maps deprecated key to DeprecatedConf with the same key. */
+  protected abstract Map<String, DeprecatedConf> getDeprecatedConfigs();
+
+  public static interface DeprecatedConf {
+
+    /** The key in the configuration file. */
+    String key();
+
+    /** The Livy version in which the key was deprecated. */
+    String version();
+
+    /** Message to include in the deprecation warning for configs without alternatives */
+    String deprecationMessage();
+  }
+
 }

client-common/src/test/java/com/cloudera/livy/client/common/TestClientConf.java

@@ -18,6 +18,8 @@
 
 package com.cloudera.livy.client.common;
 
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Properties;
 
@@ -122,6 +124,31 @@ public void testTimeWithoutDefault() {
     conf.getTimeAsMs(TestConf.Entry.TIME_NO_DEFAULT);
   }
 
+
+  @Test
+  public void testDeprecation() {
+    TestConf conf = new TestConf(null);
+
+    assertNull(conf.get("depKey"));
+    assertNull(conf.get("dep_alt"));
+    assertNull(conf.get("new-key"));
+    assertEquals("value", conf.get(TestConf.Entry.NEW_CONF));
+
+    TestConf depProps = new TestConf(null);
+    depProps.set("depKey", "dep-val");
+    depProps.set("dep_alt", "alt-val");
+    conf.setAll(depProps);
+    assertEquals("dep-val", conf.get("depKey"));
+    assertEquals("alt-val", conf.get("dep_alt"));
+    assertEquals("alt-val", conf.get(TestConf.Entry.NEW_CONF));
+    assertEquals("alt-val", conf.get("new-key"));
+
+    conf.set("new-key", "new-val");
+    assertEquals("new-val", conf.get(TestConf.Entry.NEW_CONF));
+    assertEquals("alt-val", conf.get("dep_alt"));
+    assertEquals("new-val", conf.get("new-key"));
+  }
+
   private static class TestConf extends ClientConf<TestConf> {
 
     static enum Entry implements ConfEntry {
@@ -131,7 +158,8 @@ static enum Entry implements ConfEntry {
       INT("int", 42),
       LONG("long", 84L),
       TIME("time", "168ms"),
-      TIME_NO_DEFAULT("time2", null);
+      TIME_NO_DEFAULT("time2", null),
+      NEW_CONF("new-key", "value");
 
       private final String key;
       private final Object dflt;
@@ -153,6 +181,51 @@ private Entry(String key, Object dflt) {
       super(p);
     }
 
+    private static final Map<String, DeprecatedConf> configsWithAlternatives
+      = Collections.unmodifiableMap(new HashMap<String, DeprecatedConf>() {{
+      put(TestConf.Entry.NEW_CONF.key, DepConf.DEP_WITH_ALT);
+    }});
+
+    private static final Map<String, DeprecatedConf> deprecatedConfigs
+      = Collections.unmodifiableMap(new HashMap<String, DeprecatedConf>() {{
+      put(DepConf.DEP_NO_ALT.key, DepConf.DEP_NO_ALT);
+    }});
+
+    protected Map<String, DeprecatedConf> getConfigsWithAlternatives() {
+      return configsWithAlternatives;
+    }
+
+    protected Map<String, DeprecatedConf> getDeprecatedConfigs() {
+      return deprecatedConfigs;
+    }
+
+    static enum DepConf implements DeprecatedConf {
+      DEP_WITH_ALT("dep_alt", "0.4"),
+      DEP_NO_ALT("depKey", "1.0");
+
+      private final String key;
+      private final String version;
+      private final String deprecationMessage;
+
+      private DepConf(String key, String version) {
+        this(key, version, "");
+      }
+
+      private DepConf(String key, String version, String deprecationMessage) {
+        this.key = key;
+        this.version = version;
+        this.deprecationMessage = deprecationMessage;
+      }
+
+      @Override
+      public String key() { return key; }
+
+      @Override
+      public String version() { return version; }
+
+      @Override
+      public String deprecationMessage() { return deprecationMessage; }
+    }
   }
 
 }

client-http/src/main/java/com/cloudera/livy/client/http/HttpConf.java

@@ -18,19 +18,24 @@
 
 package com.cloudera.livy.client.http;
 
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.Properties;
 
 import com.cloudera.livy.client.common.ClientConf;
 
 class HttpConf extends ClientConf<HttpConf> {
 
+  private static final String HTTP_CONF_PREFIX = "livy.client.http.";
+
   static enum Entry implements ConfEntry {
     CONNECTION_TIMEOUT("connection.timeout", "10s"),
     CONNECTION_IDLE_TIMEOUT("connection.idle.timeout", "10m"),
     SOCKET_TIMEOUT("connection.socket.timeout", "5m"),
 
-    JOB_INITIAL_POLL_INTERVAL("job.initial_poll_interval", "100ms"),
-    JOB_MAX_POLL_INTERVAL("job.max_poll_interval", "5s"),
+    JOB_INITIAL_POLL_INTERVAL("job.initial-poll-interval", "100ms"),
+    JOB_MAX_POLL_INTERVAL("job.max-poll-interval", "5s"),
 
     CONTENT_COMPRESS_ENABLE("content.compress.enable", true),
 
@@ -44,7 +49,7 @@ static enum Entry implements ConfEntry {
     private final Object dflt;
 
     private Entry(String key, Object dflt) {
-      this.key = "livy.client.http." + key;
+      this.key = HTTP_CONF_PREFIX + key;
       this.dflt = dflt;
     }
 
@@ -80,4 +85,52 @@ private Entry(String key, Object dflt) {
   boolean isSpnegoEnabled() {
     return getBoolean(Entry.SPNEGO_ENABLED);
   }
+
+  private static final Map<String, DeprecatedConf> configsWithAlternatives
+    = Collections.unmodifiableMap(new HashMap<String, DeprecatedConf>() {{
+      put(HttpConf.Entry.JOB_INITIAL_POLL_INTERVAL.key, DepConf.JOB_INITIAL_POLL_INTERVAL);
+      put(HttpConf.Entry.JOB_MAX_POLL_INTERVAL.key, DepConf.JOB_MAX_POLL_INTERVAL);
+  }});
+
+  // Maps deprecated key to DeprecatedConf with the same key.
+  // There are no deprecated configs without alternatives currently.
+  private static final Map<String, DeprecatedConf> deprecatedConfigs
+    = Collections.unmodifiableMap(new HashMap<String, DeprecatedConf>());
+
+  protected Map<String, DeprecatedConf> getConfigsWithAlternatives() {
+    return configsWithAlternatives;
+  }
+
+  protected Map<String, DeprecatedConf> getDeprecatedConfigs() {
+    return deprecatedConfigs;
+  }
+
+  static enum DepConf implements DeprecatedConf {
+    JOB_INITIAL_POLL_INTERVAL("job.initial_poll_interval", "0.4"),
+    JOB_MAX_POLL_INTERVAL("job.max_poll_interval", "0.4");
+
+    private final String key;
+    private final String version;
+    private final String deprecationMessage;
+
+    private DepConf(String key, String version) {
+      this(key, version, "");
+    }
+
+    private DepConf(String key, String version, String deprecationMessage) {
+      this.key = HTTP_CONF_PREFIX + key;
+      this.version = version;
+      this.deprecationMessage = deprecationMessage;
+    }
+
+    @Override
+    public String key() { return key; }
+
+    @Override
+    public String version() { return version; }
+
+    @Override
+    public String deprecationMessage() { return deprecationMessage; }
+  }
+
 }

conf/livy-client.conf.template

@@ -22,9 +22,9 @@
 # livy.client.http.connection.idle.timeout = 10m
 
 # Initial interval before polling for Job results
-# livy.client.http.job.initial_poll_interval = 100ms
+# livy.client.http.job.initial-poll-interval = 100ms
 # Maximum interval between successive polls
-# livy.client.http.job.max_poll_interval = 5s
+# livy.client.http.job.max-poll-interval = 5s
 
 #
 # Configurations for Livy RSCClient
@@ -37,10 +37,10 @@
 # livy.rsc.client.auth.secret =
 
 # Timeout when stopping a rsc client
-# livy.rsc.client.shutdown_timeout = 10s
+# livy.rsc.client.shutdown-timeout = 10s
 
 # Class of the rsc driver to use
-# livy.rsc.driver_class =
+# livy.rsc.driver-class =
 # The kind of rsc session. Examples: pyspark or sparkr
 # livy.rsc.session.kind =
 
@@ -58,10 +58,10 @@
 # livy.rsc.launcher.port = -1
 
 # How long will the RSC wait for a connection for a Livy server before shutting itself down.
-# livy.rsc.server.idle_timeout = 10m
+# livy.rsc.server.idle-timeout = 10m
 
 # The user that should be impersonated when requesting a Livy session
-# livy.rsc.proxy_user =
+# livy.rsc.proxy-user =
 
 # Host or IP adress of the rpc server
 # livy.rsc.rpc.server.address =
@@ -78,9 +78,9 @@
 # livy.rsc.rpc.sasl.qop =
 
 # Time between status checks for cancelled a Job
-# livy.rsc.job_cancel.trigger_interval = 100ms
+# livy.rsc.job-cancel.trigger-interval = 100ms
 # Time before a cancelled a Job is forced into a Cancelled state
-# livy.rsc.job_cancel.timeout = 30s
+# livy.rsc.job-cancel.timeout = 30s
 
 # Number of statements kept in driver's memory
-# livy.rsc.retained_statements = 100
+# livy.rsc.retained-statements = 100