You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by pd...@apache.org on 2020/09/17 06:57:32 UTC

[zeppelin] branch master updated: [ZEPPELIN-5044] Refactoring some viewed classes

This is an automated email from the ASF dual-hosted git repository.

pdallig pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git


The following commit(s) were added to refs/heads/master by this push:
     new fbea3e0  [ZEPPELIN-5044] Refactoring some viewed classes
fbea3e0 is described below

commit fbea3e085841c2332f7ba91050706aa21b5be09a
Author: Philipp Dallig <ph...@gmail.com>
AuthorDate: Mon Aug 3 16:59:32 2020 +0200

    [ZEPPELIN-5044] Refactoring some viewed classes
    
    ### What is this PR for?
    During development, I came across code that did not conform to current best practices.
    For example:
      - Logging
        - [Anti-Pattern-1](https://rolf-engelhard.de/2013/03/logging-anti-patterns-part-i/), [Anti-Pattern-2](https://rolf-engelhard.de/2013/04/logging-anti-patterns-part-ii/), [Anti-Pattern-3](https://rolf-engelhard.de/2013/10/logging-anti-patterns-part-iii/)
      - [try-with-resources](https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html)
    
    ### What type of PR is it?
    Refactoring
    
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-5044
    
    ### How should this be tested?
    * Travis CI: https://travis-ci.org/github/Reamer/zeppelin/builds/727295209
    
    ### Questions:
    * Does the licenses files need update? No
    * Is there breaking changes for older versions? No
    * Does this needs documentation? No
    
    Author: Philipp Dallig <ph...@gmail.com>
    
    Closes #3909 from Reamer/refactoring and squashes the following commits:
    
    dcf0417db [Philipp Dallig] Refactoring some viewed classes
---
 .../zeppelin/cassandra/CassandraInterpreter.java   | 12 +++---
 .../apache/zeppelin/geode/GeodeOqlInterpreter.java | 16 ++++----
 .../zeppelin/influxdb/InfluxDBInterpreter.java     |  8 +++-
 .../org/apache/zeppelin/java/JavaInterpreter.java  |  4 +-
 .../org/apache/zeppelin/ksql/KSQLInterpreter.java  |  4 +-
 .../apache/zeppelin/livy/BaseLivyInterpreter.java  | 33 ++++++----------
 .../graph/neo4j/Neo4jCypherInterpreter.java        | 19 ++++++----
 .../java/org/apache/zeppelin/r/IRInterpreter.java  | 12 +-----
 .../java/org/apache/zeppelin/r/RInterpreter.java   |  2 +-
 .../apache/zeppelin/sap/UniverseInterpreter.java   | 16 +++++---
 .../zeppelin/scalding/ScaldingInterpreter.java     | 25 ++++++------
 .../apache/zeppelin/cluster/ClusterManager.java    |  2 +-
 .../ZeppelinClusterMembershipEventListener.java    | 10 ++---
 .../apache/zeppelin/completer/CachedCompleter.java |  7 +---
 .../zeppelin/completer/StringsCompleter.java       |  2 +-
 .../zeppelin/conf/ZeppelinConfiguration.java       |  7 +---
 .../apache/zeppelin/interpreter/Interpreter.java   | 12 +++---
 .../zeppelin/interpreter/InterpreterOutput.java    |  4 +-
 .../remote/RemoteInterpreterServer.java            | 23 ++++++-----
 .../zeppelin/interpreter/remote/YarnUtils.java     |  5 +--
 .../java/org/apache/zeppelin/scheduler/Job.java    |  2 +-
 .../zeppelin/service/InterpreterService.java       | 22 +++++------
 .../zeppelin/interpreter/ConfInterpreter.java      |  5 +--
 .../interpreter/InterpreterInfoSaving.java         |  9 +++--
 .../zeppelin/interpreter/InterpreterSetting.java   | 44 +++++++++++-----------
 .../interpreter/InterpreterSettingManager.java     | 21 +++++------
 .../remote/mock/MockInterpreterAngular.java        |  6 ++-
 27 files changed, 159 insertions(+), 173 deletions(-)

diff --git a/cassandra/src/main/java/org/apache/zeppelin/cassandra/CassandraInterpreter.java b/cassandra/src/main/java/org/apache/zeppelin/cassandra/CassandraInterpreter.java
index e0e5167..9b3daed 100644
--- a/cassandra/src/main/java/org/apache/zeppelin/cassandra/CassandraInterpreter.java
+++ b/cassandra/src/main/java/org/apache/zeppelin/cassandra/CassandraInterpreter.java
@@ -181,7 +181,7 @@ public class CassandraInterpreter extends Interpreter {
     Collection<InetSocketAddress> hosts = new ArrayList<>();
     for (String address : addresses) {
       if (!StringUtils.isBlank(address)) {
-        logger.debug("Adding contact point: {}", address);
+        LOGGER.debug("Adding contact point: {}", address);
         if (InetAddresses.isInetAddress(address)) {
           hosts.add(new InetSocketAddress(address, port));
         } else {
@@ -249,14 +249,14 @@ public class CassandraInterpreter extends Interpreter {
     LOGGER.debug("Session configuration");
     for (Map.Entry<String, Object> entry:
             session.getContext().getConfig().getDefaultProfile().entrySet()) {
-      logger.debug("{} = {}", entry.getKey(), entry.getValue().toString());
+      LOGGER.debug("{} = {}", entry.getKey(), entry.getValue().toString());
     }
     LOGGER.debug("Creating helper");
     helper = new InterpreterLogic(session, properties);
   }
 
   private DriverConfigLoader createLoader() {
-    logger.debug("Creating programmatic config loader");
+    LOGGER.debug("Creating programmatic config loader");
     // start generation of the config
     ProgrammaticDriverConfigLoaderBuilder configBuilder = DriverConfigLoader.programmaticBuilder();
 
@@ -341,12 +341,12 @@ public class CassandraInterpreter extends Interpreter {
     for (String pname: properties.stringPropertyNames()) {
       if (pname.startsWith(DATASTAX_JAVA_DRIVER_PREFIX)) {
         String pvalue = properties.getProperty(pname);
-        logger.info("Custom config values: {} = {}", pname, pvalue);
+        LOGGER.info("Custom config values: {} = {}", pname, pvalue);
         String shortName = pname.substring(DATASTAX_JAVA_DRIVER_PREFIX.length());
         if (optionMap.containsKey(shortName)) {
           allOptions.put(optionMap.get(shortName), pvalue);
         } else {
-          logger.warn("Incorrect option name: {}", pname);
+          LOGGER.warn("Incorrect option name: {}", pname);
         }
       }
     }
@@ -356,7 +356,7 @@ public class CassandraInterpreter extends Interpreter {
     }
 
     DriverConfigLoader loader = configBuilder.endProfile().build();
-    logger.debug("Config loader is created");
+    LOGGER.debug("Config loader is created");
 
     return loader;
   }
diff --git a/geode/src/main/java/org/apache/zeppelin/geode/GeodeOqlInterpreter.java b/geode/src/main/java/org/apache/zeppelin/geode/GeodeOqlInterpreter.java
index 3e91238..a83410c 100644
--- a/geode/src/main/java/org/apache/zeppelin/geode/GeodeOqlInterpreter.java
+++ b/geode/src/main/java/org/apache/zeppelin/geode/GeodeOqlInterpreter.java
@@ -83,7 +83,7 @@ import java.util.Properties;
  */
 public class GeodeOqlInterpreter extends Interpreter {
 
-  private Logger logger = LoggerFactory.getLogger(GeodeOqlInterpreter.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(GeodeOqlInterpreter.class);
 
   private static final char NEWLINE = '\n';
   private static final char TAB = '\t';
@@ -113,7 +113,7 @@ public class GeodeOqlInterpreter extends Interpreter {
 
   @Override
   public void open() {
-    logger.info("Geode open connection called!");
+    LOGGER.info("Geode open connection called!");
 
     // Close the previous open connections.
     close();
@@ -125,9 +125,9 @@ public class GeodeOqlInterpreter extends Interpreter {
       queryService = clientCache.getQueryService();
 
       exceptionOnConnect = null;
-      logger.info("Successfully created Geode connection");
+      LOGGER.info("Successfully created Geode connection");
     } catch (Exception e) {
-      logger.error("Cannot open connection", e);
+      LOGGER.error("Cannot open connection", e);
       exceptionOnConnect = e;
     }
   }
@@ -144,7 +144,7 @@ public class GeodeOqlInterpreter extends Interpreter {
       }
 
     } catch (Exception e) {
-      logger.error("Cannot close connection", e);
+      LOGGER.error("Cannot close connection", e);
     } finally {
       clientCache = null;
       queryService = null;
@@ -191,7 +191,7 @@ public class GeodeOqlInterpreter extends Interpreter {
       return new InterpreterResult(Code.SUCCESS, msg.toString());
 
     } catch (Exception ex) {
-      logger.error("Cannot run " + oql, ex);
+      LOGGER.error("Cannot run " + oql, ex);
       return new InterpreterResult(Code.ERROR, ex.getMessage());
     }
   }
@@ -242,7 +242,7 @@ public class GeodeOqlInterpreter extends Interpreter {
     if (!isHeaderSet) {
       msg.append("Result").append(NEWLINE);
     }
-    msg.append((Number) entry);
+    msg.append(entry);
   }
 
   private void handleUnsupportedTypeEntry(boolean isHeaderSet, Object entry, StringBuilder msg) {
@@ -255,7 +255,7 @@ public class GeodeOqlInterpreter extends Interpreter {
 
   @Override
   public InterpreterResult interpret(String cmd, InterpreterContext contextInterpreter) {
-    logger.info("Run OQL command '{}'", cmd);
+    LOGGER.info("Run OQL command '{}'", cmd);
     return executeOql(cmd);
   }
 
diff --git a/influxdb/src/main/java/org/apache/zeppelin/influxdb/InfluxDBInterpreter.java b/influxdb/src/main/java/org/apache/zeppelin/influxdb/InfluxDBInterpreter.java
index 1bbcfcd..bb8e62b 100644
--- a/influxdb/src/main/java/org/apache/zeppelin/influxdb/InfluxDBInterpreter.java
+++ b/influxdb/src/main/java/org/apache/zeppelin/influxdb/InfluxDBInterpreter.java
@@ -26,6 +26,8 @@ import com.influxdb.client.InfluxDBClientOptions;
 import com.influxdb.client.QueryApi;
 import org.apache.zeppelin.interpreter.AbstractInterpreter;
 import org.apache.zeppelin.interpreter.ZeppelinContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.apache.zeppelin.interpreter.InterpreterContext;
 import org.apache.zeppelin.interpreter.InterpreterException;
 import org.apache.zeppelin.interpreter.InterpreterResult;
@@ -49,6 +51,8 @@ import org.apache.zeppelin.interpreter.InterpreterResult;
  */
 public class InfluxDBInterpreter extends AbstractInterpreter {
 
+  private static final Logger LOGGER = LoggerFactory.getLogger(InfluxDBInterpreter.class);
+
   private static final String INFLUXDB_API_URL_PROPERTY = "influxdb.url";
   private static final String INFLUXDB_TOKEN_PROPERTY = "influxdb.token";
   private static final String INFLUXDB_ORG_PROPERTY = "influxdb.org";
@@ -76,7 +80,7 @@ public class InfluxDBInterpreter extends AbstractInterpreter {
   protected InterpreterResult internalInterpret(String query, InterpreterContext context)
       throws InterpreterException {
 
-    logger.debug("Run Flux command '{}'", query);
+    LOGGER.debug("Run Flux command '{}'", query);
     query = query.trim();
 
     QueryApi queryService = getInfluxDBClient(context);
@@ -119,7 +123,7 @@ public class InfluxDBInterpreter extends AbstractInterpreter {
 
         throwable -> {
 
-          logger.error(throwable.getMessage(), throwable);
+          LOGGER.error(throwable.getMessage(), throwable);
           resultRef.set(new InterpreterResult(InterpreterResult.Code.ERROR,
               throwable.getMessage()));
 
diff --git a/java/src/main/java/org/apache/zeppelin/java/JavaInterpreter.java b/java/src/main/java/org/apache/zeppelin/java/JavaInterpreter.java
index 1c5e9e7..7637c21 100644
--- a/java/src/main/java/org/apache/zeppelin/java/JavaInterpreter.java
+++ b/java/src/main/java/org/apache/zeppelin/java/JavaInterpreter.java
@@ -36,7 +36,7 @@ import org.slf4j.LoggerFactory;
  */
 public class JavaInterpreter extends Interpreter {
 
-  private static final Logger logger = LoggerFactory.getLogger(JavaInterpreter.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(JavaInterpreter.class);
 
   public JavaInterpreter(Properties property) {
     super(property);
@@ -65,7 +65,7 @@ public class JavaInterpreter extends Interpreter {
       String res = StaticRepl.execute(generatedClassName, code);
       return new InterpreterResult(InterpreterResult.Code.SUCCESS, res);
     } catch (Exception e) {
-      logger.error("Exception in Interpreter while interpret", e);
+      LOGGER.error("Exception in Interpreter while interpret", e);
       return new InterpreterResult(InterpreterResult.Code.ERROR, e.getMessage());
 
     }
diff --git a/ksql/src/main/java/org/apache/zeppelin/ksql/KSQLInterpreter.java b/ksql/src/main/java/org/apache/zeppelin/ksql/KSQLInterpreter.java
index f6079f1..92cb8da 100644
--- a/ksql/src/main/java/org/apache/zeppelin/ksql/KSQLInterpreter.java
+++ b/ksql/src/main/java/org/apache/zeppelin/ksql/KSQLInterpreter.java
@@ -142,10 +142,10 @@ public class KSQLInterpreter extends Interpreter {
 
   @Override
   public void cancel(InterpreterContext context) throws InterpreterException {
-    logger.info("Trying to cancel paragraphId {}", context.getParagraphId());
+    LOGGER.info("Trying to cancel paragraphId {}", context.getParagraphId());
     try {
       ksqlRestService.closeClient(context.getParagraphId());
-      logger.info("Removed");
+      LOGGER.info("Removed");
     } catch (IOException e) {
       throw new RuntimeException(e);
     }
diff --git a/livy/src/main/java/org/apache/zeppelin/livy/BaseLivyInterpreter.java b/livy/src/main/java/org/apache/zeppelin/livy/BaseLivyInterpreter.java
index fdbe545..f7b576d 100644
--- a/livy/src/main/java/org/apache/zeppelin/livy/BaseLivyInterpreter.java
+++ b/livy/src/main/java/org/apache/zeppelin/livy/BaseLivyInterpreter.java
@@ -55,7 +55,6 @@ import org.springframework.web.client.RestClientException;
 import org.springframework.web.client.RestTemplate;
 
 import java.io.FileInputStream;
-import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.security.KeyStore;
 import java.security.Principal;
@@ -235,7 +234,7 @@ public abstract class BaseLivyInterpreter extends Interpreter {
     try {
       return interpret(st, null, context.getParagraphId(), this.displayAppInfo, true, true);
     } catch (LivyException e) {
-      LOGGER.error("Fail to interpret:" + st, e);
+      LOGGER.error("Fail to interpret: {}", st, e);
       return new InterpreterResult(InterpreterResult.Code.ERROR,
           InterpreterUtils.getMostRelevantMessage(e));
     }
@@ -251,7 +250,7 @@ public abstract class BaseLivyInterpreter extends Interpreter {
       LOGGER.warn("Livy session {} is expired. Will return empty list of candidates.",
           getSessionInfo().id);
     } catch (LivyException le) {
-      logger.error("Failed to call code completions. Will return empty list of candidates", le);
+      LOGGER.error("Failed to call code completions. Will return empty list of candidates", le);
     }
     return candidates;
   }
@@ -265,7 +264,7 @@ public abstract class BaseLivyInterpreter extends Interpreter {
         candidates.add(new InterpreterCompletion(candidate, candidate, StringUtils.EMPTY));
       }
     } catch (APINotFoundException e) {
-      logger.debug("completion api seems not to be available. (available from livy 0.5)", e);
+      LOGGER.debug("completion api seems not to be available. (available from livy 0.5)", e);
     }
     return candidates;
   }
@@ -277,7 +276,7 @@ public abstract class BaseLivyInterpreter extends Interpreter {
       return;
     }
     paragraphsToCancel.add(context.getParagraphId());
-    LOGGER.info("Added paragraph " + context.getParagraphId() + " for cancellation.");
+    LOGGER.info("Added paragraph {} for cancellation.", context.getParagraphId());
   }
 
   @Override
@@ -335,7 +334,7 @@ public abstract class BaseLivyInterpreter extends Interpreter {
       }
       return sessionInfo;
     } catch (Exception e) {
-      LOGGER.error("Error when creating livy session for user " + user, e);
+      LOGGER.error("Error when creating livy session for user {}", user, e);
       throw new LivyException(e);
     }
   }
@@ -433,15 +432,15 @@ public abstract class BaseLivyInterpreter extends Interpreter {
   private void cancel(int id, String paragraphId) {
     if (livyVersion.isCancelSupported()) {
       try {
-        LOGGER.info("Cancelling statement " + id);
+        LOGGER.info("Cancelling statement {}", id);
         cancelStatement(id);
       } catch (LivyException e) {
-        LOGGER.error("Fail to cancel statement " + id + " for paragraph " + paragraphId, e);
+        LOGGER.error("Fail to cancel statement {} for paragraph {}", id, paragraphId, e);
       } finally {
         paragraphsToCancel.remove(paragraphId);
       }
     } else {
-      LOGGER.warn("cancel is not supported for this version of livy: " + livyVersion);
+      LOGGER.warn("cancel is not supported for this version of livy: {}", livyVersion);
       paragraphsToCancel.clear();
     }
   }
@@ -531,7 +530,7 @@ public abstract class BaseLivyInterpreter extends Interpreter {
             InterpreterResult.Type.TABLE, outputBuilder.toString());
       } else if (stmtInfo.output.data.imagePng != null) {
         return new InterpreterResult(InterpreterResult.Code.SUCCESS,
-            InterpreterResult.Type.IMG, (String) stmtInfo.output.data.imagePng);
+            InterpreterResult.Type.IMG, stmtInfo.output.data.imagePng);
       } else if (result != null) {
         result = result.trim();
         if (result.startsWith("<link")
@@ -607,22 +606,12 @@ public abstract class BaseLivyInterpreter extends Interpreter {
   }
 
   private KeyStore getStore(String file, String type, String password) {
-    FileInputStream inputStream = null;
-    try {
-      inputStream = new FileInputStream(file);
+    try (FileInputStream inputStream = new FileInputStream(file)) {
       KeyStore trustStore = KeyStore.getInstance(type);
-      trustStore.load(new FileInputStream(file), password.toCharArray());
+      trustStore.load(inputStream, password.toCharArray());
       return trustStore;
     } catch (Exception e) {
       throw new RuntimeException("Failed to open keystore " + file, e);
-    } finally {
-      if (inputStream != null) {
-        try {
-          inputStream.close();
-        } catch (IOException e) {
-          LOGGER.error("Failed to close keystore file", e);
-        }
-      }
     }
   }
 
diff --git a/neo4j/src/main/java/org/apache/zeppelin/graph/neo4j/Neo4jCypherInterpreter.java b/neo4j/src/main/java/org/apache/zeppelin/graph/neo4j/Neo4jCypherInterpreter.java
index 9127122..1944943 100644
--- a/neo4j/src/main/java/org/apache/zeppelin/graph/neo4j/Neo4jCypherInterpreter.java
+++ b/neo4j/src/main/java/org/apache/zeppelin/graph/neo4j/Neo4jCypherInterpreter.java
@@ -27,6 +27,8 @@ import org.neo4j.driver.v1.types.Node;
 import org.neo4j.driver.v1.types.Relationship;
 import org.neo4j.driver.v1.types.TypeSystem;
 import org.neo4j.driver.v1.util.Pair;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -53,6 +55,9 @@ import org.apache.zeppelin.scheduler.SchedulerFactory;
  * Neo4j interpreter for Zeppelin.
  */
 public class Neo4jCypherInterpreter extends Interpreter {
+
+  private static final Logger LOGGER = LoggerFactory.getLogger(Neo4jCypherInterpreter.class);
+
   private static final String TABLE = "%table";
   public static final String NEW_LINE = "\n";
   public static final String TAB = "\t";
@@ -62,9 +67,9 @@ public class Neo4jCypherInterpreter extends Interpreter {
   private Map<String, String> labels;
 
   private Set<String> types;
-  
+
   private final Neo4jConnectionManager neo4jConnectionManager;
-  
+
   private final ObjectMapper jsonMapper = new ObjectMapper();
 
   public Neo4jCypherInterpreter(Properties properties) {
@@ -117,7 +122,7 @@ public class Neo4jCypherInterpreter extends Interpreter {
 
   @Override
   public InterpreterResult interpret(String cypherQuery, InterpreterContext interpreterContext) {
-    logger.info("Opening session");
+    LOGGER.info("Opening session");
     if (StringUtils.isBlank(cypherQuery)) {
       return new InterpreterResult(Code.SUCCESS);
     }
@@ -155,7 +160,7 @@ public class Neo4jCypherInterpreter extends Interpreter {
         return renderTable(columns, lines);
       }
     } catch (Exception e) {
-      logger.error("Exception while interpreting cypher query", e);
+      LOGGER.error("Exception while interpreting cypher query", e);
       return new InterpreterResult(Code.ERROR, e.getMessage());
     }
   }
@@ -221,7 +226,7 @@ public class Neo4jCypherInterpreter extends Interpreter {
         try {
           value = jsonMapper.writer().writeValueAsString(value);
         } catch (Exception e) {
-          logger.debug("ignored exception: " + e.getMessage());
+          LOGGER.debug("ignored exception: " + e.getMessage());
         }
       }
     }
@@ -229,7 +234,7 @@ public class Neo4jCypherInterpreter extends Interpreter {
   }
 
   private InterpreterResult renderTable(List<String> cols, List<List<String>> lines) {
-    logger.info("Executing renderTable method");
+    LOGGER.info("Executing renderTable method");
     StringBuilder msg = null;
     if (cols.isEmpty()) {
       msg = new StringBuilder();
@@ -253,7 +258,7 @@ public class Neo4jCypherInterpreter extends Interpreter {
 
   private InterpreterResult renderGraph(Set<Node> nodes,
       Set<Relationship> relationships) {
-    logger.info("Executing renderGraph method");
+    LOGGER.info("Executing renderGraph method");
     List<org.apache.zeppelin.tabledata.Node> nodesList = new ArrayList<>();
     List<org.apache.zeppelin.tabledata.Relationship> relsList = new ArrayList<>();
     for (Relationship rel : relationships) {
diff --git a/rlang/src/main/java/org/apache/zeppelin/r/IRInterpreter.java b/rlang/src/main/java/org/apache/zeppelin/r/IRInterpreter.java
index f232d7e..2b6051b 100644
--- a/rlang/src/main/java/org/apache/zeppelin/r/IRInterpreter.java
+++ b/rlang/src/main/java/org/apache/zeppelin/r/IRInterpreter.java
@@ -155,22 +155,12 @@ public class IRInterpreter extends JupyterKernelInterpreter {
   public InterpreterResult shinyUI(String st,
                                    InterpreterContext context) throws InterpreterException {
     File uiFile = new File(shinyAppFolder, "ui.R");
-    FileWriter writer = null;
-    try {
-      writer = new FileWriter(uiFile);
+    try (FileWriter writer = new FileWriter(uiFile)){
       IOUtils.copy(new StringReader(st), writer);
       return new InterpreterResult(InterpreterResult.Code.SUCCESS, "Write ui.R to "
               + shinyAppFolder.getAbsolutePath() + " successfully.");
     } catch (IOException e) {
       throw new InterpreterException("Fail to write shiny file ui.R", e);
-    } finally {
-      if (writer != null) {
-        try {
-          writer.close();
-        } catch (IOException e) {
-          throw new InterpreterException(e);
-        }
-      }
     }
   }
 
diff --git a/rlang/src/main/java/org/apache/zeppelin/r/RInterpreter.java b/rlang/src/main/java/org/apache/zeppelin/r/RInterpreter.java
index aecbab3..4a7af23 100644
--- a/rlang/src/main/java/org/apache/zeppelin/r/RInterpreter.java
+++ b/rlang/src/main/java/org/apache/zeppelin/r/RInterpreter.java
@@ -151,7 +151,7 @@ public class RInterpreter extends AbstractInterpreter {
         return new InterpreterResult(InterpreterResult.Code.SUCCESS, "");
       }
     } catch (Exception e) {
-      logger.error("Exception while connecting to R", e);
+      LOGGER.error("Exception while connecting to R", e);
       return new InterpreterResult(InterpreterResult.Code.ERROR, e.getMessage());
     }
   }
diff --git a/sap/src/main/java/org/apache/zeppelin/sap/UniverseInterpreter.java b/sap/src/main/java/org/apache/zeppelin/sap/UniverseInterpreter.java
index fc6a423..3b378d7 100644
--- a/sap/src/main/java/org/apache/zeppelin/sap/UniverseInterpreter.java
+++ b/sap/src/main/java/org/apache/zeppelin/sap/UniverseInterpreter.java
@@ -19,6 +19,7 @@ package org.apache.zeppelin.sap;
 
 import org.apache.commons.lang3.StringUtils;
 import org.apache.zeppelin.interpreter.AbstractInterpreter;
+import org.apache.zeppelin.interpreter.Interpreter;
 import org.apache.zeppelin.interpreter.ZeppelinContext;
 import org.apache.zeppelin.interpreter.InterpreterContext;
 import org.apache.zeppelin.interpreter.InterpreterException;
@@ -27,7 +28,8 @@ import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
 import org.apache.zeppelin.sap.universe.*;
 import org.apache.zeppelin.scheduler.Scheduler;
 import org.apache.zeppelin.scheduler.SchedulerFactory;
-
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.*;
 import java.util.concurrent.ExecutorService;
@@ -39,6 +41,8 @@ import java.util.concurrent.TimeUnit;
  */
 public class UniverseInterpreter extends AbstractInterpreter {
 
+  private static final Logger LOGGER = LoggerFactory.getLogger(UniverseInterpreter.class);
+
   public UniverseInterpreter(Properties properties) {
     super(properties);
   }
@@ -134,7 +138,7 @@ public class UniverseInterpreter extends AbstractInterpreter {
       try {
         client.closeSession(context.getParagraphId());
       } catch (Exception e) {
-        logger.error("Error close SAP session", e );
+        LOGGER.error("Error close SAP session", e );
       }
     }
   }
@@ -144,7 +148,7 @@ public class UniverseInterpreter extends AbstractInterpreter {
     try {
       client.closeSession(context.getParagraphId());
     } catch (Exception e) {
-      logger.error("Error close SAP session", e );
+      LOGGER.error("Error close SAP session", e );
     }
   }
 
@@ -168,7 +172,7 @@ public class UniverseInterpreter extends AbstractInterpreter {
       universeCompleter = createOrUpdateUniverseCompleter(interpreterContext, buf, cursor);
       universeCompleter.complete(buf, cursor, candidates);
     } catch (UniverseException e) {
-      logger.error("Error update completer", e );
+      LOGGER.error("Error update completer", e );
     }
 
     return candidates;
@@ -244,12 +248,12 @@ public class UniverseInterpreter extends AbstractInterpreter {
 
       executorService.awaitTermination(10, TimeUnit.SECONDS);
     } catch (InterruptedException e) {
-      logger.warn("Completion timeout", e);
+      LOGGER.warn("Completion timeout", e);
     } finally {
       try {
         client.closeSession(interpreterContext.getParagraphId());
       } catch (Exception e) {
-        logger.error("Error close SAP session", e );
+        LOGGER.error("Error close SAP session", e );
       }
     }
     return completer;
diff --git a/scalding/src/main/java/org/apache/zeppelin/scalding/ScaldingInterpreter.java b/scalding/src/main/java/org/apache/zeppelin/scalding/ScaldingInterpreter.java
index f46a1d7..f104a58 100644
--- a/scalding/src/main/java/org/apache/zeppelin/scalding/ScaldingInterpreter.java
+++ b/scalding/src/main/java/org/apache/zeppelin/scalding/ScaldingInterpreter.java
@@ -49,7 +49,7 @@ import org.apache.zeppelin.scheduler.SchedulerFactory;
  *
  */
 public class ScaldingInterpreter extends Interpreter {
-  Logger logger = LoggerFactory.getLogger(ScaldingInterpreter.class);
+  public static final Logger LOGGER = LoggerFactory.getLogger(ScaldingInterpreter.class);
 
   static final String ARGS_STRING = "args.string";
   static final String ARGS_STRING_DEFAULT = "--local --repl";
@@ -76,15 +76,15 @@ public class ScaldingInterpreter extends Interpreter {
     try {
       maxOpenInstances = Integer.valueOf(maxOpenInstancesStr);
     } catch (Exception e) {
-      logger.error("Error reading max.open.instances", e);
+      LOGGER.error("Error reading max.open.instances", e);
     }
-    logger.info("max.open.instances = {}", maxOpenInstances);
+    LOGGER.info("max.open.instances = {}", maxOpenInstances);
     if (numOpenInstances > maxOpenInstances) {
-      logger.error("Reached maximum number of open instances");
+      LOGGER.error("Reached maximum number of open instances");
       return;
     }
-    logger.info("Opening instance {}", numOpenInstances);
-    logger.info("property: {}", getProperties());
+    LOGGER.info("Opening instance {}", numOpenInstances);
+    LOGGER.info("property: {}", getProperties());
     String argsString = getProperty(ARGS_STRING, ARGS_STRING_DEFAULT);
     String[] args;
     if (argsString == null) {
@@ -92,7 +92,7 @@ public class ScaldingInterpreter extends Interpreter {
     } else {
       args = argsString.split(" ");
     }
-    logger.info("{}", Arrays.toString(args));
+    LOGGER.info("{}", Arrays.toString(args));
 
     PrintWriter printWriter = new PrintWriter(out, true);
     interpreter = ZeppelinScaldingShell.getRepl(args, printWriter);
@@ -108,10 +108,10 @@ public class ScaldingInterpreter extends Interpreter {
   @Override
   public InterpreterResult interpret(String cmd, InterpreterContext contextInterpreter) {
     String user = contextInterpreter.getAuthenticationInfo().getUser();
-    logger.info("Running Scalding command: user: {} cmd: '{}'", user, cmd);
+    LOGGER.info("Running Scalding command: user: {} cmd: '{}'", user, cmd);
 
     if (interpreter == null) {
-      logger.error(
+      LOGGER.error(
           "interpreter == null, open may not have been called because max.open.instances reached");
       return new InterpreterResult(Code.ERROR,
         "interpreter == null\n" +
@@ -127,7 +127,7 @@ public class ScaldingInterpreter extends Interpreter {
       try {
         ugi = UserGroupInformation.createProxyUser(user, UserGroupInformation.getLoginUser());
       } catch (IOException e) {
-        logger.error("Error creating UserGroupInformation", e);
+        LOGGER.error("Error creating UserGroupInformation", e);
         return new InterpreterResult(Code.ERROR, e.getMessage());
       }
       try {
@@ -137,13 +137,14 @@ public class ScaldingInterpreter extends Interpreter {
         final InterpreterContext contextInterpreter1 = contextInterpreter;
         PrivilegedExceptionAction<InterpreterResult> action =
             new PrivilegedExceptionAction<InterpreterResult>() {
+              @Override
               public InterpreterResult run() throws Exception {
                 return interpret(cmd1.split("\n"), contextInterpreter1);
               }
             };
         interpreterResult = ugi.doAs(action);
       } catch (Exception e) {
-        logger.error("Error running command with ugi.doAs", e);
+        LOGGER.error("Error running command with ugi.doAs", e);
         return new InterpreterResult(Code.ERROR, e.getMessage());
       }
     } else {
@@ -215,7 +216,7 @@ public class ScaldingInterpreter extends Interpreter {
       try {
         res = interpreter.intp().interpret(incomplete + s);
       } catch (Exception e) {
-        logger.error("Interpreter exception: ", e);
+        LOGGER.error("Interpreter exception: ", e);
         return new InterpreterResult(Code.ERROR, e.getMessage());
       }
 
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/cluster/ClusterManager.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/cluster/ClusterManager.java
index 869340f..38bb832 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/cluster/ClusterManager.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/cluster/ClusterManager.java
@@ -129,7 +129,7 @@ import static org.apache.zeppelin.cluster.meta.ClusterMetaType.INTP_PROCESS_META
  * 3. Cluster monitoring
  */
 public abstract class ClusterManager {
-  private static Logger LOGGER = LoggerFactory.getLogger(ClusterManager.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(ClusterManager.class);
 
   public ZeppelinConfiguration zConf;
 
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/cluster/listener/ZeppelinClusterMembershipEventListener.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/cluster/listener/ZeppelinClusterMembershipEventListener.java
index 6283813..42c250b 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/cluster/listener/ZeppelinClusterMembershipEventListener.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/cluster/listener/ZeppelinClusterMembershipEventListener.java
@@ -27,23 +27,23 @@ import org.slf4j.LoggerFactory;
  * Monitor whether the metadata in the cluster server changes
  */
 public class ZeppelinClusterMembershipEventListener implements ClusterMembershipEventListener {
-  private static Logger logger
+  private static final Logger LOGGER
       = LoggerFactory.getLogger(ZeppelinClusterMembershipEventListener.class);
 
   @Override
   public void event(ClusterMembershipEvent event) {
     switch (event.type()) {
       case MEMBER_ADDED:
-        logger.info(event.subject().id() + " joined the cluster.");
+        LOGGER.info("{} joined the cluster.", event.subject().id());
         break;
       case MEMBER_REMOVED:
-        logger.info(event.subject().id() + " left the cluster.");
+        LOGGER.info("{} left the cluster.", event.subject().id());
         break;
       case METADATA_CHANGED:
-        logger.info(event.subject().id() + " meta data changed.");
+        LOGGER.info("{} meta data changed.", event.subject().id());
         break;
       case REACHABILITY_CHANGED:
-        logger.info(event.subject().id() + " reachability changed.");
+        LOGGER.info("{} reachability changed.", event.subject().id());
         break;
     }
   }
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/completer/CachedCompleter.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/completer/CachedCompleter.java
index ef2223e..72edca8 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/completer/CachedCompleter.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/completer/CachedCompleter.java
@@ -31,11 +31,8 @@ public class CachedCompleter {
   }
 
   public boolean isExpired() {
-    if (ttlInSeconds == -1 || (ttlInSeconds > 0 &&
-        (System.currentTimeMillis() - createdAt) / 1000 > ttlInSeconds)) {
-      return true;
-    }
-    return false;
+    return (ttlInSeconds == -1 || (ttlInSeconds > 0 &&
+        (System.currentTimeMillis() - createdAt) / 1000 > ttlInSeconds));
   }
 
   public Completer getCompleter() {
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/completer/StringsCompleter.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/completer/StringsCompleter.java
index c117441..a452662 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/completer/StringsCompleter.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/completer/StringsCompleter.java
@@ -28,7 +28,7 @@ import jline.internal.Preconditions;
  * Case-insensitive completer for a set of strings.
  */
 public class StringsCompleter implements Completer {
-  private final SortedSet<String> strings = new TreeSet<String>(new Comparator<String>() {
+  private final SortedSet<String> strings = new TreeSet<>(new Comparator<String>() {
     @Override
     public int compare(String o1, String o2) {
       return o1.compareToIgnoreCase(o2);
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
index 62265e0..071d37c 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
@@ -798,12 +798,7 @@ public class ZeppelinConfiguration extends XMLConfiguration {
   }
 
   public boolean isClusterMode() {
-    String clusterAddr = getString(ConfVars.ZEPPELIN_CLUSTER_ADDR);
-    if (StringUtils.isEmpty(clusterAddr)) {
-      return false;
-    }
-
-    return true;
+    return !StringUtils.isEmpty(getString(ConfVars.ZEPPELIN_CLUSTER_ADDR));
   }
 
   public int getClusterHeartbeatInterval() {
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java
index ceaa314..1cff609 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java
@@ -177,7 +177,7 @@ public abstract class Interpreter {
     return SchedulerFactory.singleton().createOrGetFIFOScheduler("interpreter_" + this.hashCode());
   }
 
-  public static Logger logger = LoggerFactory.getLogger(Interpreter.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(Interpreter.class);
   private InterpreterGroup interpreterGroup;
   private URL[] classloaderUrls;
   protected Properties properties;
@@ -202,14 +202,14 @@ public abstract class Interpreter {
 
   @ZeppelinApi
   public String getProperty(String key) {
-    logger.debug("key: {}, value: {}", key, getProperties().getProperty(key));
+    LOGGER.debug("key: {}, value: {}", key, getProperties().getProperty(key));
 
     return getProperties().getProperty(key);
   }
 
   @ZeppelinApi
   public String getProperty(String key, String defaultValue) {
-    logger.debug("key: {}, value: {}", key, getProperties().getProperty(key, defaultValue));
+    LOGGER.debug("key: {}, value: {}", key, getProperties().getProperty(key, defaultValue));
 
     return getProperties().getProperty(key, defaultValue);
   }
@@ -375,20 +375,20 @@ public abstract class Interpreter {
     if (interpreterContext != null) {
       String markerTemplate = "#\\{%s\\}";
       List<String> skipFields = Arrays.asList("paragraphTitle", "paragraphId", "paragraphText");
-      List typesToProcess = Arrays.asList(String.class, Double.class, Float.class, Short.class,
+      List<Class<?>> typesToProcess = Arrays.asList(String.class, Double.class, Float.class, Short.class,
           Byte.class, Character.class, Boolean.class, Integer.class, Long.class);
       for (String key : properties.stringPropertyNames()) {
         String p = properties.getProperty(key);
         if (StringUtils.isNotEmpty(p)) {
           for (Field field : InterpreterContext.class.getDeclaredFields()) {
-            Class clazz = field.getType();
+            Class<?> clazz = field.getType();
             if (!skipFields.contains(field.getName()) && (typesToProcess.contains(clazz)
                 || clazz.isPrimitive())) {
               Object value = null;
               try {
                 value = FieldUtils.readField(field, interpreterContext, true);
               } catch (Exception e) {
-                logger.error("Cannot read value of field {0}", field.getName());
+                LOGGER.error("Cannot read value of field {}", field.getName());
               }
               p = p.replaceAll(String.format(markerTemplate, field.getName()),
                   value != null ? value.toString() : StringUtils.EMPTY);
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOutput.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOutput.java
index 928dd41..e7f28ed 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOutput.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOutput.java
@@ -36,8 +36,8 @@ import java.util.List;
  */
 public class InterpreterOutput extends OutputStream {
   Logger logger = LoggerFactory.getLogger(InterpreterOutput.class);
-  private final int NEW_LINE_CHAR = '\n';
-  private final int LINE_FEED_CHAR = '\r';
+  private static final int NEW_LINE_CHAR = '\n';
+  private static final int LINE_FEED_CHAR = '\r';
 
   private List<InterpreterResultMessageOutput> resultMessageOutputs = new LinkedList<>();
   private InterpreterResultMessageOutput currentOut;
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
index 8953eda..90e9aba 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
@@ -111,7 +111,7 @@ import static org.apache.zeppelin.cluster.meta.ClusterMetaType.INTP_PROCESS_META
 public class RemoteInterpreterServer extends Thread
     implements RemoteInterpreterService.Iface {
 
-  private static Logger LOGGER = LoggerFactory.getLogger(RemoteInterpreterServer.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(RemoteInterpreterServer.class);
 
   private String interpreterGroupId;
   private InterpreterGroup interpreterGroup;
@@ -135,7 +135,7 @@ public class RemoteInterpreterServer extends Thread
 
   private Map<String, Object> remoteWorksResponsePool;
 
-  private final long DEFAULT_SHUTDOWN_TIMEOUT = 2000;
+  private static final long DEFAULT_SHUTDOWN_TIMEOUT = 2000;
 
   // Hold information for manual progress update
   private ConcurrentMap<String, Integer> progressMap = new ConcurrentHashMap<>();
@@ -191,7 +191,7 @@ public class RemoteInterpreterServer extends Thread
       serverTransport = RemoteInterpreterUtils.createTServerSocket(portRange);
       this.port = serverTransport.getServerSocket().getLocalPort();
       this.host = RemoteInterpreterUtils.findAvailableHostAddress();
-      LOGGER.info("Launching ThriftServer at " + this.host + ":" + this.port);
+      LOGGER.info("Launching ThriftServer at {}:{}", this.host, this.port);
     }
     server = new TThreadPoolServer(
         new TThreadPoolServer.Args(serverTransport).processor(processor));
@@ -509,7 +509,7 @@ public class RemoteInterpreterServer extends Thread
 
   @Override
   public void open(String sessionId, String className) throws TException {
-    LOGGER.info(String.format("Open Interpreter %s for session %s ", className, sessionId));
+    LOGGER.info("Open Interpreter {} for session {}", className, sessionId);
     Interpreter intp = getInterpreter(sessionId, className);
     try {
       intp.open();
@@ -600,8 +600,8 @@ public class RemoteInterpreterServer extends Thread
     boolean isRecover = Boolean.parseBoolean(
             context.getLocalProperties().getOrDefault("isRecover", "false"));
     if (isRecover) {
-      LOGGER.info("Recovering paragraph: " + context.getParagraphId() + " of note: "
-              + context.getNoteId());
+      LOGGER.info("Recovering paragraph: {} of note: {}",
+              context.getParagraphId(), context.getNoteId());
       interpretJob = runningJobs.get(context.getParagraphId());
       if (interpretJob == null) {
         InterpreterResult result = new InterpreterResult(Code.ERROR, "Job is finished, unable to recover it");
@@ -772,7 +772,7 @@ public class RemoteInterpreterServer extends Thread
           //     global_post_hook
           processInterpreterHooks(context.getNoteId());
           processInterpreterHooks(null);
-          LOGGER.debug("Script after hooks: " + script);
+          LOGGER.debug("Script after hooks: {}", script);
           result = interpreter.interpret(script, context);
         }
 
@@ -792,7 +792,7 @@ public class RemoteInterpreterServer extends Thread
           if (msg.getType() == InterpreterResult.Type.IMG) {
             LOGGER.debug("InterpreterResultMessage: IMAGE_DATA");
           } else {
-            LOGGER.debug("InterpreterResultMessage: " + msg.toString());
+            LOGGER.debug("InterpreterResultMessage: {}", msg);
           }
           stringResult.add(msg.getData());
         }
@@ -847,7 +847,7 @@ public class RemoteInterpreterServer extends Thread
         try {
           intp.cancel(convert(interpreterContext, null));
         } catch (InterpreterException e) {
-          LOGGER.error("Fail to cancel paragraph: " + interpreterContext.getParagraphId());
+          LOGGER.error("Fail to cancel paragraph: {}", interpreterContext.getParagraphId());
         }
       });
       thread.start();
@@ -864,8 +864,7 @@ public class RemoteInterpreterServer extends Thread
     } else {
       Interpreter intp = getInterpreter(sessionId, className);
       if (intp == null) {
-        throw new TException("No interpreter {} existed for session {}".format(
-            className, sessionId));
+        throw new TException("No interpreter " + className + " existed for session " + sessionId);
       }
       try {
         return intp.getProgress(convert(interpreterContext, null));
@@ -1253,7 +1252,7 @@ public class RemoteInterpreterServer extends Thread
       String applicationInstanceId, String packageInfo, String noteId, String paragraphId)
       throws TException {
     if (runningApplications.containsKey(applicationInstanceId)) {
-      LOGGER.warn("Application instance {} is already running");
+      LOGGER.warn("Application instance {} is already running", applicationInstanceId);
       return new RemoteApplicationResult(true, "");
     }
     HeliumPackage pkgInfo = HeliumPackage.fromJson(packageInfo);
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/YarnUtils.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/YarnUtils.java
index daa9a94..40d3dec 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/YarnUtils.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/YarnUtils.java
@@ -20,13 +20,12 @@ package org.apache.zeppelin.interpreter.remote;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.yarn.api.records.FinalApplicationStatus;
 import org.apache.hadoop.yarn.client.api.AMRMClient;
+import org.apache.hadoop.yarn.client.api.AMRMClient.ContainerRequest;
 import org.apache.hadoop.yarn.conf.YarnConfiguration;
 import org.apache.hadoop.yarn.exceptions.YarnException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
-
 
 /**
  * Singleton class which is used for register/unregister yarn app AM.
@@ -38,7 +37,7 @@ public class YarnUtils {
 
   private static Logger LOGGER = LoggerFactory.getLogger(YarnUtils.class);
 
-  private static AMRMClient amClient = AMRMClient.createAMRMClient();
+  private static AMRMClient<ContainerRequest> amClient = AMRMClient.createAMRMClient();
   private static Configuration conf = new YarnConfiguration();
 
   static {
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/Job.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/Job.java
index b051585..7d06d49 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/Job.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/scheduler/Job.java
@@ -38,7 +38,7 @@ import java.util.Map;
  * Changing/adding/deleting non transitive field name need consideration of that.
  */
 public abstract class Job<T> {
-  private static Logger LOGGER = LoggerFactory.getLogger(Job.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(Job.class);
   private static SimpleDateFormat JOB_DATE_FORMAT = new SimpleDateFormat("yyyyMMdd-HHmmss");
 
   /**
diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/service/InterpreterService.java b/zeppelin-server/src/main/java/org/apache/zeppelin/service/InterpreterService.java
index 331f838..ff897f5 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/service/InterpreterService.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/service/InterpreterService.java
@@ -45,8 +45,8 @@ import org.sonatype.aether.RepositoryException;
 public class InterpreterService {
 
   private static final String ZEPPELIN_ARTIFACT_PREFIX = "zeppelin-";
-  private static final Logger logger = LoggerFactory.getLogger(InterpreterService.class);
-  private static final ExecutorService executorService =
+  private static final Logger LOGGER = LoggerFactory.getLogger(InterpreterService.class);
+  private static final ExecutorService EXECUTOR_SERVICE =
       Executors.newSingleThreadExecutor(
           new ThreadFactoryBuilder()
               .setNameFormat(InterpreterService.class.getSimpleName() + "-")
@@ -63,7 +63,7 @@ public class InterpreterService {
   }
 
   public void installInterpreter(
-      final InterpreterInstallationRequest request, final ServiceCallback serviceCallback)
+      final InterpreterInstallationRequest request, final ServiceCallback<String> serviceCallback)
       throws Exception {
     Preconditions.checkNotNull(request);
     String interpreterName = request.getName();
@@ -112,7 +112,7 @@ public class InterpreterService {
     }
 
     // It might take time to finish it
-    executorService.execute(
+    EXECUTOR_SERVICE.execute(
         new Runnable() {
           @Override
           public void run() {
@@ -127,28 +127,28 @@ public class InterpreterService {
       Path interpreterDir,
       ServiceCallback<String> serviceCallback) {
     try {
-      logger.info("Start to download a dependency: {}", request.getName());
+      LOGGER.info("Start to download a dependency: {}", request.getName());
       if (null != serviceCallback) {
         serviceCallback.onStart("Starting to download " + request.getName() + " interpreter", null);
       }
 
       dependencyResolver.load(request.getArtifact(), interpreterDir.toFile());
       interpreterSettingManager.refreshInterpreterTemplates();
-      logger.info(
+      LOGGER.info(
           "Finish downloading a dependency {} into {}",
           request.getName(),
-          interpreterDir.toString());
+          interpreterDir);
       if (null != serviceCallback) {
         serviceCallback.onSuccess(request.getName() + " downloaded", null);
       }
     } catch (RepositoryException | IOException e) {
-      logger.error("Error while downloading dependencies", e);
+      LOGGER.error("Error while downloading dependencies", e);
       try {
         FileUtils.deleteDirectory(interpreterDir.toFile());
       } catch (IOException e1) {
-        logger.error(
+        LOGGER.error(
             "Error while removing directory. You should handle it manually: {}",
-            interpreterDir.toString(),
+            interpreterDir,
             e1);
       }
       if (null != serviceCallback) {
@@ -157,7 +157,7 @@ public class InterpreterService {
               new Exception("Error while downloading " + request.getName() + " as " +
                   e.getMessage()), null);
         } catch (IOException e1) {
-          logger.error("ServiceCallback failure", e1);
+          LOGGER.error("ServiceCallback failure", e1);
         }
       }
     }
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ConfInterpreter.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ConfInterpreter.java
index 0a95df1..f343790 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ConfInterpreter.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ConfInterpreter.java
@@ -31,7 +31,7 @@ import java.util.Properties;
  */
 public class ConfInterpreter extends Interpreter {
 
-  private static Logger LOGGER = LoggerFactory.getLogger(ConfInterpreter.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(ConfInterpreter.class);
 
   protected String sessionId;
   protected String interpreterGroupId;
@@ -68,8 +68,7 @@ public class ConfInterpreter extends Interpreter {
       Properties newProperties = new Properties();
       newProperties.load(new StringReader(st));
       finalProperties.putAll(newProperties);
-      LOGGER.debug("Properties for InterpreterGroup: " + interpreterGroupId + " is "
-          + finalProperties);
+      LOGGER.debug("Properties for InterpreterGroup: {} is {}", interpreterGroupId, finalProperties);
       interpreterSetting.setInterpreterGroupProperties(interpreterGroupId, finalProperties);
       return new InterpreterResult(InterpreterResult.Code.SUCCESS);
     } catch (IOException e) {
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterInfoSaving.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterInfoSaving.java
index 8f89448..cc1c22d 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterInfoSaving.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterInfoSaving.java
@@ -53,7 +53,7 @@ public class InterpreterInfoSaving implements JsonSerializable {
   public List<RemoteRepository> interpreterRepositories = new ArrayList<>();
 
   public static InterpreterInfoSaving loadFromFile(Path file) throws IOException {
-    LOGGER.info("Load interpreter setting from file: " + file);
+    LOGGER.info("Load interpreter setting from file: {}", file);
     InterpreterInfoSaving infoSaving = null;
     try (BufferedReader json = Files.newBufferedReader(file, StandardCharsets.UTF_8)) {
       JsonParser jsonParser = new JsonParser();
@@ -80,12 +80,13 @@ public class InterpreterInfoSaving implements JsonSerializable {
       } catch (UnsupportedOperationException e) {
         // File system does not support Posix file permissions (likely windows) - continue anyway.
         LOGGER.warn("unable to setPosixFilePermissions on '{}'.", file);
-      };
+      }
     }
-    LOGGER.info("Save Interpreter Settings to " + file);
-    IOUtils.write(this.toJson(), new FileOutputStream(file.toFile()));
+    LOGGER.info("Save Interpreter Settings to {}", file);
+    IOUtils.write(this.toJson(), new FileOutputStream(file.toFile()), StandardCharsets.UTF_8);
   }
 
+  @Override
   public String toJson() {
     return gson.toJson(this);
   }
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
index 206236b..12d1c2b 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
@@ -64,6 +64,7 @@ import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Properties;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
@@ -291,7 +292,7 @@ public class InterpreterSetting {
     this.name = o.name;
     this.group = o.group;
     this.properties = convertInterpreterProperties(
-        (Map<String, DefaultInterpreterProperty>) o.getProperties());
+        o.getProperties());
     this.interpreterInfos = new ArrayList<>(o.getInterpreterInfos());
     this.option = InterpreterOption.fromInterpreterOption(o.getOption());
     this.dependencies = new ArrayList<>(o.getDependencies());
@@ -370,9 +371,7 @@ public class InterpreterSetting {
   }
 
   public InterpreterInfo getInterpreterInfo(String name) {
-    Iterator it = this.interpreterInfos.iterator();
-    while (it.hasNext()) {
-      InterpreterInfo info = (InterpreterInfo) it.next();
+    for (InterpreterInfo info :interpreterInfos) {
       if (StringUtils.equals(info.getName(), name)) {
         return info;
       }
@@ -479,7 +478,7 @@ public class InterpreterSetting {
       }
       return interpreterGroups.get(groupId);
     } finally {
-      interpreterGroupWriteLock.unlock();;
+      interpreterGroupWriteLock.unlock();
     }
   }
 
@@ -510,10 +509,10 @@ public class InterpreterSetting {
     return interpreterGroups.get(groupId);
   }
 
-  public ArrayList<ManagedInterpreterGroup> getAllInterpreterGroups() {
+  public List<ManagedInterpreterGroup> getAllInterpreterGroups() {
     try {
       interpreterGroupReadLock.lock();
-      return new ArrayList(interpreterGroups.values());
+      return new ArrayList<>(interpreterGroups.values());
     } finally {
       interpreterGroupReadLock.unlock();
     }
@@ -548,7 +547,7 @@ public class InterpreterSetting {
   }
 
   public void close() {
-    LOGGER.info("Close InterpreterSetting: " + name);
+    LOGGER.info("Close InterpreterSetting: {}", name);
     List<Thread> closeThreads = interpreterGroups.values().stream()
             .map(g -> new Thread(g::close, name + "-close"))
             .peek(t -> t.setUncaughtExceptionHandler((th, e) ->
@@ -571,8 +570,8 @@ public class InterpreterSetting {
     if (object instanceof StringMap) {
       StringMap<String> map = (StringMap) properties;
       Properties newProperties = new Properties();
-      for (String key : map.keySet()) {
-        newProperties.put(key, map.get(key));
+      for (Entry<String, String> mapEntries : map.entrySet()) {
+        newProperties.put(mapEntries.getKey(), mapEntries.getValue());
       }
       this.properties = newProperties;
     } else {
@@ -762,7 +761,7 @@ public class InterpreterSetting {
   }
 
   public void setStatus(Status status) {
-    LOGGER.info(String.format("Set interpreter %s status to %s", name, status.name()));
+    LOGGER.info("Set interpreter {} status to{}", name, status.name());
     this.status = status;
   }
 
@@ -791,11 +790,11 @@ public class InterpreterSetting {
       return "K8sStandardInterpreterLauncher";
     } else if (isRunningOnCluster()) {
       return InterpreterSetting.CLUSTER_INTERPRETER_LAUNCHER_NAME;
-    } if (isRunningOnDocker()) {
+    } else if (isRunningOnDocker()) {
       return "DockerInterpreterLauncher";
     } else {
       String launcher = properties.getProperty("zeppelin.interpreter.launcher");
-      LOGGER.debug("zeppelin.interpreter.launcher: " + launcher);
+      LOGGER.debug("zeppelin.interpreter.launcher: {}", launcher);
       if (group.equals("spark")) {
         return "SparkInterpreterLauncher";
       } else if (group.equals("flink")) {
@@ -976,10 +975,11 @@ public class InterpreterSetting {
     setStatus(Status.DOWNLOADING_DEPENDENCIES);
     setErrorReason(null);
     Thread t = new Thread() {
+      @Override
       public void run() {
         try {
           // dependencies to prevent library conflict
-          File localRepoDir = new File(conf.getInterpreterLocalRepoPath() + "/" + id);
+          File localRepoDir = new File(conf.getInterpreterLocalRepoPath() + '/' + id);
           if (localRepoDir.exists()) {
             try {
               FileUtils.forceDelete(localRepoDir);
@@ -991,7 +991,7 @@ public class InterpreterSetting {
           // load dependencies
           List<Dependency> deps = getDependencies();
           if (deps != null && !deps.isEmpty()) {
-            LOGGER.info("Start to download dependencies for interpreter: " + name);
+            LOGGER.info("Start to download dependencies for interpreter: {}", name);
             for (Dependency d : deps) {
               File destDir = new File(
                   conf.getRelativeDir(ZeppelinConfiguration.ConfVars.ZEPPELIN_DEP_LOCALREPO));
@@ -1004,7 +1004,7 @@ public class InterpreterSetting {
                     .load(d.getGroupArtifactVersion(), new File(destDir, id));
               }
             }
-            LOGGER.info("Finish downloading dependencies for interpreter: " + name);
+            LOGGER.info("Finish downloading dependencies for interpreter: {}", name);
           }
 
           setStatus(Status.READY);
@@ -1049,7 +1049,7 @@ public class InterpreterSetting {
 
   // For backward compatibility of interpreter.json format after ZEPPELIN-2403
   static Map<String, InterpreterProperty> convertInterpreterProperties(Object properties) {
-    if (properties != null && properties instanceof StringMap) {
+    if (properties instanceof StringMap) {
       Map<String, InterpreterProperty> newProperties = new LinkedHashMap<>();
       StringMap p = (StringMap) properties;
       for (Object o : p.entrySet()) {
@@ -1075,8 +1075,9 @@ public class InterpreterSetting {
       Map<String, Object> dProperties =
           (Map<String, Object>) properties;
       Map<String, InterpreterProperty> newProperties = new LinkedHashMap<>();
-      for (String key : dProperties.keySet()) {
-        Object value = dProperties.get(key);
+      for (Entry<String, Object> dPropertiesEntry : dProperties.entrySet()) {
+        String key = dPropertiesEntry.getKey();
+        Object value = dPropertiesEntry.getValue();
         if (value instanceof InterpreterProperty) {
           return (Map<String, InterpreterProperty>) properties;
         } else if (value instanceof StringMap) {
@@ -1111,7 +1112,7 @@ public class InterpreterSetting {
       }
       return newProperties;
     }
-    throw new RuntimeException("Can not convert this type: " + properties.getClass());
+    throw new RuntimeException("Can not convert this type: " + (properties != null ? properties.getClass() : "null"));
   }
 
   public void waitForReady(long timeout) throws InterpreterException {
@@ -1150,8 +1151,7 @@ public class InterpreterSetting {
     Gson gson = new GsonBuilder().setPrettyPrinting().create();
 
     StringWriter stringWriter = new StringWriter();
-    JsonWriter jsonWriter = new JsonWriter(stringWriter);
-    try {
+    try(JsonWriter jsonWriter = new JsonWriter(stringWriter)){
       // id
       jsonWriter.beginObject();
       jsonWriter.name("id");
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java
index a5bfc7a..ed02c0a 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java
@@ -187,13 +187,13 @@ public class InterpreterSettingManager implements NoteEventListener, ClusterEven
             conf.getRecoveryStorageClass(),
             new Class[] {ZeppelinConfiguration.class, InterpreterSettingManager.class},
             new Object[] {conf, this});
-    LOGGER.info("Using RecoveryStorage: " + this.recoveryStorage.getClass().getName());
+    LOGGER.info("Using RecoveryStorage: {}", this.recoveryStorage.getClass().getName());
     this.lifecycleManager =
         ReflectionUtils.createClazzInstance(
             conf.getLifecycleManagerClass(),
             new Class[] {ZeppelinConfiguration.class},
             new Object[] {conf});
-    LOGGER.info("Using LifecycleManager: " + this.lifecycleManager.getClass().getName());
+    LOGGER.info("Using LifecycleManager: {}", this.lifecycleManager.getClass().getName());
 
     this.configStorage = configStorage;
     init();
@@ -278,9 +278,8 @@ public class InterpreterSettingManager implements NoteEventListener, ClusterEven
         savedInterpreterSetting.setInterpreterRunner(
             interpreterSettingTemplate.getInterpreterRunner());
       } else {
-        LOGGER.warn("No InterpreterSetting Template found for InterpreterSetting: "
-            + savedInterpreterSetting.getGroup() + ", but it is found in interpreter.json, "
-            + "it would be skipped.");
+        LOGGER.warn("No InterpreterSetting Template found for InterpreterSetting: {},"
+          + " but it is found in interpreter.json, it would be skipped.", savedInterpreterSetting.getGroup());
         continue;
       }
 
@@ -496,7 +495,7 @@ public class InterpreterSettingManager implements NoteEventListener, ClusterEven
         .setName(group)
         .setInterpreterInfos(interpreterInfos)
         .setProperties(properties)
-        .setDependencies(new ArrayList<Dependency>())
+        .setDependencies(new ArrayList<>())
         .setOption(option)
         .setRunner(runner)
         .setInterpreterDir(interpreterDir)
@@ -521,7 +520,7 @@ public class InterpreterSettingManager implements NoteEventListener, ClusterEven
       }
       return interpreterSetting;
     } catch (Exception e) {
-      LOGGER.warn("Fail to get note: " + noteId, e);
+      LOGGER.warn("Fail to get note: {}", noteId, e);
       return get().get(0);
     }
   }
@@ -743,7 +742,7 @@ public class InterpreterSettingManager implements NoteEventListener, ClusterEven
     try {
       List<Dependency> deps = setting.getDependencies();
       if (deps != null) {
-        LOGGER.info("Start to copy dependencies for interpreter: " + setting.getName());
+        LOGGER.info("Start to copy dependencies for interpreter: {}", setting.getName());
         for (Dependency d : deps) {
           File destDir = new File(
               conf.getRelativeDir(ConfVars.ZEPPELIN_DEP_LOCALREPO));
@@ -754,7 +753,7 @@ public class InterpreterSettingManager implements NoteEventListener, ClusterEven
                 new File(destDir, setting.getId()));
           }
         }
-        LOGGER.info("Finish copy dependencies for interpreter: " + setting.getName());
+        LOGGER.info("Finish copy dependencies for interpreter: {}", setting.getName());
       }
     } catch (Exception e) {
       LOGGER.error(String.format("Error while copying deps for interpreter group : %s," +
@@ -847,7 +846,7 @@ public class InterpreterSettingManager implements NoteEventListener, ClusterEven
       File[] files = path.listFiles();
       if (files != null) {
         for (File f : files) {
-          urls = (URL[]) ArrayUtils.addAll(urls, recursiveBuildLibList(f));
+          urls = ArrayUtils.addAll(urls, recursiveBuildLibList(f));
         }
       }
       return urls;
@@ -973,7 +972,7 @@ public class InterpreterSettingManager implements NoteEventListener, ClusterEven
     // 2. remove this interpreter setting
     // 3. remove this interpreter setting from note binding
     // 4. clean local repo directory
-    LOGGER.info("Remove interpreter setting: " + id);
+    LOGGER.info("Remove interpreter setting: {}", id);
     if (interpreterSettings.containsKey(id)) {
       InterpreterSetting intp = interpreterSettings.get(id);
       intp.close();
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/remote/mock/MockInterpreterAngular.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/remote/mock/MockInterpreterAngular.java
index ec89241..fb956f5 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/remote/mock/MockInterpreterAngular.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/remote/mock/MockInterpreterAngular.java
@@ -24,6 +24,8 @@ import org.apache.zeppelin.interpreter.InterpreterContext;
 import org.apache.zeppelin.interpreter.InterpreterResult;
 import org.apache.zeppelin.interpreter.InterpreterResult.Code;
 import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.List;
 import java.util.Properties;
@@ -31,6 +33,8 @@ import java.util.concurrent.atomic.AtomicInteger;
 
 public class MockInterpreterAngular extends Interpreter {
 
+  private static final Logger LOGGER = LoggerFactory.getLogger(MockInterpreterAngular.class);
+
   AtomicInteger numWatch = new AtomicInteger(0);
 
   public MockInterpreterAngular(Properties property) {
@@ -82,7 +86,7 @@ public class MockInterpreterAngular extends Interpreter {
     try {
       Thread.sleep(500); // wait for watcher executed
     } catch (InterruptedException e) {
-      logger.error("Exception in MockInterpreterAngular while interpret Thread.sleep", e);
+      LOGGER.error("Exception in MockInterpreterAngular while interpret Thread.sleep", e);
     }
 
     String msg = registry.getAll(context.getNoteId(), null).size() + " " + Integer.toString(numWatch