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

Reply via email to