Claudenw commented on code in PR #1950:
URL: https://github.com/apache/cassandra/pull/1950#discussion_r1113157425
##########
bin/nodetool:
##########
@@ -22,35 +22,8 @@ if [ "`basename "$0"`" = 'nodeprobe' ]; then
echo "***************************************************************" >&2
fi
-if [ "x$CASSANDRA_INCLUDE" = "x" ]; then
- # Locations (in order) to use when searching for an include file.
- for include in "`dirname "$0"`/cassandra.in.sh" \
- "$HOME/.cassandra.in.sh" \
- /usr/share/cassandra/cassandra.in.sh \
- /usr/local/share/cassandra/cassandra.in.sh \
- /opt/cassandra/cassandra.in.sh; do
- if [ -r "$include" ]; then
- . "$include"
- break
- fi
- done
-elif [ -r "$CASSANDRA_INCLUDE" ]; then
- . "$CASSANDRA_INCLUDE"
-fi
-
-if [ -z "$CASSANDRA_CONF" -o -z "$CLASSPATH" ]; then
- echo "You must set the CASSANDRA_CONF and CLASSPATH vars" >&2
- exit 1
-fi
-
-# Run cassandra-env.sh to pick up JMX_PORT
-if [ -f "$CASSANDRA_CONF/cassandra-env.sh" ]; then
- JVM_OPTS_SAVE=$JVM_OPTS
- MAX_HEAP_SIZE_SAVE=$MAX_HEAP_SIZE
- . "$CASSANDRA_CONF/cassandra-env.sh"
- MAX_HEAP_SIZE=$MAX_HEAP_SIZE_SAVE
- JVM_OPTS="$JVM_OPTS_SAVE"
-fi
+ENV_PRESERVE="$ENV_PRESERVE MAX_HEAP_SIZE JVM_OPTS"
+. ./cassandra-conf.sh
Review Comment:
since `cassandra-conf.sh` is intended to be used "as is" without
modification I think it should be referenced in-situ. If one is writing a new
script to source `cassandra-conf.sh` then the full path should be provided.
Any calculation of the location of `cassandra-conf.sh` is, in my mind, risky
and potentially hard to diagnose when an error occurs.
The text:
```
if [ -z $CASSANDRA_HOME ]; then
. $(dirname "$0")/../conf/cassandra-conf.sh
else
. $CASSANDRA_HOME/conf/cassandra-conf.sh
fi
```
won't work because CASSANDRA_HOME is not expected to be set until after
`cassandra.in.sh` which is loaded by `cassandra-conf.sh`, also the
`cassandra-conf.sh` file is expected to be in `bin/` not `conf/`. But even if
`bin/` were used it it would still be incorrect for any new script that was not
in a sibling directory of `bin/`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
