Refactor Bash Scripts for Enhanced Readability and Robustness #60

Adriel007 · 2024-05-02T19:00:46Z

This pull request encompasses a series of commits aimed at improving the readability, portability, and robustness of various Bash scripts. Here's a summary of the changes:

Improved Variable Naming: Descriptive variable names were adopted for better understanding.
OS Detection with case Statements: case statements were employed for OS detection, enhancing script portability.
Consistent Output with printf: Replaced echo with printf for consistent output formatting.
Dependency Checks: Scripts now check for dependencies like wget before usage to prevent execution errors.
Enhanced Error Messages: Error messages were enhanced for better debugging and understanding failure cases.
Function Organization: Scripts were reorganized into functions for better maintainability.
Error Handling: Added error handling and informative messages for failure cases, improving script reliability.
Consistent Indentation and Formatting: Ensured consistent indentation and formatting for readability.
Variable Quoting: Quoted variables to prevent word splitting and pathname expansion.
Comments and Documentation: Added comments and updated usage messages for clarity and understanding of the scripts' purpose and individual sections.

These changes aim to provide more robust, maintainable, and understandable solutions for various tasks including Kubeflow installation, KubeRay installation, Minio setup, NGINX installation and management, Kind cluster management, and NGINX Ingress resource management.

- Use descriptive variable names - Employ `case` statement for OS detection - Utilize `printf` over `echo` for consistent output - Check for `wget` dependency before usage - Ensure consistent variable quoting - Enhance error messages for better debugging Signed-off-by: Adriel007 <adrielsouzaandrade8@gmail.com>

Signed-off-by: Adriel007 <adrielsouzaandrade8@gmail.com>

- Improved readability and robustness of the script by: - Adding `set -euo pipefail` for error handling - Marking `ROOT_DIR` variable as readonly - Renaming variables and functions for clarity - Adding error handling for missing environment variables and resource management issues - Consistent indentation and formatting for better readability - Encapsulated main logic within a `main` function Signed-off-by: Adriel007 <adrielsouzaandrade8@gmail.com>

This commit improves the readability, robustness, and error handling of the Bash script responsible for managing Kind clusters. Changes include consistent quoting, informative error messages for missing dependencies, enhanced usage message, simplified checks for tool presence, and added verification for port availability before cluster creation. Signed-off-by: Adriel007 <adrielsouzaandrade8@gmail.com>

…tup script Signed-off-by: Adriel007 <adrielsouzaandrade8@gmail.com>

- Improved readability by consistent indentation and formatting. - Used more descriptive variable and function names. - Enhanced error handling with informative messages. - Added comments to clarify the purpose of different sections. - Quoted variables to prevent word splitting and pathname expansion. - Updated usage message for better clarity and consistency. Signed-off-by: Adriel007 <adrielsouzaandrade8@gmail.com>

…g Minio instances in Kubernetes. The changes include consistent indentation, error handling, quoting variables, clearer function names, and enhanced usage instructions. These enhancements improve readability, maintainability, and robustness of the script. Signed-off-by: Adriel007 <adrielsouzaandrade8@gmail.com>

- Improved the Bash script responsible for KubeRay installation to enhance readability, error handling, and adherence to best practices. - Implemented consistent indentation, double quoting of variables, and error handling for critical operations to ensure script reliability. - Simplified conditionals and effectively utilized functions to encapsulate logical units of work. - Added comments for better clarity and understanding of the script's purpose and individual sections. - Overall, the script now provides a more robust and maintainable solution for deploying KubeRay in Kubernetes environments. Signed-off-by: Adriel007 <adrielsouzaandrade8@gmail.com>

- Reorganized script into functions for better readability and maintainability. - Added error handling and informative messages for failure cases. - Ensured consistent indentation and improved variable naming. - Updated comments for clarity and added usage function. Signed-off-by: Adriel007 <adrielsouzaandrade8@gmail.com>

- Fixed the KubeRay deployment script to handle reusing names. - Added English documentation to the script. - Updated the script to use appropriate echo messages. Signed-off-by: Adriel007 <adrielsouzaandrade8@gmail.com>

daw3rd · 2024-05-03T13:28:13Z

transforms/code/malware/install_clamd_mac.sh

-sudo chown $(id -u):$(id -g) /var/lib/clamav
-
-# Update a database of ClamAV
+# Copy and edit clamd.conf


This all looks very good. However, when I run this on a Mac M1, it fails make test-src. If i use the original version it does not fail. I tried adding back the \cp in place of cp, but no help.

make clean venv test-src ... Warning: clamav 1.3.1 is already installed and up-to-date. To reinstall 1.3.1, run: brew reinstall clamav ClamAV update process started at Fri May 3 09:24:24 2024 daily.cld database is up-to-date (version: 27264, sigs: 2060601, f-level: 90, builder: raynman) main.cvd database is up-to-date (version: 62, sigs: 6647427, f-level: 90, builder: sigmgr) bytecode.cvd database is up-to-date (version: 335, sigs: 86, f-level: 90, builder: raynman) source venv/bin/activate; \ export PYTHONPATH=../src; \ cd test; pytest -s . ERROR: Please define server type (local and/or TCP). ========================================================================= test session starts ========================================================================= platform darwin -- Python 3.10.11, pytest-8.2.0, pluggy-1.5.0 rootdir: /Users/dawood/git/ariel007/data-prep-lab/transforms/code/malware/test plugins: anyio-4.3.0 collecting ... 09:24:28 INFO - Clamd process is not running. Start clamd process. ERROR: Please define server type (local and/or TCP). collected 1 item / 1 error

