# Revised media ingest script

link to original question: Simple media ingest script

As per answers I have refactored my code and would like to get feedback on the current structure.

The purpose of this tool is to allow someone to copy media from cards (SD cards, CFast, etc) using rsync and to also allow someone to complete incomplete transfer that they have initiated.

I've chosen to use prompts because it's more user friendly for the environment the script will be used in. The intended users are not command line tool friendly.

I'm not very familiar with bash and am quite out of practice with code in general so I would like to check behind myself. I received excellent feedback in the previous post and have addressed several of the points raised.

#!/usr/local/bin/bash
set -e
set -u

# prompt user for input
prompt_user(){
while [[ -z "${!2:-}" ]] do read -r -p "$1" "$2" done } # prompt user for additional rsync options add_rsync_options(){ while true; do read -r -p "Add rsync options? [y/n] " add_options case$add_options in
[Yy]* )
read -r -p $'Enter additional rsync options:\n' rsync_options break;; [Nn]* ) break;; *) echo "Please enter y or no!" esac done } # make target directory for transfer make_directory(){ echo$'Follow the prompt to create a project directory.\n'

prompt_user $'Path of target directory?\n' target_directory prompt_user$'Brand Prefix?\n' brand_prefix
prompt_user $'Project Name?\n' project_name prompt_user$'Media Type?\n' media_type
prompt_user $'Location?\n' location prompt_user$'Employee?\n' employee

destination_path=${target_directory}/$(date +'%Y%m%d')_${brand_prefix}_${project_name}_${media_type}_${location}_${employee} echo "Creating directory:${destination_path}"

mkdir -p "${destination_path}" "${destination_path}/logs"
}

# run rsync command
run_rsync(){
echo $'Follow the prompt to complete the rsync command.\n' prompt_user$'Path to source media?\n' source_path

# if partial ingest indicate pre-existing target directory
if [[ "$option" == "2" ]]; then prompt_user$'Target directory?\n' target_directory
destination_path=$target_directory fi add_rsync_options echo$'Running rsync command:\n'

rsync -r --info=progress2 --log-file="${destination_path}/logs/$(date +'%Y%m%d')_transfer_log.txt" ${rsync_options:-} "${source_path}/" "${destination_path}" } # clear terminal for readability clear # start while true do # read user input and run appropriate functions read -r -p$'Enter [1] to start an ingest or [2] to complete a partial ingest.\n' option
case $option in 1 ) make_directory run_rsync break;; 2 ) run_rsync break;; * ) echo$'Please enter a valid option!\n';;
esac
done


Error handling is at a minimum still I believe and I'm not too sure what would even be relevant to handle past what's implemented. I'm not very experienced in that regard so resources are appreciated!

Thanks!

### What is $option in run_rsync? option is read in the global scope to decide if user wants a full ingest or continue a partial ingest. In that loop that reads the prompt, the meaning of option is understandable. Referring to $option in run_rsync is just too far away. It's not clear anymore where it comes from and what it means. It would be better if the prompt loop passed the decision to run_rsync as a parameter, to make it perfectly clear. For example:

while true
do
read -r -p $'Enter [1] to start an ingest or [2] to complete a partial ingest.\n' option case$option in
1 )
make_directory
run_rsync full
break;;
2 )
run_rsync partial
break;;
* )
echo $'Please enter a valid option!\n';; esac done  And then in run_rsync: run_rsync() { local ingestType=$1

echo $'Follow the prompt to complete the rsync command.\n' prompt_user$'Path to source media?\n' source_path

if [[ "$ingestType" = "partial" ]]; then prompt_user$'Target directory?\n' target_directory
destination_path=\$target_directory
fi

# ...


### Inconsistent terminology

In make_directory, the term "target directory" refers to the base directory in which a timestamped sub-directory will be created, with brand name, project name, and so on, also in the name.

In run_rsync, in case of partial ingest the user is prompted for "target directory", but here it will mean the full destination path. If would be better to name it as such.

### Inconsistent formatting

The code uses inconsistent indentation. For example the content of prompt_user is over-indented, compared to other functions. The body of the while loop in the global scope is over-indented, and also haphazardly indented. It would be easier to read if indenting was consistent throughout.

Most comments in the program state the obvious. They are just noise, and it would be better to remove them.

• Excellent points! Thanks for the detailed response. Passing the ingest type to the function makes a lot of sense and did not occur to me at all. – usulmuaddib Mar 28 at 13:20