You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by zhongneu <gi...@git.apache.org> on 2016/02/26 08:06:48 UTC
[GitHub] incubator-zeppelin pull request: Remove duplicated java option con...
GitHub user zhongneu opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/749
Remove duplicated java option concats in common.sh
### What is this PR for?
There are some java option concats in common.sh, which are executed twice when start an interpreter. This makes some of the options invalid, such as remote debugging options
### What type of PR is it?
Bug Fix
### Todos
### Is there a relevant Jira issue?
[ZEPPELIN-686](https://issues.apache.org/jira/browse/ZEPPELIN-686)
### How should this be tested?
Set remote debug options:
```
export ZEPPELIN_INTP_JAVA_OPTS=-agentlib:jdwp=transport=dt_socket,server=y,address=8000,suspend=n
```
Start the Zeppelin daemon, then create & run a job to trigger starting an interpreter. The job should fail without the fix.
### 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/zhongneu/incubator-zeppelin fix-duplicated-java-opts
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/749.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 #749
----
commit 75959ea259b633e568dc84a95f802f26cbf10be2
Author: Zhong Wang <wa...@gmail.com>
Date: 2016-02-26T06:48:39Z
remove unneccessary concats in common.sh
----
---
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] incubator-zeppelin pull request: Remove duplicated java option con...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/749
---
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] incubator-zeppelin pull request: Remove duplicated java option con...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/749#issuecomment-190965016
what if someone has set JAVA_OPTS in the env? shouldn't we be additive?
---
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] incubator-zeppelin pull request: Remove duplicated java option con...
Posted by zhongneu <gi...@git.apache.org>.
Github user zhongneu commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/749#issuecomment-192104991
I've reverted the change for JAVA_OPTS, which is fine, because it is not used by bin/interpreter.sh
---
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] incubator-zeppelin pull request: Remove duplicated java option con...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/749#issuecomment-190570128
Thanks @zhongneu for the patch.
Looks good to me.
---
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] incubator-zeppelin pull request: Remove duplicated java option con...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/749#issuecomment-192706478
looks good. I think we might want to deprecate JAVA_INTP_OPTS and replace with something more Zeppelin specific for clarify, and allow for admin to override it.
merging if no more comment.
---
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] incubator-zeppelin pull request: Remove duplicated java option con...
Posted by zhongneu <gi...@git.apache.org>.
Github user zhongneu commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/749#issuecomment-191084620
@felixcheung I am not sure about this, but I would like to make it additive if we have such case.
---
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] incubator-zeppelin pull request: Remove duplicated java option con...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/749#issuecomment-192672530
LGTM
---
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] incubator-zeppelin pull request: Remove duplicated java option con...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/749#issuecomment-191588341
I think @felixcheung got valid point. It's incompatible change to people who has JAVA_OPTS.
One alternative is document changes in
https://github.com/apache/incubator-zeppelin/blob/master/docs/install/upgrade.md
---
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] incubator-zeppelin pull request: Remove duplicated java option con...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/749#issuecomment-190867864
Merge if there're no more discsussions
---
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] incubator-zeppelin pull request: Remove duplicated java option con...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/749#issuecomment-191638729
That sounds like a reasonable approach, we could say something like
"if you are setting JAVA_OPT, set ZEPPELIN_JAVA_OPTS instead for Zeppelin, set ZEPPELIN_INTP_JAVA_OPTS instead for interpreter process" in more words ;)
---
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.
---