)]}'
{"shade/meta.py":[{"author":{"_account_id":6488,"name":"Clint Byrum","email":"clint@fewbar.com","username":"clint-fewbar"},"change_message_id":"2827766bf1be915e765c7918b1c072eac6ec3643","unresolved":false,"context_lines":[{"line_number":62,"context_line":"    return addrs[0]"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"def get_server_private_ip(cloud, server):"},{"line_number":66,"context_line":"    \"\"\"Find the private IP address"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    If Neutron is available, search for a port on a network where"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a4dcd0f_5cf1a7cc","line":65,"updated":"2015-08-03 20:47:07.000000000","message":"This is an API breakage on a public method. WOuld be good to make cloud second, and optional.","commit_id":"f96e787288007906268860d1c3f64d2c05ffa4a9"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"29627ffa7fc2945f20092f8a9208280390f5a278","unresolved":false,"context_lines":[{"line_number":62,"context_line":"    return addrs[0]"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"def get_server_private_ip(cloud, server):"},{"line_number":66,"context_line":"    \"\"\"Find the private IP address"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    If Neutron is available, search for a port on a network where"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a4dcd0f_1c047f7c","line":65,"in_reply_to":"1a4dcd0f_5cf1a7cc","updated":"2015-08-04 12:05:12.000000000","message":"Good point. Although the meta methods were never meant to be public, that doesn\u0027t mean someone hasn\u0027t used them (i.e., the Ansible modules). The intent was to match the parameters of get_server_external_ipv4().","commit_id":"f96e787288007906268860d1c3f64d2c05ffa4a9"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"3b0c9347c75ffb9863e8fd64db0b87720cdf72cc","unresolved":false,"context_lines":[{"line_number":87,"context_line":"            log.debug("},{"line_number":88,"context_line":"                \"Something went wrong talking to neutron API: \""},{"line_number":89,"context_line":"                \"\u0027{msg}\u0027. Trying with Nova.\".format(msg\u003dstr(e)))"},{"line_number":90,"context_line":"        else:"},{"line_number":91,"context_line":"            for net in nets:"},{"line_number":92,"context_line":"                for port in server_ports:"},{"line_number":93,"context_line":"                    if net[\u0027id\u0027] \u003d\u003d port[\u0027network_id\u0027]:"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_e87995fd","line":90,"updated":"2015-08-04 18:09:31.000000000","message":"This else block appears to be overindented by one level.","commit_id":"80e1fbcd9072645c040a53cd0324e2596efa567f"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"e7717e03d631e099f07f18f4e4a4df544841bd7a","unresolved":false,"context_lines":[{"line_number":87,"context_line":"            log.debug("},{"line_number":88,"context_line":"                \"Something went wrong talking to neutron API: \""},{"line_number":89,"context_line":"                \"\u0027{msg}\u0027. Trying with Nova.\".format(msg\u003dstr(e)))"},{"line_number":90,"context_line":"        else:"},{"line_number":91,"context_line":"            for net in nets:"},{"line_number":92,"context_line":"                for port in server_ports:"},{"line_number":93,"context_line":"                    if net[\u0027id\u0027] \u003d\u003d port[\u0027network_id\u0027]:"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_aee715e3","line":90,"in_reply_to":"1a4dcd0f_2b1e9763","updated":"2015-08-04 18:50:03.000000000","message":"I see, I had red this as if cloud and has service network (eg neutron) then do neutron things else do nova network things.\n\nThis method reads really weirdly as it does the other thing but that may just be me? Why not:\n\n  try:\n    do all the try things here\n  except:\n    do the nova network things here","commit_id":"80e1fbcd9072645c040a53cd0324e2596efa567f"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"c70d90181408dc9589e15655e260f5d4670537fe","unresolved":false,"context_lines":[{"line_number":87,"context_line":"            log.debug("},{"line_number":88,"context_line":"                \"Something went wrong talking to neutron API: \""},{"line_number":89,"context_line":"                \"\u0027{msg}\u0027. Trying with Nova.\".format(msg\u003dstr(e)))"},{"line_number":90,"context_line":"        else:"},{"line_number":91,"context_line":"            for net in nets:"},{"line_number":92,"context_line":"                for port in server_ports:"},{"line_number":93,"context_line":"                    if net[\u0027id\u0027] \u003d\u003d port[\u0027network_id\u0027]:"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_828a25be","line":90,"in_reply_to":"1a4dcd0f_474c2f70","updated":"2015-08-04 22:31:16.000000000","message":"The problem is you essentially have this control flow if I read it correctly:\n\n  if foo:\n      try:\n          foo step 1\n      except:\n          except step 1\n      else:\n          foo step 2\n          return\n  except step 2\n\nWhich is interleaved and incredible hard to follow. I have removed my -1 because its not technically wrong but I do worry this will be hard to maintain as networking changes in openstack.\n\nAlso, I think the only exceptions the for loop blocks can raise is StopIteration exceptions that the for loops deal with. So it should be fine to move it up.\n\nIf I were to rewrite it it would look something like:\n\n  if neutron:\n      try:\n          do neutron things\n      except neutronfail:\n          handle_nova()\n  else:\n      handle_nova()\n\nAs that very readably says, attempt to use neutron if we have neutron. If we fail to use neutron then use nova, and if we do not have neutron use nova.","commit_id":"80e1fbcd9072645c040a53cd0324e2596efa567f"},{"author":{"_account_id":6488,"name":"Clint Byrum","email":"clint@fewbar.com","username":"clint-fewbar"},"change_message_id":"0d240c8f5fd70e801370926bf41a7ec794fe508d","unresolved":false,"context_lines":[{"line_number":87,"context_line":"            log.debug("},{"line_number":88,"context_line":"                \"Something went wrong talking to neutron API: \""},{"line_number":89,"context_line":"                \"\u0027{msg}\u0027. Trying with Nova.\".format(msg\u003dstr(e)))"},{"line_number":90,"context_line":"        else:"},{"line_number":91,"context_line":"            for net in nets:"},{"line_number":92,"context_line":"                for port in server_ports:"},{"line_number":93,"context_line":"                    if net[\u0027id\u0027] \u003d\u003d port[\u0027network_id\u0027]:"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_474c2f70","line":90,"in_reply_to":"1a4dcd0f_aee715e3","updated":"2015-08-04 21:48:56.000000000","message":"The reason not to do the for loop inside that try is that nothing went wrong talking to the Neutron API in this case, so the log entry would be wrong. If there is an exception, it was all deeper issues, so you would want a different exception handler (or, as we have here, none at all because we don\u0027t expect any errors from that code and any failure would be a massive programming fail most likely).","commit_id":"80e1fbcd9072645c040a53cd0324e2596efa567f"},{"author":{"_account_id":6133,"name":"yolanda.robla","email":"yroblamo@redhat.com","username":"yolanda.robla"},"change_message_id":"fbc171108804f476da374f2d927b749435f1fe28","unresolved":false,"context_lines":[{"line_number":87,"context_line":"            log.debug("},{"line_number":88,"context_line":"                \"Something went wrong talking to neutron API: \""},{"line_number":89,"context_line":"                \"\u0027{msg}\u0027. Trying with Nova.\".format(msg\u003dstr(e)))"},{"line_number":90,"context_line":"        else:"},{"line_number":91,"context_line":"            for net in nets:"},{"line_number":92,"context_line":"                for port in server_ports:"},{"line_number":93,"context_line":"                    if net[\u0027id\u0027] \u003d\u003d port[\u0027network_id\u0027]:"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_2b1e9763","line":90,"in_reply_to":"1a4dcd0f_e87995fd","updated":"2015-08-04 18:13:26.000000000","message":"That\u0027s intentional...\nThe try ... except statement has an optional else clause, which, when present, must follow all except clauses. It is useful for code that must be executed if the try clause does not raise an exception.","commit_id":"80e1fbcd9072645c040a53cd0324e2596efa567f"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"3b0c9347c75ffb9863e8fd64db0b87720cdf72cc","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        return ip"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    # Last resort, and Rackspace"},{"line_number":103,"context_line":"    return get_server_ip(server, key_name\u003d\u0027private\u0027)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"def get_server_external_ipv4(cloud, server):"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_68dca5e4","line":103,"updated":"2015-08-04 18:09:31.000000000","message":"If we are going to return on line 96 for the nova case should the code here for the neutron case be moved up into the if block above? Worried that we can possibly do neutron, fail to nova, then fail to neutron which seems redundant and broken.","commit_id":"80e1fbcd9072645c040a53cd0324e2596efa567f"}]}