Alon Bar-Lev has posted comments on this change.

Change subject: packaging: engine-backup: support dwh and reports
......................................................................


Patch Set 1:

(6 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 write 
code that is compatible to both.

see otopi::src/otopi/__init__.py
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

 k=v
 k=v

then eval the result instead of executing n times?
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?
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 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?
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:

 BACKUP_PATHS_<appname>="...

you can even source it based on app name...

 . ${xxxx}/ovirt-engine-backup-template.${app}
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 144:  --reports-db-password=pass         set reports database password
Line 145:  --reports-db-password              set reports database password - 
interactively
Line 146:  --reports-db-name=name             set reports database name
Line 147:  --reports-db-secured               set a secured connection for 
reports
Line 148:  --reports-db-secured-validation    validate host for reports
well... I do not like this, if you are to extend usage, I very much prefer 
utility per product that calls single script.
Line 149: 
Line 150:  ENVIRONMENT VARIABLES
Line 151: 
Line 152:  OVIRT_ENGINE_DATABASE_PASSWORD


-- 
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: 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

Reply via email to