Alon Bar-Lev has posted comments on this change. Change subject: Making engine configuration optional during engine-setup ......................................................................
Patch Set 9: (4 comments) http://gerrit.ovirt.org/#/c/28413/9/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/apache/ssl.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/apache/ssl.py: Line 178 Line 179 Line 180 Line 181 Line 182 put this in own stage at higher priority, and remove the nesting? http://gerrit.ovirt.org/#/c/28413/9/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/config/appmode.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/config/appmode.py: Line 69: name=osetupcons.Stages.CONFIG_APPLICATION_MODE_AVAILABLE Line 70: ) Line 71: def _customization(self): Line 72: if not self.environment[oenginecons.CoreEnv.ENABLE]: Line 73: self._enabled = False probably same here and at other places, try to reduce nesting. Line 74: else: Line 75: self._enabled = True Line 76: if self.environment[ Line 77: osetupcons.ConfigEnv.APPLICATION_MODE http://gerrit.ovirt.org/#/c/28413/9/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/config/options.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/config/options.py: Line 68: oenginecons.Stages.DIALOG_TITLES_E_ENGINE, Line 69: ), Line 70: after=( Line 71: oengcommcons.Stages.DB_CONNECTION_STATUS, Line 72: oenginecons.Stages.CORE_ENABLE, probably best to rename CORE_ENABLE with DIALOG_TITLES_something Line 73: ), Line 74: condition=lambda self: self.environment[oenginecons.CoreEnv.ENABLE], Line 75: ) Line 76: def _customization(self): http://gerrit.ovirt.org/#/c/28413/9/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/core/misc.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/core/misc.py: Line 53: ), Line 54: after=( Line 55: osetupcons.Stages.DIALOG_TITLES_S_PRODUCT_OPTIONS, Line 56: ), Line 57: priority=plugin.Stages.PRIORITY_HIGH, why priority high? you should probably also have a start and end names for the content of other questions, what I mean is I do not understand why you use CORE_ENABLE as dependency in any of the other stages instead of being after the DIALOG_TITLES_E_PRODUCT_OPTIONS. Line 58: ) Line 59: def _customization(self): Line 60: if self.environment[oenginecons.CoreEnv.ENABLE] is None: Line 61: self.environment[ -- To view, visit http://gerrit.ovirt.org/28413 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7a7fbc9d2abc141bed49eff20549289cba4a4a61 Gerrit-PatchSet: 9 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Simone Tiraboschi <stira...@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: 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
