Sandro Bonazzola has posted comments on this change. Change subject: packaging: setup: Allow dwh on separate host ......................................................................
Patch Set 11: (6 comments) 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 post yes, I thought about it too. we added this dependency to align with engine does. I think that when we can behave like a proper application we can drop dependency on postgresql-server making this dependency optional. We provide support for remote DB so postgresql-server may be totally unneeded. But for now, I think it's better to require it. 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 Done 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 defaul Done 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 se added TODO 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? can you explain a bit more what do you mean? 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 consume If I understood correctly, it's already separed: oengcommcons.Stages.DIALOG_TITLES_S_DATABASE owners stuff oengcommcons.Stages.DB_OWNERS_CONNECTIONS_CUSTOMIZED consumers stuff oengcommcons.Stages.DIALOG_TITLES_E_DATABASE Am I missing something? 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 <d...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com> Gerrit-Reviewer: Yaniv Dary <yd...@redhat.com> Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
