Yedidyah Bar David has posted comments on this change. Change subject: packaging: setup: Add host rename script ......................................................................
Patch Set 21: (9 comments) .................................................... File packaging/setup/plugins/ovirt-engine-common/core/protocols.py Line 71: ) Line 72: self.environment.setdefault( Line 73: osetupcons.ConfigEnv.JBOSS_DIRECT_HTTPS_PORT, Line 74: None Line 75: ) Not sure I understand. Did you notice that this is in ovirt-engine-common? Moved from ovirt-engine-setup? And not in -rename? You claim that it can/should be left in -setup? Line 76: Line 77: @plugin.event( Line 78: stage=plugin.Stages.STAGE_SETUP, Line 79: ) .................................................... File packaging/setup/plugins/ovirt-engine-rename/core/database.py Line 126: select Line 127: connection Line 128: from storage_server_connections Line 129: where Line 130: id=%(storage)s Mostly copied as-is from plugins/ovirt-engine-setup/legacy/isodomain.py, I didn't think much about optimizing it. As in other cases, I'd rather not change copied code just in one place, but either to change it everywhere or merge to one place. This means that, assuming we currently decided to not try much to merge, if you think it should be changed in both places, say so. I personally see no problem in changing it later in both places. Line 131: """, Line 132: args=dict( Line 133: storage=domain['storage'] Line 134: ), Line 132: args=dict( Line 133: storage=domain['storage'] Line 134: ), Line 135: ownConnection=True, Line 136: )[0] Same. Including the comment below. Line 137: #returns only one line, can't exists duplicate and must exist one Line 138: host, path = conn_row['connection'].split(':') Line 139: if host == self.environment[osetupcons.ConfigEnv.FQDN]: Line 140: my_domains.append(domain['storage_name']) Line 134: ), Line 135: ownConnection=True, Line 136: )[0] Line 137: #returns only one line, can't exists duplicate and must exist one Line 138: host, path = conn_row['connection'].split(':') Same. Line 139: if host == self.environment[osetupcons.ConfigEnv.FQDN]: Line 140: my_domains.append(domain['storage_name']) Line 141: if my_domains: Line 142: self.logger.warning(_('Engine host hosting Storage Domains')) Line 176: statement.updateVdcOptions( Line 177: options=( Line 178: { Line 179: 'name': option, Line 180: 'value': newvalue, I personally find it more readable this way, and do not see significant reasons against leaving it so. Do you? Line 181: }, Line 182: ) Line 183: ) Line 184: .................................................... File packaging/setup/plugins/ovirt-engine-rename/core/pki.py Line 114: osetupcons.FileLocations. Line 115: OVIRT_ENGINE_PKI_ENGINE_CA_CERT Line 116: ), Line 117: ) Line 118: ) I agree. Done. Line 119: else: Line 120: self._enabled = True Line 121: Line 122: self.environment[ .................................................... File packaging/setup/plugins/ovirt-engine-rename/core/postinstall.py Line 54: ) Line 55: Line 56: @plugin.event( Line 57: stage=plugin.Stages.STAGE_MISC, Line 58: priority=plugin.Stages.PRIORITY_LAST, No reason, apart from that it was copied from -setup, where it does make sense :-) Removed. Line 59: ) Line 60: def _misc(self): Line 61: config = ( Line 62: osetupcons.FileLocations. .................................................... File packaging/setup/plugins/ovirt-engine-rename/core/protocols.py Line 51: def _setup(self): Line 52: self.environment[ Line 53: osetupcons.RenameEnv.FILES_TO_BE_MODIFIED Line 54: ].append(osetupcons.FileLocations. Line 55: OVIRT_ENGINE_SERVICE_CONFIG_PROTOCOLS) Done Line 56: Line 57: @plugin.event( Line 58: stage=plugin.Stages.STAGE_MISC, Line 59: ) Line 95: after=( Line 96: osetupcons.Stages.DIALOG_TITLES_S_SUMMARY, Line 97: ), Line 98: ) Line 99: def _closeup(self): Done Line 100: # TODO Line 101: # layout of jboss and proxy should be the same Line 102: if self.environment[osetupcons.ConfigEnv.JBOSS_AJP_PORT]: Line 103: engineURI = osetupcons.Const.ENGINE_URI -- 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: 21 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: Alex Lourie <alou...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Moran Goldboim <mgold...@redhat.com> Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com> Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches