You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by zjffdu <gi...@git.apache.org> on 2017/01/05 06:21:48 UTC
[GitHub] zeppelin pull request #1846: ZEPPELIN-1770. Restart only the client user's i...
GitHub user zjffdu opened a pull request:
https://github.com/apache/zeppelin/pull/1846
ZEPPELIN-1770. Restart only the client user's interpreter when restarting interpreter setting
### What is this PR for?
This PR would only restart the trigger user's interpreter rather than all the interpreter. So that restarting won't affect other users.
### What type of PR is it?
[Improvement]
### Todos
* [ ] - Task
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-1770
### How should this be tested?
Tested manually.
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zjffdu/zeppelin ZEPPELIN-1770
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1846.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1846
----
commit 903322f97f4b719abfd13bfb9e6baeda1da52493
Author: Jeff Zhang <zj...@apache.org>
Date: 2016-12-14T07:04:20Z
ZEPPELIN-1770. Restart only the client user's interpreter when restarting interpreter setting
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1846: ZEPPELIN-1770. Restart only the client user's interpre...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/1846
CI failure is not relevant.
```
Failed tests:
NotebookTest.testAbortParagraphStatusOnInterpreterRestart:760 expected:<ABORT> but was:<RUNNING>
AngularElem
- should provide onclick method *** FAILED ***
The code passed to eventually never returned normally. Attempted 1 times over 2.966494567 seconds. Last failure message: 0 was not equal to 1. (AbstractAngularElemTest.scala:64)
AngularElem
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1846: ZEPPELIN-1770. Restart only the client user's interpre...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/1846
Tests LGTM. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1846: ZEPPELIN-1770. Restart only the client user's interpre...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1846
@zjffdu Thanks for the patch!
I also think this PR deserves a unittest to make sure correct restart behavior, like @felixcheung mentioned, if it is not too much difficult.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1846: ZEPPELIN-1770. Restart only the client user's interpre...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/1846
ping @jongyoul @Leemoonsoo @felixcheung
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1846: ZEPPELIN-1770. Restart only the client user's interpre...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/1846
Thanks @Leemoonsoo Unit test is added. Please help review \cc @felixcheung @jongyoul
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1846: ZEPPELIN-1770. Restart only the client user's interpre...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/zeppelin/pull/1846
oh gosh :)
is it possible to have test for this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1846: ZEPPELIN-1770. Restart only the client user's interpre...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/1846
Good catch. LGTM @felixcheung We had too big InterpreterFactory, InterpreterSetting and InterpreterGroup. I will refactor all of this class by next major release - 0.8.0 - and will make them testable. I think we have to do it now. :-)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #1846: ZEPPELIN-1770. Restart only the client user's i...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zeppelin/pull/1846
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #1846: ZEPPELIN-1770. Restart only the client user's i...
Posted by zjffdu <gi...@git.apache.org>.
GitHub user zjffdu reopened a pull request:
https://github.com/apache/zeppelin/pull/1846
ZEPPELIN-1770. Restart only the client user's interpreter when restarting interpreter setting
### What is this PR for?
This PR would only restart the trigger user's interpreter rather than all the interpreter. So that restarting won't affect other users.
### What type of PR is it?
[Improvement]
### Todos
* [ ] - Task
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-1770
### How should this be tested?
Tested manually.
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zjffdu/zeppelin ZEPPELIN-1770
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1846.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1846
----
commit 8cb28a31b96c71d749f40e850e886958954136ff
Author: Jeff Zhang <zj...@apache.org>
Date: 2016-12-14T07:04:20Z
ZEPPELIN-1770. Restart only the client user's interpreter when restarting interpreter setting
commit 5ee076dd93d74d50f34aa3ef27b49450b714c5f2
Author: Jeff Zhang <zj...@apache.org>
Date: 2017-01-10T06:07:33Z
fix scoped mode and add unit test
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #1846: ZEPPELIN-1770. Restart only the client user's i...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu closed the pull request at:
https://github.com/apache/zeppelin/pull/1846
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1846: ZEPPELIN-1770. Restart only the client user's interpre...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1846
[AuthenticationIT.java](https://github.com/apache/zeppelin/blob/master/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java) is test that uses zeppelin with shiro enabled. However it's selenium test and bit difficult to make.
I think it's possible to make unittest by calling methods in InterpreterFactory class, directly.
Like [this test](https://github.com/apache/zeppelin/blob/master/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java#L148).
Let me know if you need any help.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1846: ZEPPELIN-1770. Restart only the client user's interpre...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/1846
@jongyoul agreen on refactoring Interpreter component, I also mention that in other PRs. I'd love to help on that :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1846: ZEPPELIN-1770. Restart only the client user's interpre...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/zeppelin/pull/1846
Sounds good!!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1846: ZEPPELIN-1770. Restart only the client user's interpre...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/1846
LGTM, merging if there's no more discussion
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1846: ZEPPELIN-1770. Restart only the client user's interpre...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/1846
@Leemoonsoo Could you point me the right place to put this unit test? I think this may need to enable shiro, but I didn't find any test with shiro enabled.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---