Change in vdsm[master]: net tests: IPv6 autoconf with RA server

edwardh at redhat.com edwardh at redhat.com
Tue Mar 15 16:16:15 UTC 2016


Edward Haas has posted comments on this change.

Change subject: net tests: IPv6 autoconf with RA server
......................................................................


Patch Set 4:

(2 comments)

https://gerrit.ovirt.org/#/c/54668/4/tests/network/nettestlib.py
File tests/network/nettestlib.py:

Line 420: @contextmanager
Line 421: def wait_for_ipv6(iface, wait_for_scopes=None):
Line 422:     """Wait for iface to get their IPv6 addresses with netlink Monitor"""
Line 423:     if not wait_for_scopes:
Line 424:         wait_for_scopes = ['global', 'link']
> mutables should not be default arguments?
Nope: http://docs.python-guide.org/en/latest/writing/gotchas/
Line 425:     try:
Line 426:         with monitor.Monitor(groups=('ipv6-ifaddr',), timeout=20) as mon:
Line 427:             yield
Line 428:             for event in mon:


Line 430: event.get('event') == 'new_addr' and
        :                    event.get('scope') in wait_for_scopes):
> i think it's better to enclose all conditions, now it looks like there is s
pyflakes/pep8 have not allowed me to do it, maybe you have a tip.

if (dev_name == ..... and
    event.get('scope....):
    wait_for_scopes.....()

The 'body' has the same indentation as the last condition line and it does not like it.


-- 
To view, visit https://gerrit.ovirt.org/54668
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7c153adf7d27db0151a330f55719622a8f730a9b
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list