I will fix this

daw3rd · 2024-05-03T13:33:11Z

transforms/code/malware/install_clamd_mac.sh

-sed -i '' -e 's/^Example/# Example/' $(brew --prefix)/etc/clamav/freshclam.conf
-echo "DatabaseDirectory /var/lib/clamav" >> $(brew --prefix)/etc/clamav/freshclam.conf
-
-# Create a directory for a local unix socket


Can you restore this and the next comment

roytman

Hi @Adriel007 , welcome, and thank you for your PR.

See several comments.

roytman · 2024-05-04T18:54:26Z

kind/hack/tools/install_kuberay.sh

-esac
-
+    cleanup)
+        echo "Uninstalling KubeRay..."


Please replace echo by header_text

roytman · 2024-05-04T18:56:41Z

kind/hack/tools/install_kuberay.sh

+    deploy-wait)
+        echo "Deploying and waiting for KubeRay..."
+        deploy
+        wait_for_kuberay


the deployment fails here, with the message:

Error: INSTALLATION FAILED: cannot re-use a name that is still in use
Error: Failed to install kuberay-operator.

in order to reduce installation time, the making process deploys everything and, after that, waits for one by another.
Please remove L57 (deploy) from the deploy-wait operation.

roytman · 2024-05-04T18:57:30Z

kind/hack/tools/install_nginx.sh

+        ;;
+    deploy-wait)
+        header_text "Deploying and waiting for NGINX"
+        deploy_nginx


please remove L42

roytman · 2024-05-04T19:05:07Z

kind/hack/tools/install_kuberay.sh

+# Function to wait for KubeRay deployment
+wait_for_kuberay() {
+    echo "Waiting for KubeRay deployment..."
+    wait_for_pods "kuberay" "$MAX_RETRIES" "$SLEEP_TIME" || { echo "Error: KubeRay deployment unsuccessful. Not all pods running."; exit 1; }


The command fails here:
./install_kuberay.sh: line 33: wait_for_pods: command not found,

you removed source ../common.sh, please return it

roytman · 2024-05-04T19:06:42Z

kind/hack/tools/install_kuberay.sh


+# Define functions


do we need this line? looks like it duplicates L12

roytman · 2024-05-04T19:22:37Z

kind/hack/tools/ingress.sh

-if [[ "${DEPLOY_KUBEFLOW}" -eq 1 ]]; then
-	kubectl apply -f ${ROOT_DIR}/hack/kfp_ingress.yaml
-fi
+source "${ROOT_DIR}/hack/common.sh"


the path is wrong,

./ingress.sh: line 7: /path/to/root/dir/hack/common.sh: No such file or directory

roytman · 2024-05-04T19:26:07Z

kind/hack/populate_minio.sh

+mc alias set "$minio_alias" "$minio_url" "$minio_access_key" "$minio_secret_key"
+
+# Create test buckets
+echo "Creating test buckets"


we don't need a separate bucket for each tested transformer. The "folder" prefix is enough.

roytman · 2024-05-04T19:27:41Z

kind/hack/populate_minio.sh

+# Function to copy data
+copy_data() {
+    local module="$1"
+    local src="$2"


this argument is not used

roytman · 2024-05-04T19:34:52Z

kind/hack/tools/ingress.sh


-source ${ROOT_DIR}/hack/common.sh
+readonly ROOT_DIR="/path/to/root/dir"


what is /path/to/root/dir ?

Download helper-functions.sh

Adriel007 force-pushed the dev branch from 65fd434 to 39e80c8 Compare May 2, 2024 19:10

Adriel007 added 10 commits May 2, 2024 16:29

Refactor ClamAV setup script for readability and robustness

752522c

Signed-off-by: Adriel007 <adrielsouzaandrade8@gmail.com>

Refactor data copying process and improve readability in the Minio se…

2cbbeec

…tup script Signed-off-by: Adriel007 <adrielsouzaandrade8@gmail.com>

Fix KubeRay deployment script and add English documentation

e54a80a

- Fixed the KubeRay deployment script to handle reusing names. - Added English documentation to the script. - Updated the script to use appropriate echo messages. Signed-off-by: Adriel007 <adrielsouzaandrade8@gmail.com>

Adriel007 force-pushed the dev branch from d0d58e9 to e54a80a Compare May 2, 2024 19:30

daw3rd requested changes May 3, 2024

View reviewed changes

roytman requested changes May 4, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Bash Scripts for Enhanced Readability and Robustness #60

Refactor Bash Scripts for Enhanced Readability and Robustness #60

Adriel007 commented May 2, 2024

daw3rd May 3, 2024 •

edited

Adriel007 May 3, 2024

daw3rd May 3, 2024

roytman left a comment

roytman May 4, 2024

roytman May 4, 2024

roytman May 4, 2024

roytman May 4, 2024

roytman May 4, 2024

roytman May 4, 2024

roytman May 4, 2024

roytman May 4, 2024

roytman May 4, 2024


		source ${ROOT_DIR}/hack/common.sh
		readonly ROOT_DIR="/path/to/root/dir"

Refactor Bash Scripts for Enhanced Readability and Robustness #60

Are you sure you want to change the base?

Refactor Bash Scripts for Enhanced Readability and Robustness #60

Conversation

Adriel007 commented May 2, 2024

daw3rd May 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roytman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daw3rd May 3, 2024 •

edited