Yaniv Bronhaim has posted comments on this change. Change subject: centralized manualSetupDispatcher code ......................................................................
Patch Set 10: (2 comments) https://gerrit.ovirt.org/#/c/41399/10/src/ovirt_hosted_engine_setup/check_liveliness.py File src/ovirt_hosted_engine_setup/check_liveliness.py: Line 33: def _(m): Line 34: return gettext.dgettext(message=m, domain='ovirt-hosted-engine-setup') Line 35: Line 36: Line 37: def manualSetupDispatcher(base, engine_fqdn=None): > You can get the engine fqdn from env, no need to pass it. Before we call this method I already read env fqdn variable. I assumed that it could be redundant access to read it twice, but as you prefer Line 38: response = '' Line 39: if engine_fqdn is None: Line 40: response = base.dialog.queryString( Line 41: name='OVEHOSTED_INSTALLING_OS', https://gerrit.ovirt.org/#/c/41399/10/src/plugins/ovirt-hosted-engine-setup/engine/add_host.py File src/plugins/ovirt-hosted-engine-setup/engine/add_host.py: Line 654: ohostedcons.StorageEnv.GLUSTER_PROVISIONING_ENABLED Line 655: ]: Line 656: cluster.set_gluster_service(True) Line 657: cluster.update() Line 658: cluster = engine_api.clusters.get(cluster_name) > Really need to get cluster again (searching again by name)? Please add a co probably not but I don't want to touch this logic in this patch. I can post following patch that avoid retrieving these information twice Line 659: Line 660: self.logger.debug('Adding the host to the cluster') Line 661: Line 662: engine_api.hosts.add( -- To view, visit https://gerrit.ovirt.org/41399 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I360cf8e0c3c0ed9db1e9af3127ebd7881ed11d11 Gerrit-PatchSet: 10 Gerrit-Project: ovirt-hosted-engine-setup Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Sandro Bonazzola <[email protected]> Gerrit-Reviewer: Simone Tiraboschi <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Yedidyah Bar David <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
