Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable local deployment to improve development and testing #484

Merged
merged 21 commits into from
Dec 19, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
enable log redirects
mishaschwartz committed Nov 25, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit 9f64eb80057b2fe65efbf91daf11a1f99e4cbddc
26 changes: 24 additions & 2 deletions bin/birdhouse
Original file line number Diff line number Diff line change
@@ -7,7 +7,7 @@ COMPOSE_DIR="$(dirname "${THIS_DIR}")/birdhouse"
export BIRDHOUSE_COMPOSE="${BIRDHOUSE_COMPOSE:-"${COMPOSE_DIR}/birdhouse-compose.sh"}"
export __BIRDHOUSE_SUPPORTED_INTERFACE=True

USAGE="USAGE: $0 [-h|--help] [-b|--backwards-compatible] [-e|--env-file local-env-file] {info|compose|configs}"
USAGE="USAGE: $0 [-h|--help] [-b|--backwards-compatible] [-e|--env-file local-env-file] {[--log-stdout] | [--log-file log-file-path]} {info|compose|configs}"
HELP="$USAGE

Manage the Birdhouse software stack.
@@ -21,6 +21,8 @@ Options:
-h, --help Print this message and exit
-b, --backwards-compatible Run in backwards compatible mode
-e, --env-file string Override the local environment file, default is ${COMPOSE_DIR}/env.local
--log-stdout Write logs to stdout, default is to write to stderr
--log-file string Write logs to this file path, default is to write to stderr
"

CONFIGS_USAGE="USAGE: $0 configs [-h|--help] [-d|--default] {[-p|--print-config-command] | [-c|--command command]}"
@@ -194,6 +196,26 @@ parse_args() {
shift
parse_args "$@"
;;
--log-stdout)
shift
[ "${LOG_FILE}" = 'True' ] && parse_args 'invalid arg that triggers usage message'
export BIRDHOUSE_LOG_FD=1 # The argument here takes precedence over the env variable
LOG_STDOUT=True
parse_args "$@"
;;
--log-file=*)
arg_value="${1#*=}"
shift
parse_args --log-file "${arg_value}" "$@"
;;
--log-file)
shift
[ "${LOG_STDOUT}" = 'True' ] && parse_args 'invalid arg that triggers usage message'
export BIRDHOUSE_LOG_FILE=$(realpath "$1") # The argument here takes precedence over the env variable
LOG_FILE=True
shift
parse_args "$@"
;;
info)
shift
"${BIRDHOUSE_COMPOSE}" info "$@"
@@ -212,7 +234,7 @@ parse_args() {
echo "$HELP"
;;
-??*)
parse_multiple_short_flags parse_configs_args "$@"
parse_multiple_short_flags parse_args "$@"
;;
*)
>&2 echo "$USAGE"
4 changes: 2 additions & 2 deletions birdhouse/birdhouse-compose.sh
Original file line number Diff line number Diff line change
@@ -105,8 +105,8 @@ fi

create_compose_conf_list # this sets COMPOSE_CONF_LIST
log INFO "Displaying resolved compose configurations:"
echo "COMPOSE_CONF_LIST="
echo ${COMPOSE_CONF_LIST} | tr ' ' '\n' | grep -v '^-f'
log INFO "COMPOSE_CONF_LIST="
log INFO ${COMPOSE_CONF_LIST} | tr ' ' '\n' | grep -v '^-f'

if [ x"$1" = x"info" ]; then
log INFO "Stopping before execution of docker-compose command."
6 changes: 3 additions & 3 deletions birdhouse/read-configs.include.sh
Original file line number Diff line number Diff line change
@@ -103,7 +103,7 @@ read_default_env() {

. "${COMPOSE_DIR}/default.env"
else
log WARN "'${COMPOSE_DIR}/default.env' not found" 1>&2
log WARN "'${COMPOSE_DIR}/default.env' not found"
fi
}

