-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[feat] EngineConn no longer imports the dependencies of the underlying engine #4278
[feat] EngineConn no longer imports the dependencies of the underlying engine #4278
Conversation
@@ -33,8 +33,6 @@ import java.util.concurrent.TimeUnit | |||
import scala.concurrent.{Await, ExecutionContext, Future} | |||
import scala.concurrent.duration.Duration | |||
|
|||
import org.json4s._ | |||
|
|||
/** | |||
*/ | |||
abstract class ProcessInterpreter(process: Process) extends Interpreter with Logging { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can delete ProcessInterpreter,No longer use
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems PythonInterpreter
still extends from ProcessInterpreter
private class PythonInterpreter(process: Process, gatewayServer: GatewayServer)
extends ProcessInterpreter(process)
with Logging {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GuoPhilipse Yes, but it is no longer used, we can remove inheritance and remove unused methods
…nused method from PythonInterpreter.scala
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
What is the purpose of the change
EngineConn no longer imports the dependencies of the underlying engine. and remove json4s from ec to reduce engine dependency
Related issues/PRs
Related issues: #4186
Related pr:#4278
Brief change log
Checklist