Reamer commented on code in PR #4908:
URL: https://github.com/apache/zeppelin/pull/4908#discussion_r2015904362


##########
hbase/src/main/java/org/apache/zeppelin/hbase/HbaseInterpreter.java:
##########
@@ -143,30 +156,26 @@ public Scheduler getScheduler() {
         HbaseInterpreter.class.getName() + this.hashCode());
   }
 
-  @Override
-  public List<InterpreterCompletion> completion(String buf, int cursor,
-      InterpreterContext interpreterContext) {
-    return null;
+  private void stopProcess(String paragraphId) {
+    if (runningProcesses.containsKey(paragraphId)) {

Review Comment:
   The deletion of values can also be written more elegantly here. The code 
currently accesses the map three times.
   1) containsKey
   2) get
   3) remove



##########
hbase/src/main/java/org/apache/zeppelin/hbase/HbaseInterpreter.java:
##########
@@ -143,30 +156,26 @@ public Scheduler getScheduler() {
         HbaseInterpreter.class.getName() + this.hashCode());
   }
 
-  @Override
-  public List<InterpreterCompletion> completion(String buf, int cursor,
-      InterpreterContext interpreterContext) {
-    return null;
+  private void stopProcess(String paragraphId) {
+    if (runningProcesses.containsKey(paragraphId)) {
+      final Executor executor = runningProcesses.get(paragraphId);
+      final ExecuteWatchdog watchdog = executor.getWatchdog();
+      watchdog.destroyProcess();
+      runningProcesses.remove(paragraphId);
+    }
   }
 
-  private static String getSystemDefault(
-      String envName,
-      String propertyName,
-      String defaultValue) {
-
-    if (envName != null && !envName.isEmpty()) {
-      String envValue = System.getenv().get(envName);
-      if (envValue != null) {
-        return envValue;
-      }
-    }
+  private File createTempFile(String paragraphId) throws IOException {
+    File temp = Files.createTempFile(
+            Paths.get(System.getProperty("java.io.tmpdir"), 
"zeppelin-hbase-scripts"), paragraphId, ".txt").toFile();
+    tempFiles.put(paragraphId, temp);
+    return temp;
+  }
 
-    if (propertyName != null && !propertyName.isEmpty()) {
-      String propValue = System.getProperty(propertyName);
-      if (propValue != null) {
-        return propValue;
-      }
+  private void deleteTempFile(String paragraphId) {
+    if (tempFiles.containsKey(paragraphId)) {

Review Comment:
   More elegant style for the same behavior.
   ```
       File tmpFile = tempFiles.remove(paragraphId);
       if (tmpFile != null) {
         FileUtils.deleteQuietly(tmpFile);
       }
   ```
   
   With this code style, the map is only searched once and the program 
execution is faster.
   I know that this map is usually very small and does not play a role on 
today's hardware. But maybe you have other projects where the maps are very 
large and so you can optimize your application accordingly.



-- 
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: reviews-unsubscr...@zeppelin.apache.org

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

Reply via email to