Closed juancarlosgl closed 1 year ago
@juancarlosgl I'm going to defer to @jazdw or possibly @kubeliv as to how to proceed on this one. Also can you list out how you tested it or QA has been involved?
@juancarlosgl I stared reviewing this but there are a lot of technical inaccuracies, spelling and grammar mistakes. Can you please schedule a call with @Craig-IAS to go over the language and comments added so far. I will try and give some more feedback after that.
@juancarlosgl I'm going to defer to @jazdw or possibly @kubeliv as to how to proceed on this one. Also can you list out how you tested it or QA has been involved?
Well I saw when we hit start-mango.sh
this script uses getenv.sh almost like certbot-deploy.sh does. So I modified start-mango.sh to call mango_keystore_properties that it is the funtion reported in the ticket that takes the default value for the MA_KEYSTORE_PASSWORD and finally as I do not really want the script to start up mango UI, then I commented the last lines that starts up mango UI:
#!/bin/sh
#
# Copyright (C) 2021 Radix IoT LLC. All rights reserved.
# @author Jared Wiltshire
# @author Matthew Lohbihler
#
set -e
# set options from arguments
for arg in "$@"; do
case "$arg" in
wait) mango_wait=true ;;
esac
done
mango_script_dir="$(cd "$(dirname "$0")" && pwd -P)"
. "$mango_script_dir"/getenv.sh
mango_keystore_properties
#echo "mango_script_dir is $mango_script_dir"
#echo "mango_config is $mango_config"
#mango_start
#if [ "$mango_wait" = true ]; then
# trap the SIGINT signal (Ctrl-C) and stop mango
# trap mango_stop INT TERM
# needed for trap to work
# set +e
# wait for Mango to exit
# wait $mango_pid
#fi
exit 0
@juancarlosgl I'm going to defer to @jazdw or possibly @kubeliv as to how to proceed on this one. Also can you list out how you tested it or QA has been involved?
Well I saw when we hit
start-mango.sh
this script uses getenv.sh almost like certbot-deploy.sh does. So I modified start-mango.sh to call mango_keystore_properties that it is the funtion reported in the ticket that takes the default value for the MA_KEYSTORE_PASSWORD and finally as I do not really want the script to start up mango UI, then I commented following lines:#!/bin/sh # # Copyright (C) 2021 Radix IoT LLC. All rights reserved. # @author Jared Wiltshire # @author Matthew Lohbihler # set -e # set options from arguments for arg in "$@"; do case "$arg" in wait) mango_wait=true ;; esac done mango_script_dir="$(cd "$(dirname "$0")" && pwd -P)" . "$mango_script_dir"/getenv.sh mango_keystore_properties #echo "mango_script_dir is $mango_script_dir" #echo "mango_config is $mango_config" #mango_start #if [ "$mango_wait" = true ]; then # trap the SIGINT signal (Ctrl-C) and stop mango # trap mango_stop INT TERM # needed for trap to work # set +e # wait for Mango to exit # wait $mango_pid #fi exit 0
Anyone available to go on with this PR? @jazdw @terrypacker @Craig-IAS
@juancarlosgl did you schedule a call with @Craig-IAS and do a review yet?
@juancarlosgl We've moved this PR over to the new repo. We've updated the README.md and getenv.sh files in the branch on the new repo.
RAD-2568-print-environment-variables
After investigating the scripts it was determined that there is no issue with the scripts, however, they are tightly coupled to host machines' environmental variables. So some users may not be aware of these requirements and be unaware of why the script is failing and in some cases where default values are being used they may not even know there is a problem.
The suggestion to mitigate this:
For the getEnv.sh script is to provide feedback to the terminal when environmental variables are not found and default values are taken.
Provide a README file to detail the environmental variables required.
Script remains the same we are just printing some variables to guide user about the places where data would be.
example: