You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by dawidwys <gi...@git.apache.org> on 2018/07/23 07:12:59 UTC
[GitHub] flink pull request #6388: [FLINK-6222] Allow passing env variables to start ...
GitHub user dawidwys opened a pull request:
https://github.com/apache/flink/pull/6388
[FLINK-6222] Allow passing env variables to start scripts via
Added possibility to pass env variables (e.g. HADOOP_CONF_DIR) through flink-env.sh
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dawidwys/flink FLINK-6222
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/6388.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 #6388
----
commit 7f700d36f2057b38a4ec8873444c3f488447e241
Author: Dawid Wysakowicz <dw...@...>
Date: 2018-07-19T13:36:56Z
[FLINK-6222] Allow passing env variables to start scripts via
flink-env.sh
----
---
[GitHub] flink issue #6388: [FLINK-6222] Allow passing env variables to start scripts...
Posted by StefanRRichter <gi...@git.apache.org>.
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6388
LGTM, except the one remaining indentation problem mentioned by @zentol .
---
[GitHub] flink issue #6388: [FLINK-6222] Allow passing env variables to start scripts...
Posted by dawidwys <gi...@git.apache.org>.
Github user dawidwys commented on the issue:
https://github.com/apache/flink/pull/6388
Thanks for the review @StefanRRichter and @zentol. Will fix the indentation while merging.
---
[GitHub] flink issue #6388: [FLINK-6222] Allow passing env variables to start scripts...
Posted by dawidwys <gi...@git.apache.org>.
Github user dawidwys commented on the issue:
https://github.com/apache/flink/pull/6388
I added a small section. I was thorn a little in the first place if we should make it highly visible in docs as it might be wrongly understood those variables will be distributed to the cluster as well, which is not true. What do you think?
---
[GitHub] flink pull request #6388: [FLINK-6222] Allow passing env variables to start ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/6388
---
[GitHub] flink issue #6388: [FLINK-6222] Allow passing env variables to start scripts...
Posted by dawidwys <gi...@git.apache.org>.
Github user dawidwys commented on the issue:
https://github.com/apache/flink/pull/6388
In the first place I thought it might make providing the variables a bit more flexible with `flink-env.sh`. In the end it is just a bash script, but the longer I think the more convinced I am it was not the right choice. As you said it does not go well with the other opts, plus we might confuse some users.
I will close this PR and open new one with `env.hadoop.home` tomorrow.
---
[GitHub] flink issue #6388: [FLINK-6222] Allow passing env variables to start scripts...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/6388
If a feature isn't visibly documented chances are no one will use it ;)
I'm not sure if the configuration page is the right place to put it, as it so far deals exclusively with settings set in `flink-conf.yaml`. Most notable this line in the introduction sticks out:
```
All configuration is done in conf/flink-conf.yaml, which is expected to be a flat collection of YAML key value pairs with format key: value.
```
You could name it `flink-client-env-sh`, that would make it make it more obvious that it only applies to the client.
However i have to ask, why a separate file in the first place? We already have config options for setting environment variables (`env.java.opts`); couldn't we introduce a separate option for clients?
---
[GitHub] flink pull request #6388: [FLINK-6222] Allow passing env variables to start ...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/6388#discussion_r204750473
--- Diff: flink-dist/src/main/flink-bin/bin/config.sh ---
@@ -96,6 +96,8 @@ DEFAULT_ENV_JAVA_OPTS="" # Optional JVM args
DEFAULT_ENV_JAVA_OPTS_JM="" # Optional JVM args (JobManager)
DEFAULT_ENV_JAVA_OPTS_TM="" # Optional JVM args (TaskManager)
DEFAULT_ENV_SSH_OPTS="" # Optional SSH parameters running in cluster mode
+DEFAULT_YARN_CONF_DIR="" # YARN Configuration Directory, if necessary
--- End diff --
indentation?
---