Yedidyah Bar David has posted comments on this change.

Change subject: packaging: setup: Inform the user about availability of DWH and 
Reports
......................................................................


Patch Set 18:

(6 comments)

https://gerrit.ovirt.org/#/c/38277/18/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/core/__init__.py
File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/core/__init__.py:

Line 1
Line 2
Line 3
2014-2015


https://gerrit.ovirt.org/#/c/38277/18/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/core/advertise_dwh.py
File 
packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/core/advertise_dwh.py:

Line 42: class Plugin(plugin.PluginBase):
Line 43:     """Advertise DWH and Reports plugin."""
Line 44: 
Line 45:     def __init__(self, context):
Line 46:         super(Plugin, self).__init__(context=context)
Perhaps:

 self._engine_manual = None
 self._dwhIsUp = False
Line 47: 
Line 48:     @plugin.event(
Line 49:         stage=plugin.Stages.STAGE_INIT,
Line 50:     )


Line 45:     def __init__(self, context):
Line 46:         super(Plugin, self).__init__(context=context)
Line 47: 
Line 48:     @plugin.event(
Line 49:         stage=plugin.Stages.STAGE_INIT,
We usually do such things in STAGE_SETUP
Line 50:     )
Line 51:     def _init(self):
Line 52:         config = configfile.ConfigFile([
Line 53:             
oenginecons.FileLocations.OVIRT_ENGINE_SERVICE_CONFIG_DEFAULTS,


Line 91:         condition=lambda self: (
Line 92:             self.environment[oenginecons.CoreEnv.ENABLE] and
Line 93:             not self.environment[oenginecons.EngineDBEnv.NEW_DATABASE]
Line 94:         ),
Line 95:         priority=plugin.Stages.PRIORITY_HIGH,
Please use before or after and not priority.
Line 96:     )
Line 97:     def _check_dwh_status(self):
Line 98:         self._statement = database.Statement(
Line 99:             dbenvkeys=oenginecons.Const.ENGINE_DB_ENV_KEYS,


Line 138:                         ],
Line 139:                     )
Line 140:                 )
Line 141:             else:
Line 142:                 if 
self.environment[oengcommcons.ConfigEnv.JBOSS_AJP_PORT]:
Why is this test?

Anyway, can be nicer with:
 engineURI = (
   oenginecons.Const.ENGINE_URI
   if self.environment[oengcommcons.ConfigEnv.JBOSS_AJP_PORT]
   else '/'
 )
Line 143:                     engineURI = oenginecons.Const.ENGINE_URI
Line 144:                 else:
Line 145:                     engineURI = '/'
Line 146: 


Line 155:                         ],
Line 156:                         httpsPort=self.environment[
Line 157:                             oengcommcons.ConfigEnv.PUBLIC_HTTPS_PORT
Line 158:                         ],
Line 159:                         engineURI=engineURI,
or even put it directly here and drop the variable
Line 160:                         dwhURL=self.environment[
Line 161:                             osetupcons.DocsEnv.DWH_DOC_URL
Line 162:                         ],
Line 163:                         reportsURL=self.environment[


-- 
To view, visit https://gerrit.ovirt.org/38277
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I61fb88a6430eec6b6425075df0e749644654753a
Gerrit-PatchSet: 18
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lev Veyde <lve...@redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com>
Gerrit-Reviewer: Simone Tiraboschi <stira...@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

Reply via email to