From dab9ed00f9f4df2ff7c5b5f90d9046d886eab143 Mon Sep 17 00:00:00 2001 From: snickerjp Date: Wed, 31 Dec 2025 16:42:43 +0000 Subject: [PATCH 1/8] Add clean progress indication with --debug option Replace cluttered Docker Compose progress output with clean dot-based progress indication by default. Add --debug (-g) option to show detailed Docker progress when needed. Default behavior: - Shows user-friendly messages with dot progress - Clean terminal output Debug mode (--debug): - Shows full Docker Compose progress output - Useful for troubleshooting installation issues --- setup.sh | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/setup.sh b/setup.sh index 617c8bb7..645fc370 100755 --- a/setup.sh +++ b/setup.sh @@ -8,6 +8,7 @@ DONT_START=no OVERWRITE=no PREVIEW=no REDASH_VERSION="" +DEBUG=no # Ensure the script is being run as root ID=$(id -u) @@ -30,7 +31,7 @@ elif [ ! -f /etc/os-release ]; then fi # Parse any user provided parameters -opts="$(getopt -o doph -l dont-start,overwrite,preview,help,version: --name "$0" -- "$@")" +opts="$(getopt -o dophg -l dont-start,overwrite,preview,help,debug,version: --name "$0" -- "$@")" eval set -- "$opts" while true; do @@ -47,13 +48,18 @@ while true; do PREVIEW=yes shift ;; + -g | --debug) + DEBUG=yes + shift + ;; --version) REDASH_VERSION="$2" shift 2 ;; -h | --help) - echo "Redash setup script usage: $0 [-d|--dont-start] [-p|--preview] [-o|--overwrite] [--version ]" + echo "Redash setup script usage: $0 [-d|--dont-start] [-p|--preview] [-g|--debug] [-o|--overwrite] [--version ]" echo " The --preview (also -p) option uses the Redash 'preview' Docker image instead of the last stable release" + echo " The --debug (also -g) option shows detailed Docker progress output instead of clean progress dots" echo " The --version option installs the specified version tag of Redash (e.g., 10.1.0)" echo " The --overwrite (also -o) option replaces any existing configuration with a fresh new install" echo " The --dont-start (also -d) option installs Redash, but doesn't automatically start it afterwards" @@ -313,10 +319,28 @@ startup() { echo "** Starting Redash **" echo "*********************" echo "** Initialising Redash database **" - docker compose run --rm server create_db + if [ "$DEBUG" = "yes" ]; then + docker compose run --rm server create_db + else + echo -n "Downloading images" + docker compose pull > /dev/null 2>&1 & + PID=$! + while kill -0 $PID 2>/dev/null; do + echo -n "." + sleep 2 + done + echo " Done!" + echo "Creating database..." + docker compose run --rm server create_db + fi echo "** Starting the rest of Redash **" - docker compose up -d + if [ "$DEBUG" = "yes" ]; then + docker compose up -d + else + echo "Starting containers..." + docker compose up -d + fi echo echo "Redash has been installed and is ready for configuring at http://$(hostname -f):5000" From 4c11e578423cad9680c564f81eed7035ee3c212f Mon Sep 17 00:00:00 2001 From: snickerjp Date: Wed, 31 Dec 2025 16:43:08 +0000 Subject: [PATCH 2/8] Add error handling with debug mode suggestion When installation fails in default (clean) mode, show a helpful error message suggesting users run with --debug flag for detailed output. Features: - Clear error indication with emoji - Actionable troubleshooting advice - Maintains clean UX while providing debugging path - Applied to critical Docker Compose operations This helps users quickly identify and resolve installation issues. --- setup.sh | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/setup.sh b/setup.sh index 645fc370..20376137 100755 --- a/setup.sh +++ b/setup.sh @@ -10,6 +10,17 @@ PREVIEW=no REDASH_VERSION="" DEBUG=no +# Error handling function +handle_error() { + if [ "$DEBUG" != "yes" ]; then + echo + echo "❌ An error occurred during installation." + echo "💡 For detailed output, please run: $0 --debug" + echo + fi + exit 1 +} + # Ensure the script is being run as root ID=$(id -u) if [ "0$ID" -ne 0 ]; then @@ -320,7 +331,7 @@ startup() { echo "*********************" echo "** Initialising Redash database **" if [ "$DEBUG" = "yes" ]; then - docker compose run --rm server create_db + docker compose run --rm server create_db || handle_error else echo -n "Downloading images" docker compose pull > /dev/null 2>&1 & @@ -329,17 +340,21 @@ startup() { echo -n "." sleep 2 done + wait $PID || { + echo " Failed!" + handle_error + } echo " Done!" echo "Creating database..." - docker compose run --rm server create_db + docker compose run --rm server create_db || handle_error fi echo "** Starting the rest of Redash **" if [ "$DEBUG" = "yes" ]; then - docker compose up -d + docker compose up -d || handle_error else echo "Starting containers..." - docker compose up -d + docker compose up -d || handle_error fi echo From 730700ed4042f2814312a7b16a3b61a72bff5ea7 Mon Sep 17 00:00:00 2001 From: snickerjp Date: Wed, 31 Dec 2025 16:45:57 +0000 Subject: [PATCH 3/8] Apply shell formatting with shfmt Fix minor formatting inconsistencies in the newly added code sections: - Standardize redirection spacing (>/dev/null instead of > /dev/null) This maintains consistency with the existing codebase formatting. --- setup.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.sh b/setup.sh index 20376137..b4adfa3d 100755 --- a/setup.sh +++ b/setup.sh @@ -334,7 +334,7 @@ startup() { docker compose run --rm server create_db || handle_error else echo -n "Downloading images" - docker compose pull > /dev/null 2>&1 & + docker compose pull >/dev/null 2>&1 & PID=$! while kill -0 $PID 2>/dev/null; do echo -n "." From 28d39e3f5e4773ee2aafc063e1e8ae6bb9fad160 Mon Sep 17 00:00:00 2001 From: snickerjp Date: Wed, 31 Dec 2025 17:05:44 +0000 Subject: [PATCH 4/8] Fix Copilot review issues: process leaks and error handling Address all 6 issues identified in Copilot review: 1. Add signal traps to prevent process leaks on interruption 2. Fix race condition in exit status handling with explicit STATUS check 3. Apply consistent progress suppression to database creation step 4. Suppress docker compose up output in non-debug mode for consistency 5. Use consistent comparison operators (x = xyes pattern) 6. Preserve error messages using temporary files while suppressing progress Key improvements: - Robust background process management with proper cleanup - Complete progress output suppression in clean mode - Error details automatically shown on failure (--debug less critical) - Maintains clean UX while providing excellent troubleshooting info --- setup.sh | 52 ++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/setup.sh b/setup.sh index b4adfa3d..d5048ce3 100755 --- a/setup.sh +++ b/setup.sh @@ -12,7 +12,7 @@ DEBUG=no # Error handling function handle_error() { - if [ "$DEBUG" != "yes" ]; then + if [ "x$DEBUG" != "xyes" ]; then echo echo "❌ An error occurred during installation." echo "💡 For detailed output, please run: $0 --debug" @@ -330,31 +330,63 @@ startup() { echo "** Starting Redash **" echo "*********************" echo "** Initialising Redash database **" - if [ "$DEBUG" = "yes" ]; then + if [ "x$DEBUG" = "xyes" ]; then docker compose run --rm server create_db || handle_error else echo -n "Downloading images" - docker compose pull >/dev/null 2>&1 & + ERROR_LOG=$(mktemp) + docker compose pull >/dev/null 2>"$ERROR_LOG" & PID=$! - while kill -0 $PID 2>/dev/null; do + trap 'kill "$PID" 2>/dev/null || true; rm -f "$ERROR_LOG"' EXIT INT TERM + while kill -0 "$PID" 2>/dev/null; do echo -n "." sleep 2 done - wait $PID || { + wait "$PID" + STATUS=$? + trap - EXIT INT TERM + if [ "$STATUS" -ne 0 ]; then echo " Failed!" + if [ -s "$ERROR_LOG" ]; then + echo "Error details:" + cat "$ERROR_LOG" + fi + rm -f "$ERROR_LOG" handle_error - } + fi + rm -f "$ERROR_LOG" + echo " Done!" + echo -n "Creating database" + ERROR_LOG=$(mktemp) + docker compose run --rm server create_db >/dev/null 2>"$ERROR_LOG" & + PID=$! + trap 'kill "$PID" 2>/dev/null || true; rm -f "$ERROR_LOG"' EXIT INT TERM + while kill -0 "$PID" 2>/dev/null; do + echo -n "." + sleep 2 + done + wait "$PID" + STATUS=$? + trap - EXIT INT TERM + if [ "$STATUS" -ne 0 ]; then + echo " Failed!" + if [ -s "$ERROR_LOG" ]; then + echo "Error details:" + cat "$ERROR_LOG" + fi + rm -f "$ERROR_LOG" + handle_error + fi + rm -f "$ERROR_LOG" echo " Done!" - echo "Creating database..." - docker compose run --rm server create_db || handle_error fi echo "** Starting the rest of Redash **" - if [ "$DEBUG" = "yes" ]; then + if [ "x$DEBUG" = "xyes" ]; then docker compose up -d || handle_error else echo "Starting containers..." - docker compose up -d || handle_error + docker compose up -d >/dev/null 2>&1 || handle_error fi echo From 99420b668f806c834656802c582fd0066c992b19 Mon Sep 17 00:00:00 2001 From: snickerjp Date: Wed, 31 Dec 2025 17:11:55 +0000 Subject: [PATCH 5/8] Fix inconsistent error handling for docker compose up Address cubic-dev-ai feedback: docker compose up -d now captures and displays error details consistently with other docker operations. Previously: - docker compose pull/run: captured errors to temp file, displayed details - docker compose up -d: discarded all error output (>/dev/null 2>&1) Now all docker operations consistently capture and display error details while maintaining clean progress output in non-debug mode. --- setup.sh | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/setup.sh b/setup.sh index d5048ce3..f465f21f 100755 --- a/setup.sh +++ b/setup.sh @@ -386,7 +386,16 @@ startup() { docker compose up -d || handle_error else echo "Starting containers..." - docker compose up -d >/dev/null 2>&1 || handle_error + ERROR_LOG=$(mktemp) + if ! docker compose up -d >/dev/null 2>"$ERROR_LOG"; then + if [ -s "$ERROR_LOG" ]; then + echo "Error details:" + cat "$ERROR_LOG" + fi + rm -f "$ERROR_LOG" + handle_error + fi + rm -f "$ERROR_LOG" fi echo From b0d3dfeec7bbcd2f02eb872124dd2a7d0a7b0bdf Mon Sep 17 00:00:00 2001 From: snickerjp Date: Wed, 31 Dec 2025 17:16:10 +0000 Subject: [PATCH 6/8] Refactor: extract common progress pattern into reusable function Address Copilot feedback on code duplication: - Extract repeated progress display pattern into run_with_progress() function - Eliminates 40+ lines of duplicated code between pull and create_db operations - Fixes ERROR_LOG variable reuse issue with proper scoping - Improves maintainability and reduces bug risk - Each operation now uses independent ERROR_LOG variables This refactoring maintains identical functionality while significantly improving code quality and maintainability. --- setup.sh | 78 +++++++++++++++++++++++--------------------------------- 1 file changed, 32 insertions(+), 46 deletions(-) diff --git a/setup.sh b/setup.sh index f465f21f..09d377e1 100755 --- a/setup.sh +++ b/setup.sh @@ -21,6 +21,36 @@ handle_error() { exit 1 } +# Background process with progress dots +run_with_progress() { + local message="$1" + local command="$2" + + echo -n "$message" + ERROR_LOG=$(mktemp) + eval "$command" >/dev/null 2>"$ERROR_LOG" & + PID=$! + trap 'kill "$PID" 2>/dev/null || true; rm -f "$ERROR_LOG"' EXIT INT TERM + while kill -0 "$PID" 2>/dev/null; do + echo -n "." + sleep 2 + done + wait "$PID" + STATUS=$? + trap - EXIT INT TERM + if [ "$STATUS" -ne 0 ]; then + echo " Failed!" + if [ -s "$ERROR_LOG" ]; then + echo "Error details:" + cat "$ERROR_LOG" + fi + rm -f "$ERROR_LOG" + handle_error + fi + rm -f "$ERROR_LOG" + echo " Done!" +} + # Ensure the script is being run as root ID=$(id -u) if [ "0$ID" -ne 0 ]; then @@ -333,52 +363,8 @@ startup() { if [ "x$DEBUG" = "xyes" ]; then docker compose run --rm server create_db || handle_error else - echo -n "Downloading images" - ERROR_LOG=$(mktemp) - docker compose pull >/dev/null 2>"$ERROR_LOG" & - PID=$! - trap 'kill "$PID" 2>/dev/null || true; rm -f "$ERROR_LOG"' EXIT INT TERM - while kill -0 "$PID" 2>/dev/null; do - echo -n "." - sleep 2 - done - wait "$PID" - STATUS=$? - trap - EXIT INT TERM - if [ "$STATUS" -ne 0 ]; then - echo " Failed!" - if [ -s "$ERROR_LOG" ]; then - echo "Error details:" - cat "$ERROR_LOG" - fi - rm -f "$ERROR_LOG" - handle_error - fi - rm -f "$ERROR_LOG" - echo " Done!" - echo -n "Creating database" - ERROR_LOG=$(mktemp) - docker compose run --rm server create_db >/dev/null 2>"$ERROR_LOG" & - PID=$! - trap 'kill "$PID" 2>/dev/null || true; rm -f "$ERROR_LOG"' EXIT INT TERM - while kill -0 "$PID" 2>/dev/null; do - echo -n "." - sleep 2 - done - wait "$PID" - STATUS=$? - trap - EXIT INT TERM - if [ "$STATUS" -ne 0 ]; then - echo " Failed!" - if [ -s "$ERROR_LOG" ]; then - echo "Error details:" - cat "$ERROR_LOG" - fi - rm -f "$ERROR_LOG" - handle_error - fi - rm -f "$ERROR_LOG" - echo " Done!" + run_with_progress "Downloading images" "docker compose pull" + run_with_progress "Creating database" "docker compose run --rm server create_db" fi echo "** Starting the rest of Redash **" From 30ec6a54596f3b53514776d0dadd99b8df550a3a Mon Sep 17 00:00:00 2001 From: snickerjp Date: Wed, 31 Dec 2025 17:17:01 +0000 Subject: [PATCH 7/8] Fix help text order and script name reference issues Address remaining Copilot feedback: - Reorder help text to match usage line order for better readability - Capture script name at startup to handle symlinks and different invocations - Use SCRIPT_NAME variable in error messages and help text for consistency These improvements enhance user experience and ensure correct script references regardless of how the script is invoked. --- setup.sh | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/setup.sh b/setup.sh index 09d377e1..54eeb32a 100755 --- a/setup.sh +++ b/setup.sh @@ -9,13 +9,14 @@ OVERWRITE=no PREVIEW=no REDASH_VERSION="" DEBUG=no +SCRIPT_NAME="$0" # Error handling function handle_error() { if [ "x$DEBUG" != "xyes" ]; then echo echo "❌ An error occurred during installation." - echo "💡 For detailed output, please run: $0 --debug" + echo "💡 For detailed output, please run: $SCRIPT_NAME --debug" echo fi exit 1 @@ -98,12 +99,12 @@ while true; do shift 2 ;; -h | --help) - echo "Redash setup script usage: $0 [-d|--dont-start] [-p|--preview] [-g|--debug] [-o|--overwrite] [--version ]" + echo "Redash setup script usage: $SCRIPT_NAME [-d|--dont-start] [-p|--preview] [-g|--debug] [-o|--overwrite] [--version ]" + echo " The --dont-start (also -d) option installs Redash, but doesn't automatically start it afterwards" echo " The --preview (also -p) option uses the Redash 'preview' Docker image instead of the last stable release" echo " The --debug (also -g) option shows detailed Docker progress output instead of clean progress dots" - echo " The --version option installs the specified version tag of Redash (e.g., 10.1.0)" echo " The --overwrite (also -o) option replaces any existing configuration with a fresh new install" - echo " The --dont-start (also -d) option installs Redash, but doesn't automatically start it afterwards" + echo " The --version option installs the specified version tag of Redash (e.g., 10.1.0)" exit 1 ;; --) From cb80fcb9fc87fadeabbf78fecbc2b6fed3f11f17 Mon Sep 17 00:00:00 2001 From: snickerjp Date: Wed, 31 Dec 2025 17:24:52 +0000 Subject: [PATCH 8/8] Fix POSIX compliance: remove local keyword Address cubic-dev-ai feedback on POSIX compliance: - Remove 'local' keyword which is a bashism, not POSIX-compliant - Use unique variable names (RWP_MESSAGE, RWP_COMMAND) instead - Maintains compatibility with strict POSIX shells - Script continues to use #!/usr/bin/env sh as intended This ensures the script works on all POSIX-compliant shells, not just bash and dash. --- setup.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/setup.sh b/setup.sh index 54eeb32a..b9b6d0bf 100755 --- a/setup.sh +++ b/setup.sh @@ -24,12 +24,12 @@ handle_error() { # Background process with progress dots run_with_progress() { - local message="$1" - local command="$2" + RWP_MESSAGE="$1" + RWP_COMMAND="$2" - echo -n "$message" + echo -n "$RWP_MESSAGE" ERROR_LOG=$(mktemp) - eval "$command" >/dev/null 2>"$ERROR_LOG" & + eval "$RWP_COMMAND" >/dev/null 2>"$ERROR_LOG" & PID=$! trap 'kill "$PID" 2>/dev/null || true; rm -f "$ERROR_LOG"' EXIT INT TERM while kill -0 "$PID" 2>/dev/null; do