Merge lp:~justin-fathomdb/nova/justinsb-api-fix-tolerate-no-ip into lp:~hudson-openstack/nova/trunk

[justinsb]

The describe servers call was throwing an error due to subscripting on None, if there was no fixed_ip element present

[Jay Pipes (jaypipes)]

Are there any bug reports to track these kinds of patches?

review: Needs Information

[justinsb (justin-fathomdb)]

Filed a bug. However, right now the OpenStack API code is not thoroughly tested, so I don't know that this is a wise requirement unless we want to spend our lives opening and closing trivial bugs.

[justinsb (justin-fathomdb)]

Also, Dan Prince pointed out that this bug has already been reported. Linked the original bug report directly to this branch also.

[Dan Prince (dan-prince)]

I like Justin's solution better. Lets use it.

review: Approve

[Jay Pipes (jaypipes)]

15 + fixed_ip = inst['fixed_ip']

should be:

fixed_ip = inst.get('fixed_ip')

the former will throw a KeyError if fixed_ip is not a key in the dict. The latter will return None.

Similarly, this:

18 + try:
19 + private_ip = fixed_ip['address']
20 + if private_ip:
21 + inst_dict['addresses']['private'].append(private_ip)
22 + except KeyError:
23 + LOG.debug(_("Failed to read private ip"))

Can be rewritten to:

private_ip = fixed_ip.get('address')
if private_ip:
    inst_dict['addresses']['private'].append(private_ip)
else:
    LOG.debug(_("Failed to read private IP"))

Cheers!
jay

review: Needs Fixing

[lp:~justin-fathomdb/nova/justinsb-api-fix-tolerate-no-ip]

702. By justinsb on 2011-02-23

Merged with trunk

[justinsb (justin-fathomdb)]

Thanks Jay - I don't know what I was thinking (or if I was thinking). But it does show that this code is error prone if "someone" isn't thinking :-)

I revised the code; I'm not the world's biggest fan of this approach though, so I also coded up a helper function that may make this cleaner, but (as you suggested on IRC) I've made it a separate patch:
http://bazaar.launchpad.net/~justin-fathomdb/nova/mini-xpath/revision/702

[lp:~justin-fathomdb/nova/justinsb-api-fix-tolerate-no-ip]

703. By justinsb on 2011-02-23

Another case where was using [] when meant .get(). Doh!

Unmerged revisions

703. By justinsb on 2011-02-23

Another case where was using [] when meant .get(). Doh!

702. By justinsb on 2011-02-23

Merged with trunk

=== modified file 'nova/api/openstack/servers.py'
--- nova/api/openstack/servers.py	2011-02-23 18:40:59 +0000
+++ nova/api/openstack/servers.py	2011-02-23 20:06:26 +0000
@@ -62,20 +62,22 @@
     inst_dict['status'] = power_mapping[inst_dict['status']]
     inst_dict['addresses'] = dict(public=[], private=[])
 
-    # grab single private fixed ip
-    try:
-        private_ip = inst['fixed_ip']['address']
+    fixed_ip = inst.get('fixed_ip')
+    if fixed_ip:
+        # grab single private fixed ip
+        private_ip = fixed_ip.get('address')
         if private_ip:
             inst_dict['addresses']['private'].append(private_ip)
-    except KeyError:
-        LOG.debug(_("Failed to read private ip"))
 
-    # grab all public floating ips
-    try:
-        for floating in inst['fixed_ip']['floating_ips']:
-            inst_dict['addresses']['public'].append(floating['address'])
-    except KeyError:
-        LOG.debug(_("Failed to read public ip(s)"))
+        # grab all public floating ips
+        floating_ips = fixed_ip['floating_ips']
+        if floating_ips:
+            for floating_ip in floating_ips:
+                if floating_ip:
+                    floating_ip_address = floating_ip.get('address')
+                    if floating_ip_address:
+                        inst_dict['addresses']['public'].append(
+                                                        floating_ip_address)
 
     inst_dict['hostId'] = ''