@@ -123,7 +123,7 @@ read_env_local() {
eval "${saved_shell_options}"

else
log WARN "'${BIRDHOUSE_LOCAL_ENV}' not found" 1>&2
log WARN "'${BIRDHOUSE_LOCAL_ENV}' not found"
fi

}
@@ -155,7 +155,7 @@ source_conf_files() {
# corresponding PR are merged and old component names can be removed
# after the corresponding PR are merge without any impact on the
# autodeploy process.
log WARN "'${adir}' in ${conf_locations} does not exist" 1>&2
log WARN "'${adir}' in ${conf_locations} does not exist"
fi
if [ -f "${adir}/default.env" ]; then
# Source config settings of dependencies first if they haven't been sourced previously.
2 changes: 1 addition & 1 deletion birdhouse/scripts/clear-running-wps-jobs-in-db.sh
Original file line number Diff line number Diff line change
@@ -11,7 +11,7 @@ fi
# eg: DB_NAME=finch
DB_NAME="$1"
if [ -z "$DB_NAME" ]; then
log ERROR "please provide a database name, ex: finch" 1>&2
log ERROR "please provide a database name, ex: finch"
exit 2
fi
shift
89 changes: 58 additions & 31 deletions birdhouse/scripts/logging.include.sh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the changes are entirely backward compatible.

Before, the 1>&2 redirects were done for specific log levels (WARN and above).
This allowed logging outputs to stdout or some file, and only potentially problematic messages to another file. It seems this distinction is not possible anymore, since all goes either to stdout, stderr or the logfile.

Also, if we want both stdout/logfile, we need to ignore both options the script provides, and do birdhouse [...] | tee logfile ourselves. It seems like specifying both options should do the same as tee using both outputs simultaneously.

Copy link
Collaborator Author

@mishaschwartz mishaschwartz Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before, the 1>&2 redirects were done for specific log levels (WARN and above).

This is not exactly true. Some logs at WARN level and above were redirected to stderr but others weren't. The previous approach was inconsistent.

This allowed logging outputs to stdout or some file, and only potentially problematic messages to another file.

This can be implemented but it should be done in logging.include.sh, not on an ad hoc basis in the files that call the logger. I'm happy to implement this functionality if it's useful.

It seems like specifying both options should do the same as tee using both outputs simultaneously.

Sure, we can make that work

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If some >WARN logs were not redirected, those were unintended bugs.

I agree it should be implemented in logging.include.sh.

Which levels to log under with std/file output could be controlled by different CLI options, so it doesn't use any hardcoded redirects.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes are now fully backwards compatible as of e655404

Original file line number Diff line number Diff line change
@@ -14,48 +14,75 @@ if [ "${BIRDHOUSE_COLOR}" -eq "1" ]; then
fi

BIRDHOUSE_LOG_LEVEL=${BIRDHOUSE_LOG_LEVEL:-INFO}
if [ "${__BIRDHOUSE_SUPPORTED_INTERFACE}" = 'True' ]; then
__DEFAULT_BIRDHOUSE_LOG_FD=2
else
# logs were previously written to stdout
# (this supports backwards compatible scripts that don't use the interface)
__DEFAULT_BIRDHOUSE_LOG_FD=1
fi
export LOG_DEBUG="${GRAY}DEBUG${NORMAL}: "
export LOG_INFO="${BLUE}INFO${NORMAL}: "
export LOG_WARN="${YELLOW}WARNING${NORMAL}: "
export LOG_ERROR="${RED}ERROR${NORMAL}: "
export LOG_CRITICAL="${REG_BG_BOLD}CRITICAL${NORMAL}: " # to report misuse of functions

if [ -n "${BIRDHOUSE_LOG_FD}" ]; then
if [ -z "${BIRDHOUSE_LOG_FD##*[!0-9]*}" ]; then
echo "${LOG_CRITICAL}Invalid log file descriptor setting (not an integer): [BIRDHOUSE_LOG_FD=${BIRDHOUSE_LOG_FD}]."
exit 2
fi
fi


log_dest() {
read log_line
if [ -n "${log_line}" ]; then
if [ -n "${BIRDHOUSE_LOG_FILE}" ]; then
echo "$log_line" >> "${BIRDHOUSE_LOG_FILE}"
else
echo "$log_line" 1>&"${BIRDHOUSE_LOG_FD:-${__DEFAULT_BIRDHOUSE_LOG_FD}}"
fi
fi
}

# Usage: log {LEVEL} "{message}" [...]
# Any amount of messages can be passed to the function.
log() {
if [ "${BIRDHOUSE_LOG_LEVEL}" != DEBUG ] \
&& [ "${BIRDHOUSE_LOG_LEVEL}" != INFO ] \
&& [ "${BIRDHOUSE_LOG_LEVEL}" != WARN ] \
&& [ "${BIRDHOUSE_LOG_LEVEL}" != ERROR ]; then
echo "${LOG_CRITICAL}Invalid log level setting: [BIRDHOUSE_LOG_LEVEL=${BIRDHOUSE_LOG_LEVEL}]."
exit 2
fi
level="$1"
shift
if [ "$*" = "" ]; then
echo "${LOG_CRITICAL}Invalid log message is missing."
exit 2
fi
if [ "${level}" = "DEBUG" ]; then
if [ "${BIRDHOUSE_LOG_LEVEL}" = DEBUG ]; then
echo "${LOG_DEBUG}$*"
{
if [ "${BIRDHOUSE_LOG_LEVEL}" != DEBUG ] \
&& [ "${BIRDHOUSE_LOG_LEVEL}" != INFO ] \
&& [ "${BIRDHOUSE_LOG_LEVEL}" != WARN ] \
&& [ "${BIRDHOUSE_LOG_LEVEL}" != ERROR ]; then
echo "${LOG_CRITICAL}Invalid log level setting: [BIRDHOUSE_LOG_LEVEL=${BIRDHOUSE_LOG_LEVEL}]."
exit 2
fi
elif [ "${level}" = "INFO" ]; then
if [ "${BIRDHOUSE_LOG_LEVEL}" = DEBUG ] \
|| [ "${BIRDHOUSE_LOG_LEVEL}" = INFO ]; then
echo "${LOG_INFO}$*"
level="$1"
shift
if [ "$*" = "" ]; then
echo "${LOG_CRITICAL}Invalid log message is missing."
exit 2
fi
elif [ "${level}" = "WARN" ]; then
if [ "${BIRDHOUSE_LOG_LEVEL}" = DEBUG ] \
|| [ "${BIRDHOUSE_LOG_LEVEL}" = INFO ] \
|| [ "${BIRDHOUSE_LOG_LEVEL}" = WARN ]; then
echo "${LOG_WARN}$*"
if [ "${level}" = "DEBUG" ]; then
if [ "${BIRDHOUSE_LOG_LEVEL}" = DEBUG ]; then
echo "${LOG_DEBUG}$*"
fi
elif [ "${level}" = "INFO" ]; then
if [ "${BIRDHOUSE_LOG_LEVEL}" = DEBUG ] \
|| [ "${BIRDHOUSE_LOG_LEVEL}" = INFO ]; then
echo "${LOG_INFO}$*"
fi
elif [ "${level}" = "WARN" ]; then
if [ "${BIRDHOUSE_LOG_LEVEL}" = DEBUG ] \
|| [ "${BIRDHOUSE_LOG_LEVEL}" = INFO ] \
|| [ "${BIRDHOUSE_LOG_LEVEL}" = WARN ]; then
echo "${LOG_WARN}$*"
fi
elif [ "${level}" = "ERROR" ]; then
echo "${LOG_ERROR}$*"
else
echo "${LOG_CRITICAL}Invalid log level: [${level}]"
exit 2
fi
elif [ "${level}" = "ERROR" ]; then
echo "${LOG_ERROR}$*"
else
echo "${LOG_CRITICAL}Invalid log level: [${level}]"
exit 2
fi
} | log_dest
}
2 changes: 1 addition & 1 deletion birdhouse/scripts/sync-data
Original file line number Diff line number Diff line change
@@ -28,7 +28,7 @@ SOURCE_HOST="$1"; shift
FORCE_MODE="$1"

if [ -z "$SOURCE_HOST" ]; then
log ERROR "no source host provided" 1>&2
log ERROR "no source host provided"
exit 2
fi