Sandro Bonazzola has posted comments on this change. Change subject: WIP: packaging: setup: host rename ......................................................................
Patch Set 5: I would prefer that you didn't submit this (9 inline comments) .................................................... File packaging/setup/plugins/ovirt-engine-rename/core/pki.py Line 101: osetupcons.FileLocations.OVIRT_ENGINE_PKI_APACHE_STORE, Line 102: osetupcons.FileLocations.OVIRT_ENGINE_PKI_APACHE_KEY, Line 103: osetupcons.FileLocations.OVIRT_ENGINE_PKI_APACHE_CERT, Line 104: ) Line 105: ) is there any reason for marking as modified files which are not modified? Line 106: Line 107: @plugin.event( Line 108: stage=plugin.Stages.STAGE_VALIDATION, Line 109: condition=lambda self: self._enabled, .................................................... File packaging/setup/plugins/ovirt-engine-rename/core/protocols.py Line 81: self.environment[ Line 82: osetupcons.ConfigEnv.HTTPS_PORT Line 83: ] = self.environment[ Line 84: osetupcons.ConfigEnv.JBOSS_HTTPS_PORT Line 85: ] maybe this needs to be updated for http://gerrit.ovirt.org/#/c/17310/ Line 86: self.environment[ Line 87: otopicons.CoreEnv.MODIFIED_FILES Line 88: ].append( Line 89: osetupcons.FileLocations. Line 87: otopicons.CoreEnv.MODIFIED_FILES Line 88: ].append( Line 89: osetupcons.FileLocations. Line 90: OVIRT_ENGINE_SERVICE_CONFIG_PROTOCOLS Line 91: ) This file is not modified yet, any reason for marking it as modified? Line 92: Line 93: @plugin.event( Line 94: stage=plugin.Stages.STAGE_MISC, Line 95: ) Line 96: def _misc(self): Line 97: # Line 98: # TODO Line 99: # Defaults of engine should be HTTP[s]_ENABLED=false Line 100: # maybe this needs to be updated for http://gerrit.ovirt.org/#/c/17310/ Line 101: if self.environment[osetupcons.CoreEnv.DEVELOPER_MODE]: Line 102: content = ( Line 103: 'ENGINE_FQDN={fqdn}\n' Line 104: 'ENGINE_PROXY_ENABLED=false\n' Line 150: OVIRT_ENGINE_SERVICE_CONFIG_PROTOCOLS Line 151: ), Line 152: content=content, Line 153: ) Line 154: ) Here should be added to modified list Line 155: Line 156: @plugin.event( Line 157: stage=plugin.Stages.STAGE_CLOSEUP, Line 158: before=[ Line 182: Line 183: if self.environment[osetupcons.CoreEnv.DEVELOPER_MODE]: Line 184: self.dialog.note( Line 185: text=_( Line 186: 'JBoss is listending for debug connection at: {address}' listending -> listening Line 187: ).format( Line 188: address=self.environment[ Line 189: osetupcons.ConfigEnv.JBOSS_DEBUG_ADDRESS Line 190: ], .................................................... File packaging/setup/plugins/ovirt-engine-rename/core/tools.py Line 72: def _setup(self): Line 73: for entry in self.TOOLS_CONFIG: Line 74: self.environment[ Line 75: otopicons.CoreEnv.MODIFIED_FILES Line 76: ].append(self._entry_filename(entry)) as in previous comments, no reason for marking modified here Line 77: Line 78: @plugin.event( Line 79: stage=plugin.Stages.STAGE_MISC, Line 80: ) Line 98: ], Line 99: user=osetupcons.Const.USER_ADMIN, Line 100: domain=osetupcons.Const.DOMAIN_INTERNAL, Line 101: ) Line 102: ), should be added to modified list here Line 103: ) Line 104: ) Line 105: Line 106: .................................................... File packaging/setup/plugins/ovirt-engine-rename/core/uninstall.py Line 33: Line 34: Line 35: from ovirt_engine_setup import constants as osetupcons Line 36: from ovirt_engine_setup import dialog Line 37: I suggest to avoid using MODIFIED_FILES list for doing this check. Maybe creating a list like "CHECK_MODIFIED_FILES" would be better. So at validation you can check the files in CHECK_MODIFIED_FILES that you populate at SETUP stage in other plugins and write the uninstall file with only the files in MODIFIED_FILES allowing cleanup to do its work correctly. Line 38: Line 39: @util.export Line 40: class Plugin(plugin.PluginBase): Line 41: """Simple plugin.""" -- To view, visit http://gerrit.ovirt.org/17408 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I28cb0285424236fd3e6907694f6bf1ce6ce3367f Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yedidyah Bar David <[email protected]> Gerrit-Reviewer: Alex Lourie <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Moran Goldboim <[email protected]> Gerrit-Reviewer: Ofer Schreiber <[email protected]> Gerrit-Reviewer: Sandro Bonazzola <[email protected]> Gerrit-Reviewer: Yedidyah Bar David <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
