You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by SachinJanani <gi...@git.apache.org> on 2016/07/16 19:15:44 UTC
[GitHub] zeppelin pull request #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
GitHub user SachinJanani opened a pull request:
https://github.com/apache/zeppelin/pull/1197
[ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
### What is this PR for?
Fixed issue related to connecting to remote running interpreter process with multiple interpreters in interpreter group throws illegal thread state exception
### What type of PR is it?
Bug Fix
### Todos
### What is the Jira issue?
[ZEPPELIN-1196] https://issues.apache.org/jira/browse/ZEPPELIN-1196
### 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/SachinJanani/zeppelin branch-0.6
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1197.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 #1197
----
commit b922b8ffe60c1512ab54f8763ebeaaf1158e9efb
Author: Sachin <sj...@snappydata.io>
Date: 2016-07-16T19:00:26Z
[ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
----
---
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 #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/1197
@SachinJanani I tried to merge it but this is based on branch-0.6. Could you please rebase from current master? I've tried to do it but I occurs error about merge conflict. Can you handle it? And for the next time, you'd better contribute the codes to avoid this situation.
---
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 #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by SachinJanani <gi...@git.apache.org>.
Github user SachinJanani commented on the issue:
https://github.com/apache/zeppelin/pull/1197
Can someone please review this PR
---
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 #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/1197
@SachinJanani 30 seconds would be realistic. And could you please leave memo that it may occurs a potential bug when it fails launching it?
---
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 #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by SachinJanani <gi...@git.apache.org>.
Github user SachinJanani commented on the issue:
https://github.com/apache/zeppelin/pull/1197
@jongyoul One more thing I found that as per ZEPPELIN_INTERPRETER_CONNECT_TIMEOUT property default timeout that we consider is 30 seconds.So I think setting it to 30 seconds will make more sense.
---
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 #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by SachinJanani <gi...@git.apache.org>.
Github user SachinJanani commented on the issue:
https://github.com/apache/zeppelin/pull/1197
@jongyoul This changes are only for branch-0.6 as master does not have this files (even this code) anymore due to helium changes.Also i think issue might not occur for zeppelin-0.7.So this is fix will only be for zeppelin-0.6.1.I think it should be only be merged in branch-0.6.
---
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 #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/1197
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 pull request #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zeppelin/pull/1197
---
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 #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by SachinJanani <gi...@git.apache.org>.
Github user SachinJanani commented on the issue:
https://github.com/apache/zeppelin/pull/1197
One of the travis check is failing but its not related to my changes
---
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 #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by SachinJanani <gi...@git.apache.org>.
Github user SachinJanani commented on the issue:
https://github.com/apache/zeppelin/pull/1197
Thanks @jongyoul. Sure will make the change also will add a comment in the code about about the 30 seconds timeout
---
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 #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/1197
I'll merge it after 24 hours.
---
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 #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by minahlee <gi...@git.apache.org>.
Github user minahlee commented on the issue:
https://github.com/apache/zeppelin/pull/1197
@jongyoul @SachinJanani I managed it to be merged only to branch-0.6. Will close this PR manually.
---
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 #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/1197
@SachinJanani Thanks for the quick fix, but if it fails to launch RemoteInterpreterServer with 10 seconds, this test always fails, isn't it? how to deal with that case? I think we need to make it more concrete. How about you?
---
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 #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by SachinJanani <gi...@git.apache.org>.
Github user SachinJanani commented on the issue:
https://github.com/apache/zeppelin/pull/1197
Thanks @minahlee
---
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 #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/1197
@SachinJanani Hi, can you add a test case for it? It would help understand and test this PR.
---
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 #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by SachinJanani <gi...@git.apache.org>.
Github user SachinJanani commented on the issue:
https://github.com/apache/zeppelin/pull/1197
@jongyoul Yes you are right if RemoteInterpreterServer fails to launch within 10 seconds then this test will fail,but we need to have some threshold for that because if say there is some bug in RemoteInterpreterServer which causes it to hang then our test will never complete.Should I increase it to 20 seconds.What you think?
---
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 #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by SachinJanani <gi...@git.apache.org>.
Github user SachinJanani commented on the issue:
https://github.com/apache/zeppelin/pull/1197
@jongyoul Thanks for reviewing.As suggested I have added a test case for this PR
---
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 #1197: [ZEPPELIN-1196] Fix for bug ZEPPELIN-1196
Posted by SachinJanani <gi...@git.apache.org>.
Github user SachinJanani commented on the issue:
https://github.com/apache/zeppelin/pull/1197
Can someone please merge this PR as I don't have permission
---
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.
---