2
\$\begingroup\$

This is a follow on to a previous question:

Part 3: User Scripts

All the code I am submitting here is available in github

The reason to write these tools is so I can prototype and quickly verify that I am doing things correctly before implementing them in C++. This is mainly necessary because the documentation these scripts are based on are relatively bad and confusing.

The scripts used by users:

s3cp

#!/usr/bin/env bash

badflag=0
declare -a ARGS

function usage {
    echo "Usage:"
    echo "s3cp  <file> <s3-url> [--key=<key>] [--secret=<secret>] [--verbose] [--dryrun]"
    echo "s3cp  <s3-url> <file> [--key=<key>] [--secret=<secret>] [--verbose] [--dryrun]"
    echo
    echo "s3-url:  s3://<bucket>/<Object-Path>"
    exit 1
}

verbose=''
dryrun=0
access=${AMZ_ACCESS_KEY}
secret=${AMZ_SECRET_KEY}
for var in "$@"; do
    if [[ "${var}" == '--verbose' ]]; then
        verbose='/dev/stdout'
        continue
    fi
    if [[ "${var}" == '--dryrun' ]]; then
        dryrun=1
        continue;
    fi
    if [[ "${var%=*}" == "--key" ]]; then
        access=${var#*=}
        continue;
    fi
    if [[ "${var%=*}" == "--secret" ]]; then
        secret=${var#*=}
        continue;
    fi
    flag=${var#-}
    if [[ "${flag}" != ${var} ]]; then
        badflag=1
        echo "Bad flag: ${var}"
        continue
    fi
    ARGS+=("$var")
done

if [[ "${access}" == "" || "${secret}" == "" ]]; then
    echo "Error: No Access or Secret provided."
    echo "These can be set in the environment variables AMZ_ACCESS_KEY/AMZ_SECRET_KEY or passed with flags"
    usage
fi

if [[ ${#ARGS[@]} != 2 || ${badflag} != 0 ]]; then
    usage
fi

src=${ARGS[0]}
dst=${ARGS[1]}

doUpload=0
doDownload=0
srcS3=${src#s3://}
if [[ "${src}" == "${srcS3}" ]]; then
    doUpload=1
fi
dstS3=${dst#s3://}
if [[ "${dst}" == "${dstS3}" ]]; then
    doDownload=1
fi

if [[ ${doUpload} == 0 && ${doDownload} == 0 ]]; then
    echo "Neither source or destination is on S3. Just use normal cp"
    usage
fi
if [[ ${doUpload} == 1 && ${doDownload} == 1 ]]; then
    echo "Both source or destination is on S3. Don't support move yet."
    usage
fi

dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
source ${dir}/s3/signature


if [[ ${doUpload} == 1 ]]; then
    source ${dir}/s3/upload
    upload "${src}" "${dst}" "${access}" "${secret}" '' "${verbose}" "${dryrun}"
else
    source ${dir}/s3/download
    download "${src}" "${dst}" "${access}" "${secret}" '' "${verbose}" "${dryrun}"
fi
\$\endgroup\$
2
\$\begingroup\$

In case of errors/warnings, write your output to stderr and not just echo them to stdout stream. This might also include you returning the usage section.

Having a chain of if block is imo, always very bad. You can use a switch-case block there, and keep shifting the arguments. A simple example would be:

ARGS=()
while [[ $# -gt 0 ]]
do
    key="$1"

    case $key in
      -v|--verbose)
        verbose='/dev/stdout'
        shift # past argument
        ;;
      --dryrun)
        dryrun=1
        shift # past argument
        ;;
      -k=*|--key=*)
        access="${key#*=}"
        shift # past argument
        shift # past value
        ;;
      -k|--key)
        shift # past argument
        access="$1"
        shift # past value
        ;;
      .
      .
      .
      *)    # unknown option
        if [[ "${key#-}" != ${key} ]]; then
            badflag=1
            >&2 echo "Bad flag: ${var}"
        fi
        ARGS+=("$1") # save it in an array for later
        shift # past argument
        ;;
    esac
done

Notice how easily you can maintain the --key=KEY and --key KEY format parameters. Apart from that, it is really easy to later on add another new argument to your utility.

Keep defining your globals in UPPER_CASE, so that it easy to distinguish them later in your ever-expanding code.

echo "These can be set in the environment variables AMZ_ACCESS_KEY/AMZ_SECRET_KEY or passed with flags"

AWS recommends having the environment variables named AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY. I would consider it breaking the norm if you are not respecting defaults.

Define the usage content (or man page?) as a heredoc:

USAGE=<<EOF

Usage:
s3cp  <file> <s3-url> [--key=<key>] [--secret=<secret>] [--verbose] [--dryrun]
s3cp  <s3-url> <file> [--key=<key>] [--secret=<secret>] [--verbose] [--dryrun]

s3-url:  s3://<bucket>/<Object-Path>

EOF
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.