Nir Soffer has posted comments on this change. Change subject: packaging: setup: SANWipeAfterDelete configuration ......................................................................
Patch Set 3: (8 comments) http://gerrit.ovirt.org/#/c/37397/3/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/config/storage.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/config/storage.py: Line 26: from otopi import util Line 27: from otopi import plugin Line 28: Line 29: Line 30: from ovirt_engine_setup.constants import ConfigEnv as configEnv Why do you import as another name? what is wrong with the original name? Line 31: from ovirt_engine_setup.engine import constants as oenginecons Line 32: from ovirt_engine_setup.engine_common \ Line 33: import constants as oengcommcons Line 34: from ovirt_engine_setup import dialog Line 46: def _init(self): Line 47: self.environment.setdefault( Line 48: SAN_WIPE_AFTER_DELETE, Line 49: None Line 50: ) Does it fit in one line? Line 51: Line 52: @plugin.event( Line 53: stage=plugin.Stages.STAGE_CUSTOMIZATION, Line 54: before=( Line 57: after=( Line 58: oengcommcons.Stages.DIALOG_TITLES_S_STORAGE, Line 59: ), Line 60: condition=lambda self: Line 61: self._newDatabaseCondition(), This does not make sense - condition requires a function returning boolean. You use a lambda (anonymous function) accepting self and returning the value of self._newDatabaseCondition(). self._newDatabaseCondition is a function accepting self and returning a boolean. Use: condition=self._newDatabaseCondition Line 62: ) Line 63: def _configureSANWipeAfterDelete(self): Line 64: if self.environment[SAN_WIPE_AFTER_DELETE] is None: Line 65: # Value for SAN_WIPE_AFTER_DELETE is not forced. Line 61: self._newDatabaseCondition(), Line 62: ) Line 63: def _configureSANWipeAfterDelete(self): Line 64: if self.environment[SAN_WIPE_AFTER_DELETE] is None: Line 65: # Value for SAN_WIPE_AFTER_DELETE is not forced. Better use this form: if abnormal flow: # explain this case return normal flow This form is even more important in this code, since you have to work with much too long variables, and satisfy pep8 line length limits. Line 66: sanWipeAfterDelete = dialog.queryBoolean( Line 67: dialog=self.dialog, Line 68: name='OVESETUP_CONFIG_SAN_WIPE_AFTER_DELETE', Line 69: note=_( Line 74: default=False, Line 75: ) Line 76: self.environment[ Line 77: SAN_WIPE_AFTER_DELETE Line 78: ] = sanWipeAfterDelete Does it fit on one line? Line 79: Line 80: @plugin.event( Line 81: stage=plugin.Stages.STAGE_MISC, Line 82: after=( Line 82: after=( Line 83: oengcommcons.Stages.DB_CONNECTION_AVAILABLE, Line 84: ), Line 85: condition=lambda self: Line 86: self._newDatabaseCondition(), Does it fit on the same line? Line 87: ) Line 88: def _updateSANWipeAfterDelete(self): Line 89: option = vdcoption.VdcOption( Line 90: statement=self.environment[ Line 88: def _updateSANWipeAfterDelete(self): Line 89: option = vdcoption.VdcOption( Line 90: statement=self.environment[ Line 91: oenginecons.EngineDBEnv.STATEMENT Line 92: ] Does this fit on one line? Line 93: ) Line 94: options=( Line 95: { Line 96: 'name': 'SANWipeAfterDelete', Line 98: }, Line 99: ) Line 100: option.updateVdcOptions(options=options,) Line 101: Line 102: def _newDatabaseCondition(self): How about _settingUpNewDatabase() ? Line 103: # Returns a condition that validates Line 104: # we are running the setup on a new database. Line 105: return self.environment[oenginecons.EngineDBEnv.NEW_DATABASE] Line 106: -- To view, visit http://gerrit.ovirt.org/37397 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I911f87ad34eafc83eb2c7aa346fc4278ada28b5b Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com> Gerrit-Reviewer: Candace Sheremeta <csher...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@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
