Yedidyah Bar David has posted comments on this change. Change subject: packaging: engine-backup: support dwh and reports ......................................................................
Patch Set 1: (5 comments) http://gerrit.ovirt.org/#/c/25850/1/packaging/bin/engine-backup.sh File packaging/bin/engine-backup.sh: Line 28: # Load jasper reports db credentials Line 29: get_jasper_db_cred() { Line 30: local key="$1" Line 31: Line 32: python -c " > this will not work if default python is python3... please make sure you wri Already talked with Sandro about that, he said we currently do not support python3 and intend to replace this code anyway. Line 33: import ConfigParser Line 34: import io Line 35: import os Line 36: Line 45: ) Line 46: ) Line 47: Line 48: print config.get('default', '${key}') Line 49: " > why not provider "$@" as parameter and print Done Line 50: } Line 51: Line 52: load_jasper_reports_db_creds() { Line 53: REPORTS_DB_HOST="$(get_jasper_db_cred 'dbHost')" Line 52: load_jasper_reports_db_creds() { Line 53: REPORTS_DB_HOST="$(get_jasper_db_cred 'dbHost')" Line 54: REPORTS_DB_PORT="$(get_jasper_db_cred 'dbPort')" Line 55: REPORTS_DB_USER="$(get_jasper_db_cred 'dbUsername')" Line 56: REPORTS_DB_PASSWORD="$(get_jasper_db_cred 'dbPassword')" > unescape? Sorry, I did not understand if that's needed or not, and if it's different in 3.3 and 3.4. I currently intend to make the code easily-backported to 3.3. I'll test this later with various passwords and fix. Line 57: REPORTS_DB_DATABASE="$(get_jasper_db_cred 'js.dbName')" Line 58: } Line 59: Line 60: JASPER_PROPERTIES=/var/lib/ovirt-engine-reports/build-conf/master.properties Line 66: Line 67: # Globals Line 68: BACKUP_PATHS="/etc/ovirt-engine Line 69: /etc/ovirt-engine-dwh Line 70: /etc/ovirt-engine-reports > but how do you handle different applications, I thought there will be: We'll make this clean and nice when we rewrite in python... Line 71: /etc/pki/ovirt-engine Line 72: /etc/ovirt-engine-setup.conf.d Line 73: /var/lib/ovirt-engine-reports/build-conf Line 74: /etc/httpd/conf.d/ovirt-engine-root-redirect.conf Line 261: --db-user=*) Line 262: MY_DB_USER="${v}" Line 263: ;; Line 264: --db-passfile=*) Line 265: DB_PASSFILE="${v}" > Isn't better to call it MY_DB_PASSFILE just for homogeneity? It's local to this function, just used to quickly set MY*DB_PASSWORD. Not sure we really need three different local variables. Line 266: [ -r "${DB_PASSFILE}" ] || \ Line 267: die "Can not read password file ${DB_PASSFILE}" Line 268: read -r MY_DB_PASSWORD < "${DB_PASSFILE}" Line 269: ;; -- To view, visit http://gerrit.ovirt.org/25850 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I188a1823686b211fefb18ceb41e1a80afd9c5de5 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yedidyah Bar David <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Sandro Bonazzola <[email protected]> Gerrit-Reviewer: Simone Tiraboschi <[email protected]> Gerrit-Reviewer: Yaniv Dary <[email protected]> Gerrit-Reviewer: Yedidyah Bar David <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
