Alon Bar-Lev has posted comments on this change.
Change subject: core: Provide a wrapper script with ovirt to...
......................................................................
Patch Set 4: (5 inline comments)
Hi Eli,
I think that while you do this, there is something much more important to do.
Currently you relay on the output of psql which is centered and separated by
'|', you parse the output skip headers etc... this instead of executing
statement as CSV and have standard format to work with.
So I suggest having one function in common utilities that is sourced, to
execute psql in CSV mode and use this all over.
Oh... Do I understand correctly that this script will be executed only manually?
....................................................
File backend/manager/tools/dbutils/engine-psql.sh
Line 1: #!/bin/bash
I suggest new scripts be written in POSIX sh, so we start the migration.
Line 2:
#################################################################################
Line 3: # The purpose of this script is to be a wrapper for support data
manipulations on the
Line 4: # database done up to now by psql.
Line 5: # The script supports all original psql flags and records the executed
SQL and its
Line 16: exit 0
Line 17: }
Line 18:
Line 19: permission_error() {
Line 20: echo -e "Insufficient permissions on $LOGDIR, \n Please execute
again with a user that has write accees to this directory or use sudo"
please do not use echo -e, non standard.
cat << __EOF__
bla bla
bla bla
__EOF__
or:
echo "bla bla"
echo "bla bla"
Line 21: exit 4
Line 22: }
Line 23:
Line 24: # Check for all available command switches but change only -c and -f
behaviour
Line 21: exit 4
Line 22: }
Line 23:
Line 24: # Check for all available command switches but change only -c and -f
behaviour
Line 25: while getopts c:d:f:v:p:U:h:hlvX1aeELnoqsSAFHPRtTxz0wW option; do
if you delegate everything to psql, why check syntax?
Line 26: case $option in
Line 27: c) COMMAND=$OPTARG;;
Line 28: f) FILE=$OPTARG;;
Line 29: h) ret=0 && usage;;
Line 30: \?) ret=1 && usage;;
Line 31: esac
Line 32: done
Line 33:
Line 34: LOGDIR="/var/log/ovirt-engine"
you cannot have hard coded files any more.
please produce a vars.sh script that is generated during makefile and include
it.
See:
./packaging/bin/engine-prolog.sh.in
Usage:
./packaging/bin/engine-config.sh
Line 35: if [ ! -d "${LOGDIR}" ]; then
Line 36: echo "Can not find log directory ${LOGDIR}"
Line 37: exit 1
Line 38: fi
Line 58: permission_error
Line 59: fi
Line 60: fi
Line 61:
Line 62: psql ${PARAMS} &>> ${LOGFILE}
if you want to pass all parameters just use "$@", see example in:
./xxx/share/ovirt-engine/bin/engine-config.sh
of how to go over existing parameters and still delegate all, then you do not
need to parse anything nor create a file.
Line 63: if [ $? -ne 0 ]; then
Line 64: echo "$(date): Failed to execute command ${0} $*" >> ${LOGFILE}
Line 65: exit 2
--
To view, visit http://gerrit.ovirt.org/16273
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I468f830196bd95dc013a5142f9aa0d508e687d90
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eli Mesika <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
