Alon Bar-Lev has posted comments on this change.
Change subject: packaging: Allow restore to different db location
......................................................................
Patch Set 7:
(4 comments)
....................................................
File packaging/bin/engine-backup.sh
Line 122: MY_DB_USER="${v}"
Line 123: ;;
Line 124: --db-passfile=*)
Line 125: DB_PASSFILE="${v}"
Line 126: if ! [ -r "${DB_PASSFILE}" ]; then
maybe:
[ -r xxx ] || die ?
Line 127: die "Can not read password file
${DB_PASSFILE}"
Line 128: fi
Line 129: read MY_DB_PASSWORD < "${DB_PASSFILE}"
Line 130: ;;
Line 266: restoreFiles "${BACKUP_PATHS}"
Line 267: fi
Line 268:
Line 269: log "Reloading configuration"
Line 270: [ -n "${CHANGE_DB_CREDENTIALS}" ] && changeDBConf
only if success if entire process?
Line 271: load_config
Line 272: log "Generating pgpass"
Line 273: generatePgPass # Must run after configuration reload
Line 274: log "Verifying connection"
Line 328:
Line 329: setMyDBCredentials() {
Line 330: local options
Line 331: [ "${MY_DB_SECURED}" = "True" ] && \
Line 332: options="${options}ssl=true"
please always append with spearator (&), then remove the first separator per my
previous comment.
this will avoid someone add new option before the first and break
configuration, or second be added without first result in having first
separator.
I understand that in this local use case we can have some local assumptions,
but better write generic.
Line 333: [ "${MY_DB_SECURED_VALIDATION}" != "True" ] && \
Line 334:
options="${options}&sslfactory=org.postgresql.ssl.NonValidatingFactory"
Line 335:
Line 336: MY_DB_CREDS="ENGINE_DB_HOST=${MY_DB_HOST}
Line 342: ENGINE_DB_SECURED_VALIDATION=${MY_DB_SECURED_VALIDATION}
Line 343: ENGINE_DB_DRIVER=org.postgresql.Driver
Line 344:
ENGINE_DB_URL=jdbc:postgresql://\${ENGINE_DB_HOST}:\${ENGINE_DB_PORT}/\${ENGINE_DB_DATABASE}?${options}
Line 345: "
Line 346: eval "${MY_DB_CREDS}"
well, this is somewhat more complex that the flow I imagine....
if use_existing_credentials
read from file into vars
else
set vars
fi
<snip>
if success and not use_existing_credentials
write vars to file (1:1 mapping)
# this can be done also if not use_existing_credentials as nothing
actually changes...
Line 347: }
Line 348:
Line 349: changeDBConf() {
Line 350: local conf="${ENGINE_ETC}/engine.conf.d/10-setup-database.conf"
--
To view, visit http://gerrit.ovirt.org/20258
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2c84111808c638a1c589cf3c14c271184a0a7c2
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yedidyah Bar David <d...@redhat.com>
Gerrit-Reviewer: Alex Lourie <alou...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com>
Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com>
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
