Ofer Schreiber has posted comments on this change. Change subject: packaging: remoteDB support ......................................................................
Patch Set 8: Fails (7 inline comments) .................................................... File backend/manager/dbscripts/engine-db-install.sh Line 103: if [[ "$DB_HOST" == "$addr" ]] What will happen if you get hostname instead of IP? Doesn't sound like a good check, can't you just pass a boolean? .................................................... File packaging/fedora/setup/engine-setup.py Line 1345: line_user = "" I don't get why do you need this. Line 1353: if line_user: same Line 1871: return Why are you doing this check inside the function and not outside? .................................................... File packaging/fedora/setup/engine_validators.py Line 246: # TODO: stop execution if it fails? Why would we want to stop execution? printing the error is enough .................................................... File packaging/fedora/setup/output_messages.py Line 135: INFO_CONF_PARAMS_DB_PASSWD_PROMPT="Enter local database password" If you're already changing it, why not change to the simple question "database password" or "local database password"? Line 148: INFO_CONF_PARAMS_REMOTE_DB_PASSWD_PROMPT="Enter remote database password" same, there's no neeed to use "enter" -- To view, visit http://gerrit.ovirt.org/3110 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2ff1f8c87836609ec1bd558bd3cd7e9a51e2db22 Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alex Lourie <alou...@redhat.com> Gerrit-Reviewer: Alex Lourie <alou...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Eyal Edri <ee...@redhat.com> Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
