You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by Leemoonsoo <gi...@git.apache.org> on 2016/03/21 19:09:43 UTC
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-95] Make environment va...
GitHub user Leemoonsoo opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/789
[ZEPPELIN-95] Make environment variables and jvm properties customizable per Interpreter
### What is this PR for?
This PR make environment variable and jvm properties customizable per interpreter settings.
by export property value as environment variable when property key is matches pattern [A-Z_0-9] otherwise set property value as JVM property.
### What type of PR is it?
Feature
### Todos
* [x] - export property as environment variable when property name matches pattern [A-Z_0-9]
* [x] - set property as JVM property when property name does not match pattern [A-Z_0-9]
* [x] - unit test
* [x] - doc
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-95
### How should this be tested?
set some property in interpreter setting page, such as
| name | value|
| ------------- | ------------- |
| MY_ENV_VAR | some value |
| my.jvm.property | some value |
And read value in interpreter. (for example spark interpreter let user access environment variable and jvm property)
### Screenshots (if appropriate)
N/A
### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? yes
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Leemoonsoo/incubator-zeppelin ZEPPELIN-95
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/789.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 #789
----
commit f5a130a40ef2662e467e9d0e5c87c81b5e9fac36
Author: Lee moon soo <mo...@apache.org>
Date: 2016-03-21T17:56:02Z
export interpreter property as environment variable or jvm property
commit 077c8ccc915abd8914353072aa4f9e2dcef60822
Author: Lee moon soo <mo...@apache.org>
Date: 2016-03-21T18:04:10Z
Add docs
----
---
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: [ZEPPELIN-95] Make environment va...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-204737179
+1 @bzz. And this PR 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: [ZEPPELIN-95] Make environment va...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-201949330
@elbamos Can you provide link to your explanation in multiple forums? i didn't see any.
---
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: [ZEPPELIN-95] Make environment va...
Posted by elbamos <gi...@git.apache.org>.
Github user elbamos commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-201953405
No - if I have to do that, I think it will have to go to the community, so instead of wasting my time let's just discuss it there.
> On Mar 26, 2016, at 7:21 PM, Lee moon soo <no...@github.com> wrote:
>
> @elbamos Can you provide link to your explanation in multiple forums? i didn't see any.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly or view it on GitHub
>
---
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: [ZEPPELIN-95] Make environment va...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-202126499
@elbamos If you have objection in this PR, not me, but you'll need to explain your **reason** in this pullrequest comment. And i'm repeating that pullrequest comment is automatically sent to dev mailing list. Please get yourself more familiarize with [contribution guide](https://github.com/apache/incubator-zeppelin/blob/master/CONTRIBUTING.md) if you want to participate the community.
---
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: [ZEPPELIN-95] Make environment va...
Posted by elbamos <gi...@git.apache.org>.
Github user elbamos commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-201866799
No -echarles is right. The current preference system is terribly confusing and often inconsistent. It is a source of user complaint.
This PR in its current form will only make the situation worse.
It's also not clear that it's even fully defined in terms of what preference settings will take precedence over what.
> On Mar 26, 2016, at 2:12 AM, Lee moon soo <no...@github.com> wrote:
>
> Thanks @echarles for the review. Having separate ui for env variables can be a future improvement, but i also think having env and java properties mixed is good for now.
> Merge if there're no more discussions.
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly or view it on GitHub
>
---
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: [ZEPPELIN-95] Make environment va...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-201716379
Thanks @echarles for the review. Having separate ui for env variables can be a future improvement, but i also think having env and java properties mixed is good for now.
Merge if there're no more discussions.
---
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: [ZEPPELIN-95] Make environment va...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-200419212
This patch is ready, please review.
---
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: [ZEPPELIN-95] Make environment va...
Posted by bzz <gi...@git.apache.org>.
Github user bzz commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-202170740
@Leemoonsoo code looks good and rationale for customizing at least JVM properties now and leaving ui for env vars for further improvements makes sense.
If there are other PRs that handle this issue better - we can discuss alternative approaches and pick what's better implemented, otherwise @Leemoonsoo contribution looks good to merge for 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: [ZEPPELIN-95] Make environment va...
Posted by echarles <gi...@git.apache.org>.
Github user echarles commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-201174476
Thx, Makes sense.
The rolls would be to have separated ui for env variables, but having env and java properties mixed in the interpreter settings is good for 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] incubator-zeppelin pull request: [ZEPPELIN-95] Make environment va...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-201968581
@elbamos Note that, every comments in PullRequest already being broadcasted to developer mailing list.
If you have any other subject to discuss about development, feel free to start discussion on developer mailing list.
---
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: [ZEPPELIN-95] Make environment va...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/789
---
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: [ZEPPELIN-95] Make environment va...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-201166366
The reason set env in RemoteInterpreter is, environment variable need to set before the process creation. So shell script (./bin/interpreter.sh) can take these environment variables.
The reason system properties set in RemoteInterpreterServer is, if system properties are set through commandline argument, very long list of system properties may exceeds the limit of maximum argument length on the system.
I think this PR will not impact much about Spark interpreter and it's properties (e.g. `spark.executor.memory`). Without this PR, Spark interpreter already overrides spark conf value from zeppelin interpreter properties.
---
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: [ZEPPELIN-95] Make environment va...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-210248478
Merge if there're no more discussions.
---
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: [ZEPPELIN-95] Make environment va...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-201871771
@elbamos
> No -echarles is right. The current preference system is terribly confusing and often inconsistent. It is a source of user complaint.
Can you explain exactly what is "terribly confusing and often inconsistent?"
> This PR in its current form will only make the situation worse.
Can you add detail why you think this PR make situation worse?
> It's also not clear that it's even fully defined in terms of what preference settings will take precedence over what.
Can you add more detail here, too?
---
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: [ZEPPELIN-95] Make environment va...
Posted by elbamos <gi...@git.apache.org>.
Github user elbamos commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-202133105
Moon I've explained the objection to you several times. You're choosing to ignore that. I am therefore telling you don't have a consensus and it needs to be discussed with the community. If you're unwilling to commence that discussion, then I will commence it.
What I will not do, is continue to spend my time, trying to "convince" someone who has made abundantly clear that they have no interest in respecting a community process and will simply ignore objections they don't like.
I will be commencing the discussion on the mailing list later on when I have a few minutes to write the email, since you have expressed a preference not to do it.
> On Mar 27, 2016, at 3:31 PM, Lee moon soo <no...@github.com> wrote:
>
> Please note that reason must be provided for objection to the code contribution.
> Unless this PR doesn't have any proper objection comes with reason, i'm merging it.
>
> @elbamos Please find someone who can explain instead of you. I honestly can not understand what you're trying to say.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly or view it on GitHub
>
---
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: [ZEPPELIN-95] Make environment va...
Posted by elbamos <gi...@git.apache.org>.
Github user elbamos commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-202116183
Moon - will you be raising this on the mailing list or are you abandoning the issue?
---
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: [ZEPPELIN-95] Make environment va...
Posted by vgmartinez <gi...@git.apache.org>.
Github user vgmartinez commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-205201358
+1...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: [ZEPPELIN-95] Make environment va...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-201964685
@elbamos
If the 'community' you mean is not a Zeppelin community, please understand that I don't discuss / review the code contribution anywhere Zeppelin mailing list can not archive for the history.
Please feel free to provide links to your opinion anytime you think it's not wasting time.
---
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.
---
Re: [GitHub] incubator-zeppelin pull request: [ZEPPELIN-95] Make environment va...
Posted by "Amos B. Elberg" <am...@gmail.com>.
Moon - is there really such an urgency with this PR that it can't wait 12-24 hours for a discussion over whether we should do it before we have an interface to support it?
> On Mar 27, 2016, at 7:10 PM, Leemoonsoo <gi...@git.apache.org> wrote:
>
> Github user Leemoonsoo commented on the pull request:
>
> https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-202167530
>
> @elbamos
>
>> I've explained the objection to you several times.
>
> False.
>
> Here's your objection https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-201866799, but there're no reason or explanation after. So i asked multiple times but you didn't provide any, and you just saying 'let's discuss in dev list'. And i suggested you open any discussion on dev list, but you didn't. And now you claim you have been ignored.
>
> Am i missing something?
>
>
> ---
> 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: [ZEPPELIN-95] Make environment va...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-202167530
@elbamos
> I've explained the objection to you several times.
False.
Here's your objection https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-201866799, but there're no reason or explanation after. So i asked multiple times but you didn't provide any, and you just saying 'let's discuss in dev list'. And i suggested you open any discussion on dev list, but you didn't. And now you claim you have been ignored.
Am i missing something?
---
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: [ZEPPELIN-95] Make environment va...
Posted by elbamos <gi...@git.apache.org>.
Github user elbamos commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-201965711
I meant the developer mailing list for Zeppelin. We can continue the discussion there in the presence of others.
> On Mar 26, 2016, at 8:51 PM, Lee moon soo <no...@github.com> wrote:
>
> @elbamos
> If the 'community' you mean is not a Zeppelin community, please understand that I don't discuss / review the code contribution anywhere Zeppelin mailing list can not archive for the history.
>
> Please feel free to provide links to your opinion anytime you think it's not wasting time.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly or view it on GitHub
>
---
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: [ZEPPELIN-95] Make environment va...
Posted by elbamos <gi...@git.apache.org>.
Github user elbamos commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-201949274
I've explained this several times already in multiple forums. If you don't want to accept it, then you should raise the issue on the developers list so the community can participate.
> On Mar 26, 2016, at 12:05 PM, Lee moon soo <no...@github.com> wrote:
>
> @elbamos
>
> No -echarles is right. The current preference system is terribly confusing and often inconsistent. It is a source of user complaint.
>
> Can you explain exactly what is "terribly confusing and often inconsistent?"
>
> This PR in its current form will only make the situation worse.
>
> Can you add detail why you think this PR make situation worse?
>
> It's also not clear that it's even fully defined in terms of what preference settings will take precedence over what.
>
> Can you add more detail here, too?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly or view it on GitHub
>
---
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: [ZEPPELIN-95] Make environment va...
Posted by elbamos <gi...@git.apache.org>.
Github user elbamos commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-202127132
I'm not sure why it is you don't want to send the email to the list, but fine, I will do so later on.
When you decided to submit this to apache the second time, after its initial rejection, it ceased to be a "lee moon soo" project and became a community project. Its not only a community project when the community does what you want.
If you need instruction on what that means, I'm sure I can find someone who can explain to 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] incubator-zeppelin pull request: [ZEPPELIN-95] Make environment va...
Posted by jeffsteinmetz <gi...@git.apache.org>.
Github user jeffsteinmetz commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-202180621
after these merged changes, does the build still make use of `-Drscala.version=1.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] incubator-zeppelin pull request: [ZEPPELIN-95] Make environment va...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-202128120
Please note that reason must be provided for objection to the code contribution.
Unless this PR doesn't have any proper objection comes with reason, i'm merging it.
@elbamos Please find someone who can explain instead of you. I honestly can not understand what you're trying to say.
---
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: [ZEPPELIN-95] Make environment va...
Posted by echarles <gi...@git.apache.org>.
Github user echarles commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/789#issuecomment-201156913
Current patch sets env in RemoteInterpreter and system properties in RemoteInterpreterServer. Why is this unbalanced (I would have expected setting env and props at the same place).
What is the impact of settings the props for e.g. Spark. Work has been done to rely on the configured deployed spark (if any). Would e.g. `spark.executor.memory` override the value of the deployed spark distribution?
---
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.
---