Code review comment for lp:~tylesmit/neutron/lp845140

Hi Tyler.
As already stated by Dan this mergeprop came a bit too close to the deadline. I have a few comments for you and I hope you can address them before rbp is released.

None of the issues I found appear to break the plugin, but my judgement is merely based on static code analysis. Moreover, I would not consider this review that I've done as 'accurate'; however since the risk is 'contained' to the Cisco plugin, I'm willing to approve it.

29 for device_ip in device_ips:
30 new_device_params = deepcopy(device_params)
Looks like there no need to perfom the deepcopy operation in the loop.

31 new_device_params[const.DEVICE_IP] = device_ip
At a first glance this seems a workaround needed as _invoke_plugin with the specific function used in this case does not accept a list of IPs. Probably the ideal approach would be to update _invoke_plugin to deal with multiple ips. But this is not a problem at all.

226 + """Tear down our tests"""
227 + try:
228 + self._l2network_multiblade.delete_port([tenant_id, self.net_id,
229 + self.port_id])
230 + except exc.PortNotFound:
231 + # We won't always have a port to remove
232 + pass
233 +
234 + try:
235 + self._l2network_multiblade.delete_network([tenant_id, self.net_id])
236 + except exc.NetworkNotFound:
237 + # We won't always have a network to remove
238 + pass

The above is probably not necessary as the last line in tearDown destroys all the contents of the test database.

538 +class TestUCSInventory(unittest.TestCase):

This class has not teardown method for clearing the database.

1544 + def tearDown(self):
1545 + """Clear the test environment"""
1546 + # Remove database contents
1547 + db.clear_db()

Not very useful, but probably still good to have, as this class has methods for tearing down reasources created in each unit test.