fluo-env.sh now supports FLUO_JAVA_OPTS set in environment #966
Conversation
This allows to pass FLUO_JAVA_OPTS as a variable from env . reference : #954
| @@ -35,7 +35,9 @@ export FLUO_CONN_PROPS="${FLUO_CONN_PROPS:-${conf}/fluo-conn.properties}" | |||
| ## Fluo log4j configuration | |||
| export FLUO_LOG4J_CONFIG="${FLUO_LOG4J_CONFIG:-${conf}/log4j.properties}" | |||
| ## Java options for Fluo command | |||
| JAVA_OPTS=("-Dlog4j.configuration=file:${FLUO_LOG4J_CONFIG}") | |||
| ##PREPENDING JAVA_OPTS WITH FLUO_JAVA_OPTS ENVIRONMENT VARIABLE | |||
| JAVA_OPTS=("$FLUO_JAVA_OPTS -Dlog4j.configuration=file:${FLUO_LOG4J_CONFIG}") | |||
There was a problem hiding this comment.
This variable is an array*. Each element in the array should be quoted.
* It's not clear to me that this is used consistently as an array later, but at least here, it's an array.
This enables setting FLUO_JAVA_OPTS in environment. Reference : #954
| ##Prepending JAVA_OPTS with FLUO_JAVA_OPTS | ||
| for var in "${FLUO_JAVA_OPTS[@]}" | ||
| do | ||
| JAVA_OPTS=("$var $JAVA_OPTS") |
There was a problem hiding this comment.
You don't need to do the for loop, and you still need the elements separately quoted.
If FLUO_JAVA_OPTS is an array, then all of this can become:
JAVA_OPTS=("${FLUO_JAVA_OPTS[@]}" "-Dlog4j.configuration=file:${FLUO_LOG4J_CONFIG}")If FLUO_JAVA_OPTS is a scalar, then it should look like:
JAVA_OPTS=(${FLUO_JAVA_OPTS} "-Dlog4j.configuration=file:${FLUO_LOG4J_CONFIG}")Personally, I prefer the former, but it does require users to know it's an array, and to set it accordingly. If they do, then they are much better off, because they don't need to deal with messy quote-escaping.
See http://wiki.bash-hackers.org/syntax/arrays for a decent write-up of bash arrays, especially the section titled "Getting Values" which talks about how quoted arrays are expanded, and the "Storing Values" section which shows how elements are space-separated in array assignment. Just like elsewhere in bash, quotes around spaces are a way of escaping that space... so: x=("a b") is an array containing one element, and x=("a" "b") is an array containing two.
There was a problem hiding this comment.
Yes this make sense .. will submit an updated PR .
| @@ -35,7 +35,11 @@ export FLUO_CONN_PROPS="${FLUO_CONN_PROPS:-${conf}/fluo-conn.properties}" | |||
| ## Fluo log4j configuration | |||
| export FLUO_LOG4J_CONFIG="${FLUO_LOG4J_CONFIG:-${conf}/log4j.properties}" | |||
| ## Java options for Fluo command | |||
| JAVA_OPTS=("-Dlog4j.configuration=file:${FLUO_LOG4J_CONFIG}") | |||
| JAVA_OPTS="-Dlog4j.configuration=file:${FLUO_LOG4J_CONFIG}" | |||
There was a problem hiding this comment.
No longer need this first line here, since it is overridden two lines lower now. :)
…AVA_OPTS as environment variables .
|
@srikailash thanks for the contribution. It seems the email address you used in the commit is not linked to your github account. If you want you can got to your github account settings and add that email address. Then when someone looks at the commits your github account will show up. If you would like to be added to Fluo's people page you can give me the info or submit a PR to the website repo modifying people.md. Are you ok with @ApacheFluo tweeting about your first contribution? If so, do you have a twitter user we could mention? |
Appended FLUO_JAVA_OPTS to JAVA_OPTS in fluo-env.sh . This allows to set FLUO_JAVA_OPTS to set from command line . #954