ctubbsii commented on a change in pull request #263:
URL: https://github.com/apache/fluo-uno/pull/263#discussion_r542401413
##########
File path: bin/impl/commands.sh
##########
@@ -261,19 +261,18 @@ EOF
}
function uno_status_main() {
- # TODO this should be converted to using pgrep
# shellcheck disable=SC2009
- atmp="$(ps -ef | grep accumulo\\.start | awk '{print $NF "(" $2 ")"}' | tr
'\n' ' ')"
+ atmp="$(pgrep -f accumulo\\.start -a | awk '{pid =
$1;for(i=1;i<=NF;i++)if($i=="org.apache.accumulo.start.Main")print $(i+1)
"("pid")"}' | tr '\n' ' ')"
Review comment:
Nice!
With the switch to pgrep, we can remove the shellcheck exceptions before the
lines that were changed (just remove the line containing the `# shellcheck
disable=SC2009`).
##########
File path: bin/impl/commands.sh
##########
@@ -261,19 +261,18 @@ EOF
}
function uno_status_main() {
- # TODO this should be converted to using pgrep
# shellcheck disable=SC2009
- atmp="$(ps -ef | grep accumulo\\.start | awk '{print $NF "(" $2 ")"}' | tr
'\n' ' ')"
+ atmp="$(pgrep -f accumulo\\.start -a | awk '{pid =
$1;for(i=1;i<=NF;i++)if($i=="org.apache.accumulo.start.Main")print $(i+1)
"("pid")"}' | tr '\n' ' ')"
# shellcheck disable=SC2009
- htmp="$(ps -ef | grep -e hadoop\\.hdfs -e hadoop\\.yarn | tr '.' ' ' | awk
'{print $NF "(" $2 ")"}' | tr '\n' ' ')"
+ htmp="$(pgrep -f hadoop\\. -a | tr '.' ' ' | awk '{print $NF "(" $1 ")"}' |
tr '\n' ' ')"
ztmp="$(pgrep -f QuorumPeerMain | awk '{print "zoo(" $1 ")"}' | tr '\n' ' ')"
if [[ -n $atmp || -n $ztmp || -n $htmp ]]; then
[[ -n $atmp ]] && echo "Accumulo processes running: $atmp"
[[ -n $ztmp ]] && echo "ZooKeeper processes running: $ztmp"
[[ -n $htmp ]] && echo "Hadoop processes running: $htmp"
else
- echo "No components runnning."
+ echo "No components running."
Review comment:
:smiley_cat:
##########
File path: bin/impl/commands.sh
##########
@@ -300,7 +299,7 @@ function uno_fetch_main() {
function uno_wipe_main() {
local yn
uno_kill_main
- read -r -p "Are you sure you want to wipe '$INSTALL'? " yn
+ read -r -p "Are you sure you want to wipe '$INSTALL'? [Y/n] " yn
Review comment:
I don't think this change is necessary. The capital `Y` implies it's the
default if you just press Enter, but it isn't. No is the default. It also
implies that only `Y` and `n` are acceptable responses, but spelling out "yes"
also is acceptable. Is the current wording unclear about what one can enter? If
not, then I'd leave this one the way it was.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
