Change in vdsm[master]: net: OVS driver

edwardh at redhat.com edwardh at redhat.com
Mon Apr 25 07:43:04 UTC 2016


Edward Haas has posted comments on this change.

Change subject: net: OVS driver
......................................................................


Patch Set 18: -Verified

(4 comments)

https://gerrit.ovirt.org/#/c/55958/18/lib/vdsm/network/ovs/driver/vsctl.py
File lib/vdsm/network/ovs/driver/vsctl.py:

Line 60:                 'Executing commands failed: %s' % '\n'.join(err))
Line 61:         if out is None:
Line 62:             return
Line 63: 
Line 64:         for i, row in enumerate(out):
> everybody calls this "line"
Done
Line 65:             self.commands[i].result = row
Line 66:         return [cmd.result for cmd in self.commands]
Line 67: 
Line 68:     def add(self, *commands):


Line 86: 
Line 87:     @result.setter
Line 88:     def result(self, data):
Line 89:         self._result = data.split(r'\n') if data else []
Line 90: 
> any reason not to use splitlines()?
For commands like ovs-vsctl list-br, we get an output that is converted to a list. Because we use '--oneline', the raw output includes '\n' as a delimiter.
So we must use r'\n' ('\\n).

Here is a raw example of this: 'ovstest0\novstest1'
Line 91: 
Line 92: class CommandResultDB(Command):
Line 93: 
Line 94:     @Command.result.setter


PS18, Line 92: CommandResultDB
> naming: this is not a DB, but a Command.
Yoda and me would not agree but...
Done


PS18, Line 95: result
> Pardon, but I need a unit test for this object. Something that eats a typic
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic24a9cea8d50e801de12da1c9cfcd2d195aed684
Gerrit-PatchSet: 18
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: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list