Ofer Schreiber has posted comments on this change.
Change subject: packaging: Adding remoteDb support to dwh
......................................................................
Patch Set 4: Fails
(12 inline comments)
....................................................
File packaging/common_utils.py
Line 31: ERR_RC_CODE = "Return Code is not zero"
Please add TODO: Move all ERRORS here
Line 381: def _maskString(string, maskList=[]):
please check how password logging works
Line 392: def execCmd(cmdList, cwd=None, failOnError=False, msg=ERR_RC_CODE,
maskList=[], useShell=False, usePipeFiles=False):
Again, add TODO to move all commands to execCmd
....................................................
File packaging/ovirt-engine-dwh-setup.py
Line 33: ERR_DB_CREATE_FAILED="error while trying to create
ovirt_engine_history db"
is that the right place?
if not, please add TODO
Line 65: "-r", install_type]
please use db_dict
Line 96: def getDbAdminUser():
move to utils?
Line 104: return DB_USER_NAME
please check why do we need default values
Line 120: Use default settings if file is not found, or '*' was used.
wrong desc
Line 140: if "oVirt-engine DB ADMIN settings section" in line:
Please add yourself a TODO to mark it as critical in engine-setup
also, please check why we have two lines in the DB_ADMIN in .pgpass
Line 150: return False
exception
Line 168: db_pass = list[4].rstrip('\n')
break?
what if you didn't find any password??
Line 184:
"jdbc\:postgresql\://%s\:%s/engine?stringtype\=unspecified" % (getDbHostName(),
getDbPort()))
stringtype\=unspecified?
--
To view, visit http://gerrit.ovirt.org/4707
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iec31497fceae9c335d6ec2c073388fea1335ece4
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-dwh
Gerrit-Branch: master
Gerrit-Owner: Alex Lourie <alou...@redhat.com>
Gerrit-Reviewer: Alex Lourie <alou...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com>
Gerrit-Reviewer: Yaniv Dary <yd...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
