Alon Bar-Lev has posted comments on this change. Change subject: packaging: setup: Allow dwh on separate host ......................................................................
Patch Set 11: (6 comments) all this applies to engine as well http://gerrit.ovirt.org/#/c/27516/11/ovirt-engine-dwh.spec.in File ovirt-engine-dwh.spec.in: Line 103: Requires: java-1.7.0-openjdk Line 104: Requires: jpackage-utils Line 105: Requires: logrotate Line 106: Requires: postgresql-jdbc Line 107: Requires: postgresql-server >= 8.4.7 not sure about this dependency, usually if installed on engine you get postgres from engine which is also wrong, but if you install on different host you probably do not need engine Line 108: Line 109: %description Line 110: The %{product_description} package provides Line 111: the ETL process and DB scripts to create a historic database API. http://gerrit.ovirt.org/#/c/27516/11/packaging/setup/ovirt_engine_setup/dwh/constants.py File packaging/setup/ovirt_engine_setup/dwh/constants.py: Line 101: 'secured': EngineDefaults.DEFAULT_DB_SECURED, Line 102: 'hostValidation': EngineDefaults.DEFAULT_DB_SECURED_HOST_VALIDATION, Line 103: 'user': EngineDefaults.DEFAULT_DB_USER, Line 104: 'password': EngineDefaults.DEFAULT_DB_PASSWORD, Line 105: 'database': EngineDefaults.DEFAULT_DB_DATABASE, again, we do not need engine defaults if we are consumers Line 106: } Line 107: Line 108: Line 109: @util.export Line 123: class EngineDefaults(object): Line 124: DEFAULT_DB_HOST = 'localhost' Line 125: DEFAULT_DB_PORT = 5432 Line 126: DEFAULT_DB_DATABASE = 'engine' Line 127: DEFAULT_DB_USER = 'engine' and probably all these defaults can be dropped as used only at above defaults Line 128: DEFAULT_DB_PASSWORD = '' Line 129: DEFAULT_DB_SECURED = False Line 130: DEFAULT_DB_SECURED_HOST_VALIDATION = False Line 131: http://gerrit.ovirt.org/#/c/27516/11/packaging/setup/plugins/ovirt-engine-common/ovirt-engine-dwh/db/connection.py File packaging/setup/plugins/ovirt-engine-common/ovirt-engine-dwh/db/connection.py: Line 116: ) Line 117: self.environment.setdefault( Line 118: odwhcons.EngineDBEnv.DATABASE, Line 119: None Line 120: ) probably we can add helper function within database.py to get dbkeys and set all to none, instead of duplicating this. Line 121: Line 122: self.environment[odwhcons.EngineDBEnv.CONNECTION] = None Line 123: self.environment[odwhcons.EngineDBEnv.STATEMENT] = None Line 124: self.environment[odwhcons.EngineDBEnv.NEW_DATABASE] = True http://gerrit.ovirt.org/#/c/27516/11/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine-dwh/core/config.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine-dwh/core/config.py: Line 140 Line 141 Line 142 Line 143 Line 144 why can't we have getDBConfig that gets dbkeys? http://gerrit.ovirt.org/#/c/27516/11/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine-dwh/db/connection.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine-dwh/db/connection.py: Line 108: ), Line 109: after=( Line 110: oengcommcons.Stages.DIALOG_TITLES_S_DATABASE, Line 111: oengcommcons.Stages.DB_OWNERS_CONNECTIONS_CUSTOMIZED, Line 112: ), once again, there should be separate start *AND* end for owners and consumers. Line 113: ) Line 114: def _engine_customization(self): Line 115: dbovirtutils = database.OvirtUtils( Line 116: plugin=self, -- To view, visit http://gerrit.ovirt.org/27516 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06fc960481af258b3954a8968be3439393d3ebdb Gerrit-PatchSet: 11 Gerrit-Project: ovirt-dwh 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: Yaniv Dary <[email protected]> Gerrit-Reviewer: Yedidyah Bar David <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
