weiqingy commented on a change in pull request #1106: Making Samza-Sql-Shell 
commands pluggable
URL: https://github.com/apache/samza/pull/1106#discussion_r305554322
 
 

 ##########
 File path: 
samza-sql-shell/src/main/java/org/apache/samza/sql/client/cli/CliEnvironment.java
 ##########
 @@ -173,11 +187,21 @@ public SqlExecutor getExecutor() {
     return executor;
   }
 
+  public List<CommandHandler> getCommandHandlers() { return commandHandlers; }
+
   private void createShellExecutor(String executorClassName) throws 
ExecutorException {
+    executor = (SqlExecutor) createInstance(executorClassName);
+  }
+
+  private void createCommandHandler(String handlerClassName) throws 
ExecutorException {
+    commandHandlers.add((CommandHandler) createInstance(handlerClassName));
+  }
+
+  private Object createInstance(String className) throws ExecutorException {
 
 Review comment:
   It's fine to throw a `ExecutorException` for a method of 
`createShellExecutor`, but would be misleading for a method named 
`createInstance` as it creates `commandhandler`s as well.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to