Ofer Schreiber has posted comments on this change. Change subject: packaging: Remote DB support - DO NOT MERGE!!!!. ......................................................................
Patch Set 7: Fails (9 inline comments) .................................................... File packaging/fedora/setup/common_utils.py Line 542: def getDbParam(param): Param is a very bad function/param name, please avoid using it as much as you can. Line 576: if host != "localhost" and host != "127.0.0.1": You're preforming this check multiple times. it's better to create a separate function, in case we will need to add a new "localhost" name/ip .................................................... File packaging/fedora/setup/engine-setup.py Line 34: remoteInstallation = False Why do you need this global? Line 981: def _upgradeDB(): you probably want to move some of the code here to utils, so engine-upgrade will use the same code. Line 1830: if getHostName() != "localhost" and getHostName() != "127.0.0.1": looks wrong to me to have this condition inside a function name "restartPostgresql"... Line 1832: pass either way, you probably want to use "return" and not "pass" .................................................... File packaging/fedora/setup/engine-upgrade.py Line 394: class DB(): Where is all the DB rename flow during upgrade/rollback? .................................................... File packaging/fedora/setup/engine_validators.py Line 125: return False ? .................................................... File packaging/fedora/setup/output_messages.py Line 125: INFO_CONF_PARAMS_USE_DB_HOST_USAGE="Please enter the host IP or host name where DB is running. Use 'localhost' for default local installation" This is the USAGE, not prompt. No need to use "please" so much, it looks bad. -- To view, visit http://gerrit.ovirt.org/2245 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iab66d6f8ffe33f9674e79753df7541c212012190 Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alex Lourie <[email protected]> Gerrit-Reviewer: Alex Lourie <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Eyal Edri <[email protected]> Gerrit-Reviewer: Ofer Schreiber <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
