)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"49b54188850cfee770d36ea73c0a36ccaa69610b","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        try:"},{"line_number":742,"context_line":"            image_href \u003d instance.get(\u0027image_ref\u0027)"},{"line_number":743,"context_line":"            image_meta \u003d _get_image_meta(context, image_href)"},{"line_number":744,"context_line":"        except:"},{"line_number":745,"context_line":"            msg \u003d _(\"Cannot rebuild instance [%s] with bad image [%s]\")"},{"line_number":746,"context_line":"            LOG.error(msg % (instance_uuid, image_href))"},{"line_number":747,"context_line":"            self._instance_update(context,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEH%2F%2F%2B1s%3D","line":744,"updated":"2012-01-24 19:18:22.000000000","message":"Can you catch a specific exception here?","commit_id":"d3eac6ba060dfb85ca46f7b2344019e139f888f9"},{"author":{"_account_id":998,"name":"Naveed Massjouni","email":"naveedm9@gmail.com","username":"ironcamel"},"change_message_id":"e965f5f82d07f54e23e88b2f19e4e543b71cdbb0","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        try:"},{"line_number":742,"context_line":"            image_href \u003d instance.get(\u0027image_ref\u0027)"},{"line_number":743,"context_line":"            image_meta \u003d _get_image_meta(context, image_href)"},{"line_number":744,"context_line":"        except:"},{"line_number":745,"context_line":"            msg \u003d _(\"Cannot rebuild instance [%s] with bad image [%s]\")"},{"line_number":746,"context_line":"            LOG.error(msg % (instance_uuid, image_href))"},{"line_number":747,"context_line":"            self._instance_update(context,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEH%2F%2F%2Bt4%3D","line":744,"in_reply_to":"AAAAEH%2F%2F%2B1s%3D","updated":"2012-01-24 21:13:34.000000000","message":"This needs to guard against any exception. If we only caught  a specific exception here, and another exception occurred, we would be back to having an undeletable instance in a bad state. I don\u0027t see why catching a certain type (or types) of exceptions would be better in this case.","commit_id":"d3eac6ba060dfb85ca46f7b2344019e139f888f9"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"e0c484fffe7d73ebd7125c4666609b839d82e2ca","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        try:"},{"line_number":742,"context_line":"            image_href \u003d instance.get(\u0027image_ref\u0027)"},{"line_number":743,"context_line":"            image_meta \u003d _get_image_meta(context, image_href)"},{"line_number":744,"context_line":"        except:"},{"line_number":745,"context_line":"            msg \u003d _(\"Cannot rebuild instance [%s] with bad image [%s]\")"},{"line_number":746,"context_line":"            LOG.error(msg % (instance_uuid, image_href))"},{"line_number":747,"context_line":"            self._instance_update(context,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEH%2F%2F%2BsA%3D","line":744,"in_reply_to":"AAAAEH%2F%2F%2Bsg%3D","updated":"2012-01-24 21:36:24.000000000","message":"I do like your proposed solution. As Naveed points out, this is one of those cases that we really do need to catch *everything*. But I also like catching explicit exceptions where we can. I would like to know that we couldn\u0027t find the image vs. there was a KeyError.\n\nTo keep this code cleaner, you could abstract out the update call into a function above this try/except block.","commit_id":"d3eac6ba060dfb85ca46f7b2344019e139f888f9"},{"author":{"_account_id":998,"name":"Naveed Massjouni","email":"naveedm9@gmail.com","username":"ironcamel"},"change_message_id":"401980cdda3f115786aa44e6ec0a5585600f09d9","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        try:"},{"line_number":742,"context_line":"            image_href \u003d instance.get(\u0027image_ref\u0027)"},{"line_number":743,"context_line":"            image_meta \u003d _get_image_meta(context, image_href)"},{"line_number":744,"context_line":"        except:"},{"line_number":745,"context_line":"            msg \u003d _(\"Cannot rebuild instance [%s] with bad image [%s]\")"},{"line_number":746,"context_line":"            LOG.error(msg % (instance_uuid, image_href))"},{"line_number":747,"context_line":"            self._instance_update(context,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEH%2F%2F%2BrQ%3D","line":744,"in_reply_to":"AAAAEH%2F%2F%2Bsg%3D","updated":"2012-01-24 21:45:13.000000000","message":"Thanks Lamar, I like your solution. Waldon, I will try to DRY up the update call as well.","commit_id":"d3eac6ba060dfb85ca46f7b2344019e139f888f9"},{"author":{"_account_id":706,"name":"Brian Lamar","email":"brian.lamar@rackspace.com","username":"blamar"},"change_message_id":"b112a14f4a1f793cd8e5fe848ee7e0429ebe22ec","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        try:"},{"line_number":742,"context_line":"            image_href \u003d instance.get(\u0027image_ref\u0027)"},{"line_number":743,"context_line":"            image_meta \u003d _get_image_meta(context, image_href)"},{"line_number":744,"context_line":"        except:"},{"line_number":745,"context_line":"            msg \u003d _(\"Cannot rebuild instance [%s] with bad image [%s]\")"},{"line_number":746,"context_line":"            LOG.error(msg % (instance_uuid, image_href))"},{"line_number":747,"context_line":"            self._instance_update(context,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEH%2F%2F%2Bsg%3D","line":744,"in_reply_to":"AAAAEH%2F%2F%2Bt4%3D","updated":"2012-01-24 21:30:36.000000000","message":"Yeah, I see what you\u0027re saying...I\u0027m a bit torn between catching the exception we know... and any/all exceptions that might come along.\n\nI personally like catching very specific exceptions and having as little code in try/except blocks as possible. For example, this block could become:\n\n  image_href \u003d instance.get(\u0027image_ref\u0027)\n  try:\n    image_meta \u003d _get_image_meta(context, image_href)\n  except NotFoundException:\n    ...\n    return\n\nCatching specific exceptions allows us to provide specific error messages to users, although I imagine you could do the same by just including str(exception) in the error message. Right now, with this code, when things fail we\u0027re just assuming it\u0027s because of an ImageNotFound, when it actually could be a TypeError, KeyError, SyntaxError, SystemExit, KeyboardInterrupt (control-c), etc.\n\nWould you agree to something like this?\n\n  image_href \u003d instance.get(\u0027image_ref\u0027)\n  try:\n    image_meta \u003d _get_image_meta(context, image_href)\n  except NotFoundException:\n    # Log error message specific to NotFoundException\n    # Set error state of instance\n    return\n  except Exception as exc:\n    # Log generic error message with contents of \u0027exc\u0027\n    # Set error state of instance\n    return\n    \n\nAlthough now that I look at it, all that is doing is duplicating things, but I\u0027ve rambled on for a while so what do you two think?","commit_id":"d3eac6ba060dfb85ca46f7b2344019e139f888f9"},{"author":{"_account_id":177,"name":"Alex Meade","email":"mr.alex.meade@gmail.com","username":"alex-meade"},"change_message_id":"e06946320040292c7185e8bdac5778015866d36d","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        try:"},{"line_number":742,"context_line":"            image_href \u003d instance.get(\u0027image_ref\u0027)"},{"line_number":743,"context_line":"            image_meta \u003d _get_image_meta(context, image_href)"},{"line_number":744,"context_line":"        except:"},{"line_number":745,"context_line":"            msg \u003d _(\"Cannot rebuild instance [%s] with bad image [%s]\")"},{"line_number":746,"context_line":"            LOG.error(msg % (instance_uuid, image_href))"},{"line_number":747,"context_line":"            self._instance_update(context,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEH%2F%2F%2BsU%3D","line":744,"in_reply_to":"AAAAEH%2F%2F%2Bt4%3D","updated":"2012-01-24 21:31:06.000000000","message":"if we are catching any exception, HACKING wants it to be \"except Exception:\" not just \"except:\"","commit_id":"d3eac6ba060dfb85ca46f7b2344019e139f888f9"},{"author":{"_account_id":1419,"name":"Mike Pittaro","email":"mikeyp@LaHondaResearch.org","username":"mikeyp-3"},"change_message_id":"a228db958cc8cce5911ded0c65d3222af356e955","unresolved":false,"context_lines":[{"line_number":748,"context_line":"                                  instance_uuid,"},{"line_number":749,"context_line":"                                  vm_state\u003dvm_states.ERROR,"},{"line_number":750,"context_line":"                                  task_state\u003dNone)"},{"line_number":751,"context_line":"            return"},{"line_number":752,"context_line":""},{"line_number":753,"context_line":"        self.driver.spawn(context, instance, image_meta,"},{"line_number":754,"context_line":"                          network_info, bd_mapping)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEH%2F%2F%2B0I%3D","line":751,"updated":"2012-01-24 19:46:03.000000000","message":"Since the instance status was updated, you _might_ need to call notify_about_instance_usage() to keep the capacity cache in sync.  Check with Sandy Walsh, he can confirm whether this is needed.","commit_id":"d3eac6ba060dfb85ca46f7b2344019e139f888f9"}]}