Ondřej Svoboda has posted comments on this change.

Change subject: net: monitor: validate passed groups argument
......................................................................


Patch Set 2:

(1 comment)

There it is ;-)

https://gerrit.ovirt.org/#/c/44530/2/lib/vdsm/netlink/monitor.py
File lib/vdsm/netlink/monitor.py:

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))
> this is tricky, we are passing an iterable, so string formating assigns fir
Petr, even I have encountered this problem (implicit tuple unpacking by the % 
operator) a few times and usually handle it by putting the iterable in a 
1-tuple, so:

  raise AttributeError('Invalid groups: %s' % (unknown_groups,))

Another option is to use str.format() which removes the ambiguity (but is not 
as short, so sometimes I prefer %):

  raise AttributeError('Invalid groups: {}'.format(unknown_groups))

Note that '{}' can be used (instead of explicitly numbered '{0}') now that we 
stopped supporting Python 2.6.
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: Adam Litke <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Ido Barkan <[email protected]>
Gerrit-Reviewer: Jenkins CI
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

Reply via email to