ZEPPELIN-2645. Adding way to register RemoteInterpreterServer's port into InterpreterProcess #2562
Conversation
zjffdu
force-pushed
the
ZEPPELIN-2645
branch
2 times, most recently
from
ba2f4e6 to
0048f7c
Compare
|
@Leemoonsoo @jongyoul Please help review, thanks |
| note.setConfig(config); | ||
| notebook.refreshCron(note.getId()); | ||
| Thread.sleep(1 * 1000); | ||
| Thread.sleep(2 * 1000); |
There was a problem hiding this comment.
This is for flaky test
|
LGTM. I agree with using Thrift to implement this feature as we don't have to add new libs more. |
|
Left a question. except for that, LGTM. |
| port = RemoteInterpreterUtils.findRandomAvailablePortOnAllLocalInterfaces(); | ||
| logger.info("Choose port {} for RemoteInterpreterProcess", port); | ||
| callbackHost = RemoteInterpreterUtils.findAvailableHostname(); | ||
| callbackPort = RemoteInterpreterUtils.findRandomAvailablePortOnAllLocalInterfaces(); |
There was a problem hiding this comment.
I think it's not very uncommon that system administrator setup firewall with ports whitelisted.
Once Interpreter is run in remote machines, this randomly assigned port may make firewall setup difficult.
Any thoughts on this?
There was a problem hiding this comment.
Make sense, will update the PR.
…into InterpreterProcess
zjffdu
force-pushed
the
ZEPPELIN-2645
branch
from
0048f7c to
82bd8d0
Compare
|
@Leemoonsoo @jongyoul I implement the portRange in thrift callback server. But didn't do that for the port of |
|
Thanks for review, Will commit if no more comments. |
### What is this PR for? interpreter.cmd works incorrect after #2562 ### What type of PR is it? Bug Fix ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? no Author: tinkoff-dwh <[email protected]> Closes #2607 from tinkoff-dwh/master and squashes the following commits: 1932202 [tinkoff-dwh] Merge remote-tracking branch 'upstream/master' c7f5124 [tinkoff-dwh] Merge remote-tracking branch 'upstream/master' 8baa87b [tinkoff-dwh] [HOTFIX] fix interpreter.cmd script
3 participants
What is this PR for?
Rebase PR #2418 , still use thrift as the communication protocol between zeppelin server and interpreter process. We can change it io netty in future when we implement 2 way communication channel between zeppelin server and interpreter process.
What type of PR is it?
[Bug Fix | Improvement | Feature | Documentation | Hot Fix | Refactoring]
Todos
What is the Jira issue?
Screenshots (if appropriate)
Questions: