Antoni Segura Puimedon has posted comments on this change.
Change subject: Hidden bonds[2/2]: Refactored and updated nics implementation.
......................................................................
Patch Set 5: I would prefer that you didn't submit this
(4 inline comments)
....................................................
File lib/vdsm/netinfo.py
Line 88:
Line 89: def isHiddenBond(device):
Line 90: return _match_name(device, hidden_bonds)
Line 91:
Line 92: def isNotEnslavedHidBond(device):
isNotEnslavedByHiddenBond
Line 93: """
Line 94: Returns boolean stating if nic is not enslaved
Line 95: to an hidden bond.
Line 96: """
Line 114: else:
Line 115: return False
Line 116:
Line 117: devices = [os.path.basename(dev_path)
Line 118: for dev_path in glob.glob(NET_PATH + '/*')]
use iglob, no need to get a list of files, with a generator it should suffice.
Line 119: res = list(ifilter(predicate, devices))
Line 120:
Line 121: return res
Line 122:
Line 115: return False
Line 116:
Line 117: devices = [os.path.basename(dev_path)
Line 118: for dev_path in glob.glob(NET_PATH + '/*')]
Line 119: res = list(ifilter(predicate, devices))
I would very much prefer
res = [dev for dev in devices if predicate(dev)]
and change the name of predicate to something more telling about what the check
does.
Line 120:
Line 121: return res
Line 122:
Line 123:
Line 133: if not _match_name(bond, hidden_bonds):
Line 134: res.append(bond)
Line 135: except IOError as e:
Line 136: if e.errno == os.errno.ENOENT:
Line 137: return res
in case of exception should we return an empty list like up until now, or a
partial result like you put now?
Line 138: else:
Line 139: raise
Line 140:
Line 141: return res
--
To view, visit http://gerrit.ovirt.org/16239
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieab80a3ca6b243ade75e490b7004bebe96248140
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Vallarelli <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Giuseppe Vallarelli <[email protected]>
Gerrit-Reviewer: Livnat Peer <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
