[Kimchi-devel] [PATCH 1/3] Generate libvirt's interface XML definition for vlanned bridge

[Aline Manera]

On 01/03/2014 04:23 AM, Mark Wu wrote:
> Hi Aline,
>
> I would like to start a discussion about the code style for importing 
> modules by this chance.
>
> I saw you and Rodrigo had reorganized the "import" statements(commit 
> 65f6ad3 and e467b32).
> But personally, I don't agree with the rule you're following.  It 
> doesn't comply with PEP8 and bring
> extra unnecessary rules.
>
> 1.  Currently, the kimchi imports and external imports are separated.  
> But according to pep8:
>      we still need differentiate the standard library and third-party 
> library.  So we just have three groups
>      at most and put a blank line between each groups. [1]
>
> 2.  For better looking,  we can further organize the imports in each 
> group:
>      A.  Sort by the import statement:  all imports starting with 
> 'import' are put together while
>           all imports starting with 'from' are put together.  But we 
> don't need an explicit separating line
>           between them
>      B.  Sort by module name following the first word ('import' or 
> 'from')
>
> For this patch,  I don't think we need two blank to separate them 
> because they belongs to the same group.
>
> Does it make thanks for you?
>

As I've already said, we are a different rule from pep8 for imports

We are using the following rule:

import ...
import ...
import ...

<2 lines>

from ... import ...
from ... import ...

<2 lines>

import kimchi...
from kimchi import ...

<2 lines>

All those blocks must be in alphabetic order

So, please, organize the imports accordingly to it

> Thanks.
> Mark.
>
> [1] http://www.python.org/dev/peps/pep-0008/
>
>
>
> On 01/03/2014 02:00 AM, Aline Manera wrote:
>> On 01/02/2014 07:50 AM, Mark Wu wrote:
>>> To support vlanned virtual network, kimchi needs create the underlying
>>> vlan and bridge interface. Libvirt's interface driver can do it with
>>> a given XML definition. This patch targets to generate the XML 
>>> according
>>> to the interface and vlan id.
>>>
>>> Signed-off-by: Mark Wu <wudxw at linux.vnet.ibm.com>
>>> ---
>>>   src/kimchi/networkxml.py | 17 +++++++++++++++++
>>>   tests/test_networkxml.py | 20 ++++++++++++++++++++
>>>   tests/utils.py           |  6 ++++++
>>>   3 files changed, 43 insertions(+)
>>>
>>> diff --git a/src/kimchi/networkxml.py b/src/kimchi/networkxml.py
>>> index 786cb69..25157fd 100644
>>> --- a/src/kimchi/networkxml.py
>>> +++ b/src/kimchi/networkxml.py
>>> @@ -21,6 +21,8 @@
>>>   # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 
>>> 02110-1301 USA
>>>
>>>   import ipaddr
>>> +import lxml.etree as ET
>>
>> 2 lines
>>
>>> +from lxml.builder import E
>>>
>>>
>>>   # FIXME, do not support ipv6
>>> @@ -109,3 +111,18 @@ def to_network_xml(**kwargs):
>>>       </network>
>>>       """ % params
>>>       return xml
>>> +
>>> +
>>> +def create_vlanned_bridge_xml(bridge, interface, vlan_id):
>>> +    vlan = E.vlan(E.interface(name=interface))
>>> +    vlan.set('tag', vlan_id)
>>> +    m = E.interface(
>>> +        E.start(mode='onboot'),
>>> +        E.bridge(
>>> +            E.interface(
>>> +                vlan,
>>> +                type='vlan',
>>> +                name='.'.join([interface, vlan_id]))),
>>> +        type='bridge',
>>> +        name=bridge)
>>> +    return ET.tostring(m)
>>> diff --git a/tests/test_networkxml.py b/tests/test_networkxml.py
>>> index 3073bce..1b2e77f 100644
>>> --- a/tests/test_networkxml.py
>>> +++ b/tests/test_networkxml.py
>>> @@ -26,6 +26,7 @@ import unittest
>>>
>>>   import kimchi.networkxml as nxml
>>>   from kimchi.xmlutils import xpath_get_text
>>> +import utils
>>>
>>>
>>>   class NetworkXmlTests(unittest.TestCase):
>>> @@ -151,3 +152,22 @@ class NetworkXmlTests(unittest.TestCase):
>>>           netmask = xpath_get_text(xml, "/network/ip/@netmask")[0]
>>>           self.assertEquals(netmask,
>>> str(ipaddr.IPNetwork(params["net"]).netmask))
>>> +
>>> +
>>> +class InterfaceXmlTests(unittest.TestCase):
>>> +
>>> +    def test_vlanned_bridge_no_ip(self):
>>> +        expected_xml = """
>>> +            <interface type='bridge' name='br10'>
>>> +                <start mode='onboot'/>
>>> +                <bridge>
>>> +                    <interface type='vlan' name='em1.10'>
>>> +                      <vlan tag='10'>
>>> +                        <interface name='em1'/>
>>> +                      </vlan>
>>> +                    </interface>
>>> +              </bridge>
>>> +            </interface>
>>> +            """
>>> +        actual_xml = nxml.create_vlanned_bridge_xml('br10', 'em1', 
>>> '10')
>>> +        self.assertEquals(actual_xml, 
>>> utils.normalize_xml(expected_xml))
>>> diff --git a/tests/utils.py b/tests/utils.py
>>> index 008f668..79fc2e2 100644
>>> --- a/tests/utils.py
>>> +++ b/tests/utils.py
>>> @@ -32,6 +32,7 @@ import unittest
>>>
>>>
>>>   from contextlib import closing
>>> +from lxml import etree
>>>
>>>
>>>   import kimchi.server
>>> @@ -159,3 +160,8 @@ def patch_auth():
>>>
>>>       import kimchi.auth
>>>       kimchi.auth.authenticate = _authenticate
>>> +
>>> +
>>> +def normalize_xml(xml_str):
>>> +    return etree.tostring(etree.fromstring(xml_str,
>>> + etree.XMLParser(remove_blank_text=True)))
>>
>