Merge lp:~jtran/nova/lp810364 into lp:~hudson-openstack/nova/trunk

[John Tran]

code was checking for key in sqlalchemy instance and will ignore if value is None, but wasn't working if floating_ip was a non-sqlalchemy dict obj. Therefore, updated the error checking to work in both caes.

code was checking for key in sqlalchemy instance and will ignore if value is None, but wasn't working if floating_ip was a non-sqlalchemy dict obj. Therefore, updated the error checking to work in both caes.

[Brian Waldon (bcwaldon)]

What do you think about this code, instead? It will handle floating_ip not being a dictionary and the 'address' key not existing:

   try:
       result['fixed_ip'] = floating_ip['fixed_ip']['address']
   except (TypeError, KeyError):
       result['fixed_ip'] = None

[John Tran (jtran)]

I like that much better. I've updated the branch. Thanks, Brian.

[lp:~jtran/nova/lp810364]

1329. By John Tran on 2011-07-27

improved the code per peer review

[Brian Waldon (bcwaldon)]

Awesome.

review: Approve

[Jason Kölker (jason-koelker)]

Is bueno.

review: Approve

[Rick Harris (rconradharris)]

lgtm

review: Approve

=== modified file 'nova/api/openstack/contrib/floating_ips.py'
--- nova/api/openstack/contrib/floating_ips.py	2011-07-06 20:28:10 +0000
+++ nova/api/openstack/contrib/floating_ips.py	2011-07-27 17:36:42 +0000
@@ -27,9 +27,9 @@
 def _translate_floating_ip_view(floating_ip):
     result = {'id': floating_ip['id'],
               'ip': floating_ip['address']}
-    if 'fixed_ip' in floating_ip:
+    try:
         result['fixed_ip'] = floating_ip['fixed_ip']['address']
-    else:
+    except (TypeError, KeyError):
         result['fixed_ip'] = None
     if 'instance' in floating_ip:
         result['instance_id'] = floating_ip['instance']['id']

=== modified file 'nova/tests/api/openstack/contrib/test_floating_ips.py'
--- nova/tests/api/openstack/contrib/test_floating_ips.py	2011-07-06 20:28:10 +0000
+++ nova/tests/api/openstack/contrib/test_floating_ips.py	2011-07-27 17:36:42 +0000
@@ -111,6 +111,11 @@
         self.assertEqual(view['floating_ip']['fixed_ip'], None)
         self.assertEqual(view['floating_ip']['instance_id'], None)
 
+    def test_translate_floating_ip_view_dict(self):
+        floating_ip = {'id': 0, 'address': '10.0.0.10', 'fixed_ip': None}
+        view = _translate_floating_ip_view(floating_ip)
+        self.assertTrue('floating_ip' in view)
+
     def test_floating_ips_list(self):
         req = webob.Request.blank('/v1.1/os-floating-ips')
         res = req.get_response(fakes.wsgi_app())