Petr Horáček has posted comments on this change. Change subject: net: monitor: validate passed groups argument ......................................................................
Patch Set 2: (3 comments) https://gerrit.ovirt.org/#/c/44530/2/lib/vdsm/netlink/monitor.py File lib/vdsm/netlink/monitor.py: Line 92: self._time_start = None Line 93: self._timeout = timeout Line 94: self._silent_timeout = silent_timeout Line 95: if groups: Line 96: difference = set(groups).difference(set(_GROUPS)) > You can also call the variable "unknown_groups" or the like :-) Done Line 97: if difference: Line 98: raise AttributeError('Invalid groups: %s' % str(difference)) Line 99: self._groups = groups Line 100: else: Line 92: self._time_start = None Line 93: self._timeout = timeout Line 94: self._silent_timeout = silent_timeout Line 95: if groups: Line 96: difference = set(groups).difference(set(_GROUPS)) > frozenset() is a bit cooler, as we don't expect to change anything in these Done Line 97: if difference: Line 98: raise AttributeError('Invalid groups: %s' % str(difference)) Line 99: self._groups = groups Line 100: else: Line 94: self._silent_timeout = silent_timeout Line 95: if groups: Line 96: difference = set(groups).difference(set(_GROUPS)) Line 97: if difference: Line 98: raise AttributeError('Invalid groups: %s' % str(difference)) > no need to call str(). %s does the write thing. this is tricky, we are passing an iterable, so string formating assigns first item to the first %s, then it tries to assign second item to next %s and fails. we have to convert it first Line 99: self._groups = groups Line 100: else: Line 101: self._groups = _GROUPS.keys() Line 102: self._queue = Queue.Queue() -- To view, visit https://gerrit.ovirt.org/44530 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5cef124e79a61cf7fa2af2b0c91803c3994c99d7 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Ido Barkan <[email protected]> Gerrit-Reviewer: Ondřej Svoboda <[email protected]> Gerrit-Reviewer: Petr Horáček <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
