Yedidyah Bar David has posted comments on this change. Change subject: centralized manualSetupDispatcher code ......................................................................
Patch Set 10: (2 comments) Nice job. Some comments inside. Indentation/style is not always compatible with existing (e.g. we generally try to avoid '\'), didn't comment on that. 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. You use this param mainly as a flag to decide what to do. Perhaps name it accordingly? prompt_os=True, or prompt_engine=False, or even string - prompt=None (and inside if prompt=='engine', if prompt=='os') 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 comment if so. 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 <ybron...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
