Alon Bar-Lev has posted comments on this change.
Change subject: packaging: updated handling of pgpass file between versions
......................................................................
Patch Set 7: (7 inline comments)
....................................................
File packaging/fedora/setup/common_utils.py
Line 373: if failOnError and proc.returncode != 0:
Line 374: raise Exception(msg)
Line 375: return ("".join(output.splitlines(True)), proc.returncode)
Line 376:
Line 377: def execSqlCommand(userName, dbName, sqlQuery, failOnError=False,
errMsg=output_messages.ERR_SQL_CODE, envDict=None):
again, why do we need this function?
Line 378: logging.debug("running sql query \'%s\' on db." % sqlQuery)
Line 379: cmd = [
Line 380: basedefs.EXEC_PSQL,
Line 381: "-U", userName,
Line 679: inDbAdminSection = True
Line 680: continue
Line 681:
Line 682: if inDbAdminSection and param == "admin" and \
Line 683: not line.startswith("#"):
the space here was good... why did you remove it?
Line 684: # Means we're on DB ADMIN line, as it's for all
DBs
Line 685: dbcreds = line.split(":", 4)
Line 686: return dbcreds[field[param]]
Line 687:
Line 687:
Line 688: # Fetch the password if needed
Line 689: if param == "password" \
Line 690: and user \
Line 691: and not line.startswith("#"):
Same here...
if (
condition and
condition
):
statement
this indent is good as you see where if begins and where it ends, and the
statement is in indent.
Line 692: dbcreds = line.split(":", 4)
Line 693: if dbcreds[3] == user:
Line 694: return dbcreds[field[param]]
Line 695:
Line 918
Line 919
Line 920
Line 921
Line 922
forgive me for noticing this only now... but why don't you use
execRemoteSqlCommand for these?
....................................................
File packaging/fedora/setup/engine-cleanup.py
Line 356: dbHost=DB_HOST,
Line 357: dbPort=DB_PORT,
Line 358: dbName=basedefs.DB_NAME,
Line 359: sqlQuery="select 1",
Line 360: envDict=self.env,
Not sure this is required... as the environment is added at function no?
Line 361: )
Line 362: if rc != 0:
Line 363: if
utils.verifyStringFormat(out,".*FATAL:\s*database\s*\"%s\"\s*does not exist" %
(PREFIX)):
Line 364: # This means that the db is not installed, so we
return false
....................................................
File packaging/fedora/setup/engine-upgrade.py
Line 776: dbName=basedefs.DB_NAME,
Line 777: sqlQuery=queryCheckDCVersions,
Line 778: failOnError=True,
Line 779: errMsg=MSG_ERROR_CONNECT_DB,
Line 780: envDict=env,
is env needed? we add it in the execRemoteSqlCommand
Line 781: )
Line 782: queryCheckClusterVersions="SELECT compatibility_version FROM
vds_groups;"
Line 783: clusterVersions, rc = utils.execRemoteSqlCommand(
Line 784: userName=SERVER_ADMIN,
Line 787: dbName=basedefs.DB_NAME,
Line 788: sqlQuery=queryCheckClusterVersions,
Line 789: failOnError=True,
Line 790: errMsg=MSG_ERROR_CONNECT_DB,
Line 791: envDict=env,
Same here.
Line 792: )
Line 793:
Line 794: for versions in dcVersions, clusterVersions:
Line 795: if oldversion in versions:
--
To view, visit http://gerrit.ovirt.org/8135
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d44297725d7270982e8724e4aeefda8bc7a88e2
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alex Lourie <[email protected]>
Gerrit-Reviewer: Alex Lourie <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Kiril Nesenko <[email protected]>
Gerrit-Reviewer: Moran Goldboim <[email protected]>
Gerrit-Reviewer: Ofer Schreiber <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
