Repository: zhmcclient/python-zhmcclient

Issue:
get_connected_vnics() returns NICs as URIs
`VirtualSwitch.get_connected_vnics()` returns the NICs that are connected to the virtual switch.
The current design returns these NICs with their URIs. This is inconsistent to other list functions that return resources as their corresponding Python object (`Nic` in this case).

Proposal: Return the connected NICs as a list of `Nic` objects that have just their `object-uri` property set.

Discussion:
coveralls: 
[![Coverage Status](https://coveralls.io/builds/7812489/badge)](https://coveralls.io/builds/7812489)

Coverage increased (+0.2%) to 88.541% when pulling **f318fb7821b780b3fb20401d803f5f6bebaf9507 on andy/connected-nics-as-objects** into **5b90883d9934257d21ead1cbc548423b5fb682f8 on master**.

andy-maier: Rebased.
coveralls: 
[![Coverage Status](https://coveralls.io/builds/7857812/badge)](https://coveralls.io/builds/7857812)

Coverage increased (+0.1%) to 90.496% when pulling **609404d34172fa9aea9e971b49967a1f4ae12c83 on andy/connected-nics-as-objects** into **6fffd1db48f3cf0463bbd95a16838c843125f60c on master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/7862977/badge)](https://coveralls.io/builds/7862977)

Coverage increased (+0.2%) to 90.564% when pulling **54d6c473093a419b9307520f057cfd6990aad2fe on andy/connected-nics-as-objects** into **6fffd1db48f3cf0463bbd95a16838c843125f60c on master**.

andy-maier: +1

Patch:
diff --git a/zhmcclient/_nic.py b/zhmcclient/_nic.py
index 10b65a7..c59be21 100644
--- a/zhmcclient/_nic.py
+++ b/zhmcclient/_nic.py
@@ -124,6 +124,38 @@ class NicManager(BaseManager):
         props.update(result)
         return Nic(self, props['element-uri'], props)
 
+    def nic_object(self, nic_id):
+        """
+        Return a minimalistic :class:`~zhmcclient.Nic` object for a Nic in this
+        Partition.
+
+        This method is an internal helper function and is not normally called
+        by users.
+
+        This object will be connected in the Python object tree representing
+        the resources (i.e. it has this Partition as a parent), and will have
+        the following properties set:
+
+          * `element-uri`
+          * `element-id`
+          * `parent`
+          * `class`
+
+        Parameters:
+
+            nic_id (string): `element-id` of the Nic
+
+        Returns:
+
+            :class:`~zhmcclient.Nic`: A Python object representing the Nic.
+        """
+        part_uri = self.parent.uri
+        nic_uri = part_uri + "/nics/" + nic_id
+        return Nic(self, nic_uri, {'element-uri': nic_uri,
+                                   'element-id': nic_id,
+                                   'parent': part_uri,
+                                   'class': 'nic'})
+
 
 class Nic(BaseResource):
     """
diff --git a/zhmcclient/_partition.py b/zhmcclient/_partition.py
index e8a337d..1610aff 100644
--- a/zhmcclient/_partition.py
+++ b/zhmcclient/_partition.py
@@ -136,6 +136,38 @@ class PartitionManager(BaseManager):
         props.update(result)
         return Partition(self, props['object-uri'], props)
 
+    def partition_object(self, part_id):
+        """
+        Return a minimalistic :class:`~zhmcclient.Partition` object for a
+        Partition in this CPC.
+
+        This method is an internal helper function and is not normally called
+        by users.
+
+        This object will be connected in the Python object tree representing
+        the resources (i.e. it has this CPC as a parent), and will have the
+        following properties set:
+
+          * `object-uri`
+          * `object-id`
+          * `parent`
+          * `class`
+
+        Parameters:
+
+            part_id (string): `object-id` of the Partition
+
+        Returns:
+
+            :class:`~zhmcclient.Partition`: A Python object representing the
+            Partition.
+        """
+        part_uri = "/api/partitions/" + part_id
+        return Partition(self, part_uri, {'object-uri': part_uri,
+                                          'object-id': part_id,
+                                          'parent': self.parent.uri,
+                                          'class': 'partition'})
+
 
 class Partition(BaseResource):
     """
diff --git a/zhmcclient/_virtual_switch.py b/zhmcclient/_virtual_switch.py
index 3a8a066..1add1b7 100644
--- a/zhmcclient/_virtual_switch.py
+++ b/zhmcclient/_virtual_switch.py
@@ -26,6 +26,8 @@ Virtual Switches only exist in CPCs that are in DPM mode.
 
 from __future__ import absolute_import
 
+import re
+
 from ._manager import BaseManager
 from ._resource import BaseResource
 
@@ -127,11 +129,18 @@ class VirtualSwitch(BaseResource):
 
     def get_connected_vnics(self):
         """
-        List the NICs connected to this Virtual Switch.
+        List the :term:`NICs <NIC>` connected to this Virtual Switch.
 
         Returns:
 
-          : A list of NIC URIs.
+          : A list of :term:`Nic` objects. These objects will be connected in
+          the resource tree (i.e. have a parent :term:`Partition` object,
+          etc.) and will have the following properties set:
+
+          * `element-uri`
+          * `element-id`
+          * `parent`
+          * `class`
 
         Raises:
 
@@ -141,10 +150,24 @@ class VirtualSwitch(BaseResource):
           :exc:`~zhmcclient.ConnectionError`
         """
         vswitch_uri = self.get_property('object-uri')
-        status = self.manager.session.get(
-            vswitch_uri +
-            '/operations/get-connected-vnics')
-        return status['connected-vnic-uris']
+        result = self.manager.session.get(
+            vswitch_uri + '/operations/get-connected-vnics')
+        nic_uris = result['connected-vnic-uris']
+        nic_list = []
+        parts = {}  # Key: Partition ID; Value: Partition object
+        for nic_uri in nic_uris:
+            m = re.match(r"^/api/partitions/([^/]+)/nics/([^/]+)/?$", nic_uri)
+            part_id = m.group(1)
+            nic_id = m.group(2)
+            # We remember created Partition objects and reuse them.
+            try:
+                part = parts[part_id]
+            except KeyError:
+                part = self.manager.cpc.partitions.partition_object(part_id)
+                parts[part_id] = part
+            nic = part.nics.nic_object(nic_id)
+            nic_list.append(nic)
+        return nic_list
 
     def update_properties(self, properties):
         """


Test patch:
diff --git a/tests/test_nic.py b/tests/test_nic.py
index a5d01e7..a1ba6f5 100644
--- a/tests/test_nic.py
+++ b/tests/test_nic.py
@@ -230,5 +230,24 @@ class NicTests(unittest.TestCase):
             status = nic.update_properties(properties={})
             self.assertEqual(status, None)
 
+    def test_nic_object(self):
+        """
+        This tests the `nic_object()` method.
+        """
+        nic_mgr = self.partition.nics
+        nic_id = 'fake-nic-id0711'
+
+        nic = nic_mgr.nic_object(nic_id)
+
+        nic_uri = self.partition.uri + "/nics/" + nic_id
+
+        self.assertTrue(isinstance(nic, Nic))
+        self.assertEqual(nic.uri, nic_uri)
+        self.assertEqual(nic.properties['element-uri'], nic_uri)
+        self.assertEqual(nic.properties['element-id'], nic_id)
+        self.assertEqual(nic.properties['class'], 'nic')
+        self.assertEqual(nic.properties['parent'], self.partition.uri)
+
+
 if __name__ == '__main__':
     unittest.main()
diff --git a/tests/test_partition.py b/tests/test_partition.py
index 0d12b2d..5e488dd 100644
--- a/tests/test_partition.py
+++ b/tests/test_partition.py
@@ -434,5 +434,24 @@ class PartitionTests(unittest.TestCase):
             status = partition.unmount_iso_image()
             self.assertEqual(status, None)
 
+    def test_partition_object(self):
+        """
+        This tests the `partition_object()` method.
+        """
+        partition_mgr = self.cpc.partitions
+        partition_id = 'fake-partition-id42'
+
+        partition = partition_mgr.partition_object(partition_id)
+
+        partition_uri = "/api/partitions/" + partition_id
+
+        self.assertTrue(isinstance(partition, Partition))
+        self.assertEqual(partition.uri, partition_uri)
+        self.assertEqual(partition.properties['object-uri'], partition_uri)
+        self.assertEqual(partition.properties['object-id'], partition_id)
+        self.assertEqual(partition.properties['class'], 'partition')
+        self.assertEqual(partition.properties['parent'], self.cpc.uri)
+
+
 if __name__ == '__main__':
     unittest.main()
diff --git a/tests/test_virtual_switch.py b/tests/test_virtual_switch.py
index 5f7a81c..5109b8d 100644
--- a/tests/test_virtual_switch.py
+++ b/tests/test_virtual_switch.py
@@ -20,9 +20,10 @@ Unit tests for _virtual_switch module.
 from __future__ import absolute_import
 
 import unittest
+import re
 import requests_mock
 
-from zhmcclient import Session, Client
+from zhmcclient import Session, Client, Nic
 
 
 class VirtualSwitchTests(unittest.TestCase):
@@ -224,8 +225,20 @@ class VirtualSwitchTests(unittest.TestCase):
                 "/api/virtual-switches/fake-vswitch-id1/"
                 "operations/get-connected-vnics",
                 json=result)
-            status = vswitch.get_connected_vnics()
-            self.assertEqual(status, result['connected-vnic-uris'])
+
+            nics = vswitch.get_connected_vnics()
+
+            self.assertTrue(isinstance(nics, list))
+            for i, nic in enumerate(nics):
+                self.assertTrue(isinstance(nic, Nic))
+                nic_uri = result['connected-vnic-uris'][i]
+                self.assertEqual(nic.uri, nic_uri)
+                self.assertEqual(nic.properties['element-uri'], nic_uri)
+                m = re.match(r"^/api/partitions/([^/]+)/nics/([^/]+)/?$",
+                             nic_uri)
+                nic_id = m.group(2)
+                self.assertEqual(nic.properties['element-id'], nic_id)
+
 
 if __name__ == '__main__':
     unittest.main()