Nir Soffer has posted comments on this change.

Change subject: packaging: setup: showing the GUID instead of the physical dev 
on iSCSI setup
......................................................................


Patch Set 8:

(3 comments)

http://gerrit.ovirt.org/#/c/37494/8/src/ovirt_hosted_engine_setup/constants.py
File src/ovirt_hosted_engine_setup/constants.py:

Line 546: 
Line 547:     @ohostedattrs(
Line 548:         answerfile=True,
Line 549:         summary=True,
Line 550:         description=_('iSCSI LUN GUID'),
In engine ui, we call this LUN ID. Please check engine ui and match the wording 
used in hosted engine to those used in the ui. It is better to use less correct 
term consistently then use different terms.

Also, this has nothing to do with iSCSI - just call it LUN ID.
Line 551:     )
Line 552:     def ISCSI_LUN_GUID(self):
Line 553:         return 'OVEHOSTED_STORAGE/iSCSILunGUID'
Line 554: 


Line 549:         summary=True,
Line 550:         description=_('iSCSI LUN GUID'),
Line 551:     )
Line 552:     def ISCSI_LUN_GUID(self):
Line 553:         return 'OVEHOSTED_STORAGE/iSCSILunGUID'
Same here, you don't need iSCSI prefix, LunId or LunGuid is good enough.
Line 554: 
Line 555:     ISCSI_PASSWORD = 'OVEHOSTED_STORAGE/iSCSIPortalPassword'
Line 556: 
Line 557:     BDEVICE_SIZE_GB = 'OVEHOSTED_STORAGE/blockDeviceSizeGB'


http://gerrit.ovirt.org/#/c/37494/8/src/plugins/ovirt-hosted-engine-setup/storage/iscsi.py
File src/plugins/ovirt-hosted-engine-setup/storage/iscsi.py:

Line 213:                 vendorID=entry['vendorID'],
Line 214:                 status=entry['status'],
Line 215:                 ap=entry['activep'],
Line 216:                 fp=entry['failedp'],
Line 217:             )
Nice, but you can simplify this:

First, add an index to the entries:

    f_luns.append(
        {
             'index': len(f_luns),
             'guid': entry["GUID"],
             ...
        }
    )

Then use the dict when formatting:

    for entry in f_luns:
        lun_list += "{index} - {guid} ...".format(**entry)
Line 218: 
Line 219:         self.dialog.note(
Line 220:             _(
Line 221:                 'The following luns have been found on the requested 
target:\n'


-- 
To view, visit http://gerrit.ovirt.org/37494
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8cd9c02e8c753db5df6816a38317b493c9b1e7f
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-hosted-engine-setup
Gerrit-Branch: master
Gerrit-Owner: Simone Tiraboschi <[email protected]>
Gerrit-Reviewer: Lev Veyde <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Simone Tiraboschi <[email protected]>
Gerrit-Reviewer: Yedidyah Bar David <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to