Simone Tiraboschi has posted comments on this change. Change subject: packaging: setup: Making engine configuration optional at engine-setup ......................................................................
Patch Set 13: (11 comments) http://gerrit.ovirt.org/#/c/28413/13//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-06-05 18:42:52 +0200 Line 4: Commit: Simone Tiraboschi <stira...@redhat.com> Line 5: CommitDate: 2014-06-10 22:21:21 +0200 Line 6: Line 7: Making engine configuration optional during engine-setup > please make sure you add packaging: setup: as prefix for all setup related Done Line 8: Line 9: Making engine configuration optional asking Line 10: to the user if he/she want to configure it during Line 11: engine-setup. http://gerrit.ovirt.org/#/c/28413/13/packaging/setup/plugins/ovirt-engine-common/ovirt-engine-common/db/pgpass.py File packaging/setup/plugins/ovirt-engine-common/ovirt-engine-common/db/pgpass.py: Line 62: stage=plugin.Stages.STAGE_MISC, Line 63: name=oengcommcons.Stages.DB_CREDENTIALS_AVAILABLE_LATE, Line 64: condition=lambda self: self.environment[ Line 65: oengcommcons.EngineDBEnv.PASSWORD Line 66: ] is not None, > I am unsure how this is related to this patch, please explain. It's to prevent a crash on cleanup without a configured engine Line 67: ) Line 68: def _misc(self): Line 69: database.OvirtUtils( Line 70: plugin=self, http://gerrit.ovirt.org/#/c/28413/13/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/config/appmode.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/config/appmode.py: Line 104: 'Gluster', Line 105: ), Line 106: caseSensitive=False, Line 107: default=oenginecons.Defaults. Line 108: DEFAULT_CONFIG_APPLICATION_MODE, > please use () Done Line 109: ) Line 110: Line 111: @plugin.event( Line 112: stage=plugin.Stages.STAGE_MISC, http://gerrit.ovirt.org/#/c/28413/13/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/config/options.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/config/options.py: Line 254: ), Line 255: condition=lambda self: ( Line 256: self.environment[oenginecons.CoreEnv.ENABLE] and Line 257: self.environment[oengcommcons.EngineDBEnv.NEW_DATABASE] Line 258: ), > this is the same condition for all, please consider to add _enabled logic Done Line 259: ) Line 260: def _closeup(self): Line 261: self.dialog.note( Line 262: text=_( http://gerrit.ovirt.org/#/c/28413/13/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/pki/ca.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/pki/ca.py: Line 154: oenginecons.PKIEnv.ORG Line 155: ] = self.dialog.queryString( Line 156: name='OVESETUP_PKI_ORG', Line 157: note=_('Organization name for certificate ' Line 158: '[@DEFAULT@]: '), > please do not draw code, consistent single indent in all cases. Done Line 159: prompt=True, Line 160: default=org, Line 161: ) Line 162: else: Line 161: ) Line 162: else: Line 163: self.dialog.note( Line 164: text=_('PKI is already configured'), Line 165: ) > ? Done Line 166: Line 167: @plugin.event( Line 168: stage=plugin.Stages.STAGE_MISC, Line 169: name=osetupcons.Stages.CA_AVAILABLE, http://gerrit.ovirt.org/#/c/28413/13/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/provisioning/postgres.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/provisioning/postgres.py: Line 89: condition=lambda self: self._enabled, Line 90: priority=plugin.Stages.PRIORITY_HIGH Line 91: ) Line 92: def _customization_enable(self): Line 93: if not self.environment[oenginecons.CoreEnv.ENABLE]: > this can be one of the conditions above Done Line 94: self._enabled = False Line 95: Line 96: @plugin.event( Line 97: stage=plugin.Stages.STAGE_CUSTOMIZATION, http://gerrit.ovirt.org/#/c/28413/13/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/system/exportfs.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/system/exportfs.py: Line 118: condition=lambda self: self._enabled, Line 119: priority=plugin.Stages.PRIORITY_HIGH Line 120: ) Line 121: def _validation_enable(self): Line 122: if not self.environment[oenginecons.CoreEnv.ENABLE]: > one of the conditions above to all of these Done Line 123: self._enabled = False Line 124: Line 125: @plugin.event( Line 126: stage=plugin.Stages.STAGE_VALIDATION, Line 132: oenginecons.FileLocations.OVIRT_NFS_EXPORT_FILE Line 133: ) Line 134: elif os.path.exists(oenginecons.FileLocations.NFS_EXPORT_DIR): Line 135: self._source = oenginecons.FileLocations.NFS_EXPORT_FILE Line 136: self._destination = oenginecons.FileLocations.\ > use () Done Line 137: OVIRT_NFS_EXPORT_FILE Line 138: Line 139: content, old_line = self._getContentRemovePath( Line 140: self._source, http://gerrit.ovirt.org/#/c/28413/13/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/system/nfs.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/system/nfs.py: Line 166: osetupcons.ConfigEnv.APPLICATION_MODE Line 167: ] == 'gluster': Line 168: self.logger.info( Line 169: _('NFS configuration skipped with ' Line 170: 'application mode Gluster') > do not draw code Done Line 171: ) Line 172: self._enabled = False Line 173: else: Line 174: enabled = self.environment[ http://gerrit.ovirt.org/#/c/28413/13/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/system/selinux.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/system/selinux.py: Line 67: self.environment[oenginecons.CoreEnv.ENABLE] and Line 68: not self.environment[ Line 69: osetupcons.CoreEnv.DEVELOPER_MODE Line 70: ] Line 71: ) > I would have split this into two phases to make clear what each is doing. Done Line 72: if self._enabled: Line 73: if self.command.get('selinuxenabled', optional=True) is None: Line 74: self._enabled = False Line 75: else: -- 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: 13 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
