[MINOR] Move remoteinterpreter into zengine #2320
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Removed unnecessary classes imported
felixcheung
approved these changes
May 4, 2017
| Properties p = new Properties(); | ||
| p.put("p1", "v1"); | ||
| MockInterpreterA intp = new MockInterpreterA(p); | ||
| Interpreter intp = new Interpreter(p) { |
Member
There was a problem hiding this comment.
instead of duplicating this would it be less error prone to have a common interpreter test java file to define this?
Member
Author
There was a problem hiding this comment.
yes, I agree. I'll change it
Member
Author
|
Will merge it sooner |
Leemoonsoo
reviewed
May 5, 2017
Member
Leemoonsoo
left a comment
Leemoonsoo
left a comment
There was a problem hiding this comment.
LGTM, but can you take care of a comment i left?
| import java.util.Properties; | ||
|
|
||
| /** | ||
| * Created by jl on 06/05/2017. |
Member
There was a problem hiding this comment.
Author tag is not too much encouraged.
Member
Author
There was a problem hiding this comment.
That's my mistake. I'll remove it.
michelemilesi
pushed a commit
to icteam-spa/zeppelin
that referenced
this pull request
May 11, 2017
### What is this PR for? RemoteInterpreter is only used in the server side then zeppelin-interpreter doesn't have to include this class. Moving this class helps to reduce interpreter binary size and change RemoteInterpreter without adding more dependencies if we want ### What type of PR is it? [Refactoring] ### Todos * [x] - Move RemoteInterpreter and related files out of zeppelin-interpreter module ### What is the Jira issue? N/A ### How should this be tested? N/A ### Screenshots (if appropriate) ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Jongyoul Lee <[email protected]> Closes apache#2320 from jongyoul/minor/move-remoteinterpreter-into-zengine and squashes the following commits: 8097991 [Jongyoul Lee] Removed author tag e1425df [Jongyoul Lee] Adopted DummyInterpreter 99c0932 [Jongyoul Lee] Made DummyInterpreter 5ac8dfb [Jongyoul Lee] Moved RemoteInterpreterServer to zeppelin-interpreter 0a881c1 [Jongyoul Lee] Removed unused package imported Removed unnecessary classes imported b7e0b94 [Jongyoul Lee] moved some files related remote interpreter and fix some minor things 7e87215 [Jongyoul Lee] move some files of remote packages from zeppelin-interpreter to zeppelin-zengine
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is this PR for?
RemoteInterpreter is only used in the server side then zeppelin-interpreter doesn't have to include this class. Moving this class helps to reduce interpreter binary size and change RemoteInterpreter without adding more dependencies if we want
What type of PR is it?
[Refactoring]
Todos
What is the Jira issue?
N/A
How should this be tested?
N/A
Screenshots (if appropriate)
Questions: