Skip to content

Commit

Permalink
Fix Shellcheck errors by doing as little work as possible (#2022)
Browse files Browse the repository at this point in the history
# Summary | Résumé

These fixes a bunch of shellcheck errors and ignores a few I didn't test, actually I didn't test any of this, most of the fixes were pretty straightforward ones, there was one I fixed with co-pilot that should be tested and the others I didn't fix I just assumed they worked as is since they were in the existing codebase.

- **Fixes**: https://www.shellcheck.net/wiki/SC1091 Couldn't find the file that was being referenced (`environment_test.sh`) in `scripts/run_single_test.sh` so I just the source to /dev/null
- **Fixes**: https://www.shellcheck.net/wiki/SC2028 Replaces some echo's where a control character was passed "\n" with a printf so it should work the same and not complain
- **Fixes the read -p error**: https://www.shellcheck.net/wiki/SC3045 by asking co-pilot to fix it, I can't guarantee this one will work, **YOU SHOULD TEST THIS FILE: scripts/run_celery_purge.sh**
- **Ignores**: https://www.shellcheck.net/wiki/SC2009 I'm assuming it worked and I didn't want to find the pgrep way of doing it, also it's not posix
- **Ignores**: https://www.shellcheck.net/wiki/SC2219 I'm assuming it worked and I didn't want to test if the following works
```bash 
((wait_time++)) || true
```
- **Fixes**: https://www.shellcheck.net/wiki/SC2105 By removing the break from the case, they don't need it.
- **Fixes**: https://www.shellcheck.net/wiki/SC3046 By replacing `source` with `.`
- **Fixes**: https://www.shellcheck.net/wiki/SC2068 by wrapping array Expansion with double quotes
- **Fixes**: https://www.shellcheck.net/wiki/SC2086 By wrapping the variable in double quotes
  • Loading branch information
CalvinRodo authored Nov 15, 2023
1 parent d6d2e79 commit 8bc84d7
Show file tree
Hide file tree
Showing 10 changed files with 26 additions and 22 deletions.
2 changes: 1 addition & 1 deletion scripts/run_celery.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ fi

echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"

celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low,send-email-tasks,service-callbacks,delivery-receipts
celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low,send-email-tasks,service-callbacks,delivery-receipts
2 changes: 1 addition & 1 deletion scripts/run_celery_core_tasks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ fi

echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"

celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,service-callbacks,delivery-receipts
celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,service-callbacks,delivery-receipts
8 changes: 5 additions & 3 deletions scripts/run_celery_exit.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ function get_celery_pids {
# and keep only these PIDs

set +o pipefail # so grep returning no matches does not premature fail pipe
# shellcheck disable=SC2009 # We don't want to bother re-writing this to use pgrep
APP_PIDS=$(ps aux --sort=start_time | grep 'celery worker' | grep 'bin/celery' | head -1 | awk '{print $2}')
set -o pipefail # pipefail should be set everywhere else
}
Expand All @@ -18,12 +19,12 @@ function send_signal_to_celery_processes {
# refresh pids to account for the case that some workers may have terminated but others not
get_celery_pids
# send signal to all remaining apps
echo ${APP_PIDS} | tr -d '\n' | tr -s ' ' | xargs echo "Sending signal ${1} to processes with pids: " >> /proc/1/fd/1
echo "${APP_PIDS}" | tr -d '\n' | tr -s ' ' | xargs echo "Sending signal ${1} to processes with pids: " >> /proc/1/fd/1
echo "We will send ${1} signal" >> /proc/1/fd/1
for value in ${APP_PIDS}
do
echo kill -s ${1} $value
kill -s ${1} $value
echo kill -s "${1}" "$value"
kill -s "${1}" "$value"
done
#echo ${APP_PIDS} | xargs kill -s ${1}
}
Expand Down Expand Up @@ -59,6 +60,7 @@ function on_exit {
echo "exit function is running with wait time of 9s" >> /proc/1/fd/1
get_celery_pids
ensure_celery_is_running
# shellcheck disable=SC2219 # We could probably rewrite it as `((wait_time++)) || true` but I haven't tested and I assume this works as is
let wait_time=wait_time+1
sleep 1
done
Expand Down
2 changes: 1 addition & 1 deletion scripts/run_celery_local.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ set -e

echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"

celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low,send-throttled-sms-tasks,send-email-high,send-email-medium,send-email-low,send-email-tasks,service-callbacks,delivery-receipts
celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low,send-throttled-sms-tasks,send-email-high,send-email-medium,send-email-low,send-email-tasks,service-callbacks,delivery-receipts
2 changes: 1 addition & 1 deletion scripts/run_celery_no_sms_sending.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ fi

echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"

celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-email-tasks,send-email-high,send-email-medium,send-email-low,service-callbacks,delivery-receipts
celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-email-tasks,send-email-high,send-email-medium,send-email-low,service-callbacks,delivery-receipts
19 changes: 10 additions & 9 deletions scripts/run_celery_purge.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@

set -e

echo "\n--------------------------------------------------\n"
echo " WARNING!!!!\n"
echo " This script is for local development only!\n"
echo " It will delete everything in the celery queues.\n"
echo "--------------------------------------------------\n"
echo "Are you sure you want to continue?"
read -p "If so, type 'purge'> " check
printf "\n--------------------------------------------------\n"
printf " WARNING!!!!\n"
printf " This script is for local development only!\n"
printf " It will delete everything in the celery queues.\n"
printf "\n--------------------------------------------------\n"
printf "Are you sure you want to continue?"
echo "If so, type 'purge'> \c"
read -r check
case $check in
purge ) echo "purging!"; celery -A run_celery.notify_celery purge -f; break;;
* ) echo "\nNot purging\n";;
purge ) echo "purging!"; celery -A run_celery.notify_celery purge -f;;
* ) printf "\nNot purging\n";;
esac

2 changes: 1 addition & 1 deletion scripts/run_celery_send_email.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ fi
echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"

# TODO: we shouldn't be using the send-email-tasks queue anymore, once we verify this we can remove it
celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q send-email-tasks,send-email-high,send-email-medium,send-email-low
celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q send-email-tasks,send-email-high,send-email-medium,send-email-low
2 changes: 1 addition & 1 deletion scripts/run_celery_send_sms.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ fi
echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"

# TODO: we shouldn't be using the send-sms-tasks queue anymore - once we verify this we can remove it
celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low
celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low
5 changes: 3 additions & 2 deletions scripts/run_single_test.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#!/bin/sh
# run a single unit test, pass in the unit test name for example: tests/app/service/test_rest.py::test_get_template_list
source environment_test.sh
py.test $@
# shellcheck source=/dev/null # Not finding this file in code base
. environment_test.sh
py.test "$@"
4 changes: 2 additions & 2 deletions scripts/run_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ function display_result {
EXIT_STATUS=$2
TEST=$3

if [ $RESULT -ne 0 ]; then
if [ "$RESULT" -ne 0 ]; then
echo -e "\033[31m$TEST failed\033[0m"
exit $EXIT_STATUS
exit "$EXIT_STATUS"
else
echo -e "\033[32m$TEST passed\033[0m"
fi
Expand Down

0 comments on commit 8bc84d7

Please sign in to comment.