-
Notifications
You must be signed in to change notification settings - Fork 2.8k
ZEPPELIN-1643:Make spark web UI accesible from interpreters page #1613
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
Conversation
| ResourcePool resourcePool; | ||
| boolean angularRegistryPushed = false; | ||
|
|
||
| private RemoteInterpreterEventClient eventClient; |
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.
InterpreterGroup is not only used in RemoteInterpreter process but also in ZeppelinServer side, too.
So having reference to eventClient in InterpreterGroup is not optimal i think.
I think InterpreterContext is better place to have such a reference, and you can inject your object when InterpreterContext is being created.
InterpreterContext is not passed to interpreter in open(). So getting routines that getting SparkUI and update meta need to be moved from open() to interpret(). I think it's not a big problem in this use case.
Also i think it'll be better passing wrapper object instead of eventClient directly. Because of we don't want to let interpreter directly access all eventClient's methods.
Leemoonsoo
left a comment
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.
Thanks @karup1990 for the very useful contribution.
Left some comments. please check.
|
@Leemoonsoo Thanks for the suggestions. |
|
Ready for review |
|
Failure seems unrelated. |
|
LGTM. Merge if there're no further discussions. |
538225c to
350d665
Compare
### What is this PR for? Recently, zeppelin added some amazing features. #1613 You can easily access spark ui from the zeppelin web. However, in the implementation of the function, the Thrift interface seems to have arbitrarily modified the automatically generated code. So if you recreate the Thift interface by calling genthrift.sh. Zeppelin is not built. I modified this to get the same result with auto-generated code. ### What type of PR is it? Bug Fix ### Todos - [x] add event META_INFOS in RemoteInterpreterService.thrift - [x] import generate code ### What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-1679 ### How should this be tested? 1. First you need to install thift 0.9.2. 2. Run zeppelin-interpreter / src / main / thrift / generate_thrift.sh. 3. Build full zeppelin. 4. After build, please check the interpreter spark setting in our web ui. 5. Try using the spark ui button. If everything is running normally, it is a success. Problems occur in the current master 3. ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? no Author: CloverHearts <[email protected]> Closes #1650 from cloverhearts/fix-thift and squashes the following commits: ebd2838 [CloverHearts] fixed incorrect thrift implementation.
|
If I read it correctly, this PR assume each interpreter setting has one spark ui url ? But I think this is wrong assumption, as we may have multiple spark applications for single interpreter setting. @karup1990 @Leemoonsoo |
|
I think the right place to put spark ui link is in note page instead of interpreter setting page. |
|
Make sense. @karup1990 what do you think? |
|
@zjffdu @Leemoonsoo Thanks for letting me know. |
### What is this PR for? Make spark web UI accesible from interpreters page ### What type of PR is it? Improvement ### Todos NA ### What is the Jira issue? ZEPPELIN-1643 ### How should this be tested? Start sparkcontext. Goto interpreters page. Corresponding to the spark interpreter used in notebook, there will be *spark ui* button. Clicking the button should open a new tab with the application's spark web UI. ### Screenshots (if appropriate)  ### Questions: * Does the licenses files need update? NA * Is there breaking changes for older versions? No * Does this needs documentation? No Author: karuppayya <[email protected]> Author: Karup <[email protected]> Closes apache#1613 from karup1990/ZEPPELIN-1643 and squashes the following commits: 29e9812 [karuppayya] Fix checkstyle 350d665 [karuppayya] Fix coding err 42dde76 [Karup] Fix typo 5cfed2a [karuppayya] Fix test failure 55c45c9 [karuppayya] Address feedback 4d97196 [karuppayya] Remove comments a1304a2 [karuppayya] Change to make spark web UI accesible from interpreters page
### What is this PR for? Recently, zeppelin added some amazing features. apache#1613 You can easily access spark ui from the zeppelin web. However, in the implementation of the function, the Thrift interface seems to have arbitrarily modified the automatically generated code. So if you recreate the Thift interface by calling genthrift.sh. Zeppelin is not built. I modified this to get the same result with auto-generated code. ### What type of PR is it? Bug Fix ### Todos - [x] add event META_INFOS in RemoteInterpreterService.thrift - [x] import generate code ### What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-1679 ### How should this be tested? 1. First you need to install thift 0.9.2. 2. Run zeppelin-interpreter / src / main / thrift / generate_thrift.sh. 3. Build full zeppelin. 4. After build, please check the interpreter spark setting in our web ui. 5. Try using the spark ui button. If everything is running normally, it is a success. Problems occur in the current master 3. ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? no Author: CloverHearts <[email protected]> Closes apache#1650 from cloverhearts/fix-thift and squashes the following commits: ebd2838 [CloverHearts] fixed incorrect thrift implementation.
|
I did a code reading to see how to fix this.. Looks like I will need to handle it per user as well. |
|
@zjffdu @Leemoonsoo I have handled the cases in #1678 |
Make spark web UI accesible from interpreters page Improvement NA ZEPPELIN-1643 Start sparkcontext. Goto interpreters page. Corresponding to the spark interpreter used in notebook, there will be *spark ui* button. Clicking the button should open a new tab with the application's spark web UI.  * Does the licenses files need update? NA * Is there breaking changes for older versions? No * Does this needs documentation? No Author: karuppayya <[email protected]> Author: Karup <[email protected]> Closes apache#1613 from karup1990/ZEPPELIN-1643 and squashes the following commits: 29e9812 [karuppayya] Fix checkstyle 350d665 [karuppayya] Fix coding err 42dde76 [Karup] Fix typo 5cfed2a [karuppayya] Fix test failure 55c45c9 [karuppayya] Address feedback 4d97196 [karuppayya] Remove comments a1304a2 [karuppayya] Change to make spark web UI accesible from interpreters page (cherry picked from commit 96ca84d)
What is this PR for?
Make spark web UI accesible from interpreters page
What type of PR is it?
Improvement
Todos
NA
What is the Jira issue?
ZEPPELIN-1643
How should this be tested?
Start sparkcontext.
Goto interpreters page. Corresponding to the spark interpreter used in notebook, there will be spark ui button.
Clicking the button should open a new tab with the application's spark web UI.
Screenshots (if appropriate)
Questions: