)]}'
{"nova/compute/api.py":[{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"change_message_id":"a12b7d804d4862e2ea27f8e156eee34ce884e41b","unresolved":false,"context_lines":[{"line_number":1509,"context_line":"                    instance,"},{"line_number":1510,"context_line":"                    task_state\u003dtask_states.UPDATING_PASSWORD)"},{"line_number":1511,"context_line":""},{"line_number":1512,"context_line":"        self.compute_rpcapi.set_admin_password(context, instance\u003dinstance,"},{"line_number":1513,"context_line":"                new_pass\u003dpassword)"},{"line_number":1514,"context_line":""},{"line_number":1515,"context_line":"    @wrap_check_policy"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAALH%2F%2F7T4%3D","side":"PARENT","line":1512,"updated":"2012-08-01 14:33:48.000000000","message":"I\u0027m not a fan of unnecessary whitespace changes, but it\u0027s not worth a -1","commit_id":"37dee20d2f963ca08e56a95be8c654e1920e4a2a"}],"nova/compute/manager.py":[{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"45fb27481bc84cd7acee1e7aa3a3e571c17e707a","unresolved":false,"context_lines":[{"line_number":1231,"context_line":"                                                       reason\u003d_msg)"},{"line_number":1232,"context_line":"                    time.sleep(1)"},{"line_number":1233,"context_line":"                    continue"},{"line_number":1234,"context_line":"        return True"},{"line_number":1235,"context_line":""},{"line_number":1236,"context_line":"    @exception.wrap_exception(notifier\u003dnotifier, publisher_id\u003dpublisher_id())"},{"line_number":1237,"context_line":"    @checks_instance_lock"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAKX%2F%2F1rU%3D","line":1234,"updated":"2012-07-25 09:33:34.000000000","message":"Hmm, I don\u0027t see where we ever return False - do we?\n\nIf not, the return value isn\u0027t interesting","commit_id":"77326f809bfb0996e4b6868912ccd3a5c7e807dc"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"3a0a4fb6b42c3a08e46d7f3e04fa11b885508d11","unresolved":false,"context_lines":[{"line_number":1231,"context_line":"                                                       reason\u003d_msg)"},{"line_number":1232,"context_line":"                    time.sleep(1)"},{"line_number":1233,"context_line":"                    continue"},{"line_number":1234,"context_line":"        return True"},{"line_number":1235,"context_line":""},{"line_number":1236,"context_line":"    @exception.wrap_exception(notifier\u003dnotifier, publisher_id\u003dpublisher_id())"},{"line_number":1237,"context_line":"    @checks_instance_lock"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAKn%2F%2F%2F5g%3D","line":1234,"in_reply_to":"AAAAKX%2F%2F1rU%3D","updated":"2012-07-25 09:56:27.000000000","message":"Bah, I had figured out the answer to this and then forgot it again.\n\nset_admin_password() can fail if instance locking fails, so you\u0027re adding the boolean return here too?","commit_id":"77326f809bfb0996e4b6868912ccd3a5c7e807dc"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"0e529b151506f1dd08b347975932d4f51a567319","unresolved":false,"context_lines":[{"line_number":1231,"context_line":"                                                       reason\u003d_msg)"},{"line_number":1232,"context_line":"                    time.sleep(1)"},{"line_number":1233,"context_line":"                    continue"},{"line_number":1234,"context_line":"        return True"},{"line_number":1235,"context_line":""},{"line_number":1236,"context_line":"    @exception.wrap_exception(notifier\u003dnotifier, publisher_id\u003dpublisher_id())"},{"line_number":1237,"context_line":"    @checks_instance_lock"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAKn%2F%2F%2F5o%3D","line":1234,"in_reply_to":"AAAAKX%2F%2F1rU%3D","updated":"2012-07-25 10:04:18.000000000","message":"Yes, its either an exception to indicate failure, else a return of true.\n\nThe intention was to always provide some payload in the RPC response, even if as you point out, success may be inferred by the client in the absence of an exception.\n\nBut your point is well made, so the superfluous return value is removed in the latest patch set.","commit_id":"77326f809bfb0996e4b6868912ccd3a5c7e807dc"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"9e493af905d7ac121f370716f44384df88b5bd74","unresolved":false,"context_lines":[{"line_number":297,"context_line":"class ComputeManager(manager.SchedulerDependentManager):"},{"line_number":298,"context_line":"    \"\"\"Manages the running instances from creation to destruction.\"\"\""},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    RPC_API_VERSION \u003d \u00271.14\u0027"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def __init__(self, compute_driver\u003dNone, *args, **kwargs):"},{"line_number":303,"context_line":"        \"\"\"Load configuration options and connect to the hypervisor.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F%2BVQ%3D","line":300,"updated":"2012-07-27 14:59:33.000000000","message":"Ok, I hate to do this but ...\n\nYou don\u0027t actually need to bump the minor version here just for a change from cast to call\n\nIf you added a new return value or new behaviour that the client depended on, then it would make sense ... but you\u0027re not doing that here\n\nI don\u0027t feel too bad - you\u0027d have to rebase onto Russell\u0027s compute version updates anyway :)","commit_id":"f37a6d4a08027f4916fc7a4208dfd0d9c500032d"},{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"change_message_id":"0146727db949d5c974f41913499ed5d63202387d","unresolved":false,"context_lines":[{"line_number":297,"context_line":"class ComputeManager(manager.SchedulerDependentManager):"},{"line_number":298,"context_line":"    \"\"\"Manages the running instances from creation to destruction.\"\"\""},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    RPC_API_VERSION \u003d \u00271.14\u0027"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def __init__(self, compute_driver\u003dNone, *args, **kwargs):"},{"line_number":303,"context_line":"        \"\"\"Load configuration options and connect to the hypervisor.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F9M0%3D","line":300,"in_reply_to":"AAAALH%2F%2F%2BVQ%3D","updated":"2012-07-30 13:47:27.000000000","message":"I think the version bump may be warranted here.  It\u0027s not adding a return value, but it is depending on the exception being raised in the case of an error.  The exception won\u0027t make it back to the other side if it\u0027s a cast().","commit_id":"f37a6d4a08027f4916fc7a4208dfd0d9c500032d"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"b57f6cc72fe82846f3a590b0c64f8ca9b56d1a97","unresolved":false,"context_lines":[{"line_number":297,"context_line":"class ComputeManager(manager.SchedulerDependentManager):"},{"line_number":298,"context_line":"    \"\"\"Manages the running instances from creation to destruction.\"\"\""},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    RPC_API_VERSION \u003d \u00271.14\u0027"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def __init__(self, compute_driver\u003dNone, *args, **kwargs):"},{"line_number":303,"context_line":"        \"\"\"Load configuration options and connect to the hypervisor.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F9LA%3D","line":300,"in_reply_to":"AAAALH%2F%2F9M0%3D","updated":"2012-07-30 14:31:29.000000000","message":"There\u0027s nothing new on the client side requiring an exception - you basically just need to update the server side with the code to add the exception and the issue is resolved. This is a pure server-side bugfix.\n\nWe don\u0027t want to bump the minor version every time a server-side bug is fixed","commit_id":"f37a6d4a08027f4916fc7a4208dfd0d9c500032d"},{"author":{"_account_id":1711,"name":"Yun Mao","email":"yunmao@gmail.com","username":"yunmao"},"change_message_id":"ecb9885d71c2df5a3d642b688f59feeaca51fcad","unresolved":false,"context_lines":[{"line_number":1263,"context_line":"                                          instance[\u0027uuid\u0027],"},{"line_number":1264,"context_line":"                                          task_state\u003dNone)"},{"line_number":1265,"context_line":"                    break"},{"line_number":1266,"context_line":"                except NotImplementedError, e:"},{"line_number":1267,"context_line":"                    # NOTE(dprince): if the driver doesn\u0027t implement"},{"line_number":1268,"context_line":"                    # set_admin_password we break to avoid a loop"},{"line_number":1269,"context_line":"                    _msg \u003d _(\u0027set_admin_password is not implemented \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAALH%2F%2F7jk%3D","line":1266,"updated":"2012-08-01 00:49:29.000000000","message":"where is e used?","commit_id":"bae85dfe0c924a894cd82a76716a8d52e4b0c290"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"d3d1ca15fd77c5712de6c3d6e741af4b48da9644","unresolved":false,"context_lines":[{"line_number":1263,"context_line":"                                          instance[\u0027uuid\u0027],"},{"line_number":1264,"context_line":"                                          task_state\u003dNone)"},{"line_number":1265,"context_line":"                    break"},{"line_number":1266,"context_line":"                except NotImplementedError, e:"},{"line_number":1267,"context_line":"                    # NOTE(dprince): if the driver doesn\u0027t implement"},{"line_number":1268,"context_line":"                    # set_admin_password we break to avoid a loop"},{"line_number":1269,"context_line":"                    _msg \u003d _(\u0027set_admin_password is not implemented \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAALH%2F%2F7ZM%3D","line":1266,"in_reply_to":"AAAALH%2F%2F7jk%3D","updated":"2012-08-01 09:46:31.000000000","message":"Good spot.\n\nThat\u0027s a throwback to a previous version of the patch, when IIRC I was using the NotImplementedError in the log message.\n\nSince this is no longer the case, I\u0027ve removed it in the latest patch set.","commit_id":"bae85dfe0c924a894cd82a76716a8d52e4b0c290"},{"author":{"_account_id":1711,"name":"Yun Mao","email":"yunmao@gmail.com","username":"yunmao"},"change_message_id":"ecb9885d71c2df5a3d642b688f59feeaca51fcad","unresolved":false,"context_lines":[{"line_number":1277,"context_line":"                                                       reason\u003d_msg)"},{"line_number":1278,"context_line":"                except Exception, e:"},{"line_number":1279,"context_line":"                    # Catch all here because this could be anything."},{"line_number":1280,"context_line":"                    LOG.exception(e, instance\u003dinstance)"},{"line_number":1281,"context_line":"                    if i \u003d\u003d max_tries - 1:"},{"line_number":1282,"context_line":"                        self._set_instance_error_state(context,"},{"line_number":1283,"context_line":"                                                       instance[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAALH%2F%2F7jU%3D","line":1280,"updated":"2012-08-01 00:49:29.000000000","message":"LOG.exception should not take an exception object. it should be a msg instead.","commit_id":"bae85dfe0c924a894cd82a76716a8d52e4b0c290"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"d3d1ca15fd77c5712de6c3d6e741af4b48da9644","unresolved":false,"context_lines":[{"line_number":1277,"context_line":"                                                       reason\u003d_msg)"},{"line_number":1278,"context_line":"                except Exception, e:"},{"line_number":1279,"context_line":"                    # Catch all here because this could be anything."},{"line_number":1280,"context_line":"                    LOG.exception(e, instance\u003dinstance)"},{"line_number":1281,"context_line":"                    if i \u003d\u003d max_tries - 1:"},{"line_number":1282,"context_line":"                        self._set_instance_error_state(context,"},{"line_number":1283,"context_line":"                                                       instance[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAALH%2F%2F7ZI%3D","line":1280,"in_reply_to":"AAAALH%2F%2F7jU%3D","updated":"2012-08-01 09:46:31.000000000","message":"That\u0027s pre-existing code, but I\u0027ve fixed it none-the-less in the latest patch set.","commit_id":"bae85dfe0c924a894cd82a76716a8d52e4b0c290"},{"author":{"_account_id":1711,"name":"Yun Mao","email":"yunmao@gmail.com","username":"yunmao"},"change_message_id":"23eb6aef4bdc82e26f5b9f776f050d781d62850f","unresolved":false,"context_lines":[{"line_number":1288,"context_line":"                        raise exception.InstancePasswordSetFailed("},{"line_number":1289,"context_line":"                                                       instance\u003dinstance_uuid,"},{"line_number":1290,"context_line":"                                                       reason\u003d_msg)"},{"line_number":1291,"context_line":"                    time.sleep(1)"},{"line_number":1292,"context_line":"                    continue"},{"line_number":1293,"context_line":""},{"line_number":1294,"context_line":"    @exception.wrap_exception(notifier\u003dnotifier, publisher_id\u003dpublisher_id())"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAALH%2F%2F7jM%3D","line":1291,"updated":"2012-08-01 00:54:11.000000000","message":"I know this has little to do with this patch, but does anyone know why we sleep 1 here? seems rather arbitrary.","commit_id":"bae85dfe0c924a894cd82a76716a8d52e4b0c290"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"d3d1ca15fd77c5712de6c3d6e741af4b48da9644","unresolved":false,"context_lines":[{"line_number":1288,"context_line":"                        raise exception.InstancePasswordSetFailed("},{"line_number":1289,"context_line":"                                                       instance\u003dinstance_uuid,"},{"line_number":1290,"context_line":"                                                       reason\u003d_msg)"},{"line_number":1291,"context_line":"                    time.sleep(1)"},{"line_number":1292,"context_line":"                    continue"},{"line_number":1293,"context_line":""},{"line_number":1294,"context_line":"    @exception.wrap_exception(notifier\u003dnotifier, publisher_id\u003dpublisher_id())"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAALH%2F%2F7ZE%3D","line":1291,"in_reply_to":"AAAALH%2F%2F7jM%3D","updated":"2012-08-01 09:46:31.000000000","message":"Agreed, it does seem arbitrary, but not related to the current patch so I\u0027m minded to leave it alone for now.","commit_id":"bae85dfe0c924a894cd82a76716a8d52e4b0c290"},{"author":{"_account_id":1711,"name":"Yun Mao","email":"yunmao@gmail.com","username":"yunmao"},"change_message_id":"e1d95abf7cedf8d58bf939658469554882fc26c7","unresolved":false,"context_lines":[{"line_number":1265,"context_line":"                                                       reason\u003d_msg)"},{"line_number":1266,"context_line":"                except Exception, e:"},{"line_number":1267,"context_line":"                    # Catch all here because this could be anything."},{"line_number":1268,"context_line":"                    LOG.exception(_(\u0027set_admin_password failed: %s\u0027) % e,"},{"line_number":1269,"context_line":"                                  instance\u003dinstance)"},{"line_number":1270,"context_line":"                    if i \u003d\u003d max_tries - 1:"},{"line_number":1271,"context_line":"                        self._set_instance_error_state(context,"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAALH%2F%2F7Ww%3D","line":1268,"updated":"2012-08-01 12:51:31.000000000","message":"I think LOG.exception will automatically pick up and log the exception in an exception handler, so %e is not really necessary. But this is ok with me.","commit_id":"3ab808b23d17900cc859b75895ac1c7a8f2d7bb9"}],"nova/compute/rpcapi.py":[{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"change_message_id":"a12b7d804d4862e2ea27f8e156eee34ce884e41b","unresolved":false,"context_lines":[{"line_number":400,"context_line":""},{"line_number":401,"context_line":"    def set_admin_password(self, ctxt, instance, new_pass):"},{"line_number":402,"context_line":"        instance_p \u003d jsonutils.to_primitive(instance)"},{"line_number":403,"context_line":"        return self.call(ctxt, self.make_msg(\u0027set_admin_password\u0027,"},{"line_number":404,"context_line":"                instance\u003dinstance_p, new_pass\u003dnew_pass),"},{"line_number":405,"context_line":"                topic\u003d_compute_topic(self.topic, ctxt, None, instance),"},{"line_number":406,"context_line":"                version\u003d\u00271.33\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAALH%2F%2F7Tw%3D","line":403,"updated":"2012-08-01 14:33:48.000000000","message":"The return isn\u0027t really necessary here.  As a user of this API I would wonder what it returns.  Maybe we should leave a comment saying \"Using call just so this is synchronous.  There is no return value.\"","commit_id":"3ab808b23d17900cc859b75895ac1c7a8f2d7bb9"}],"nova/exception.py":[{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"change_message_id":"a12b7d804d4862e2ea27f8e156eee34ce884e41b","unresolved":false,"context_lines":[{"line_number":1088,"context_line":"class InstancePasswordSetFailed(NovaException):"},{"line_number":1089,"context_line":"    message \u003d _(\"Failed to set admin password on %(instance)s \""},{"line_number":1090,"context_line":"                \"because %(reason)s\")"},{"line_number":1091,"context_line":"    safe \u003d True"},{"line_number":1092,"context_line":""},{"line_number":1093,"context_line":""},{"line_number":1094,"context_line":"class SolidFireAPIException(NovaException):"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAALH%2F%2F7To%3D","line":1091,"updated":"2012-08-01 14:33:48.000000000","message":"It would be nice to have a comment somewhere in nova.exception that explains what \"safe\" is for.  I had to grep the code for it.","commit_id":"3ab808b23d17900cc859b75895ac1c7a8f2d7bb9"}]}
