)]}'
{"nova/tests/virt/libvirt/test_libvirt.py":[{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"64443b728123a181d1f7d9cb2e2f10489507d0f3","unresolved":false,"context_lines":[{"line_number":3362,"context_line":"        self.stubs.Set(conn, \u0027_lookup_by_name\u0027, fake_lookup_by_name)"},{"line_number":3363,"context_line":"        self.stubs.Set(conn, \u0027_hard_reboot\u0027, fake_hard_reboot)"},{"line_number":3364,"context_line":"        conn.reboot(None, instance, [])"},{"line_number":3365,"context_line":"        self.assertTrue(self.reboot_hard_reboot_called)"},{"line_number":3366,"context_line":""},{"line_number":3367,"context_line":"    def test_destroy_undefines(self):"},{"line_number":3368,"context_line":"        mock \u003d self.mox.CreateMock(libvirt.virDomain)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAQn%2F%2FSkE%3D","line":3365,"updated":"2013-08-02 14:16:16.000000000","message":"assertTrue is a pain to debug when it fails. Instead can we make fake_hard_reboot set self._instance or something, then check that self._instance \u003d\u003d the instance you sent down?","commit_id":"5230ba7e48100c6007f6a986712a09f88f67747e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"884d8e4f65d482c0eff0d5294963bc4e2f82a987","unresolved":false,"context_lines":[{"line_number":3362,"context_line":"        self.stubs.Set(conn, \u0027_lookup_by_name\u0027, fake_lookup_by_name)"},{"line_number":3363,"context_line":"        self.stubs.Set(conn, \u0027_hard_reboot\u0027, fake_hard_reboot)"},{"line_number":3364,"context_line":"        conn.reboot(None, instance, [])"},{"line_number":3365,"context_line":"        self.assertTrue(self.reboot_hard_reboot_called)"},{"line_number":3366,"context_line":""},{"line_number":3367,"context_line":"    def test_destroy_undefines(self):"},{"line_number":3368,"context_line":"        mock \u003d self.mox.CreateMock(libvirt.virDomain)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAQn%2F%2FSCQ%3D","line":3365,"in_reply_to":"AAAAQn%2F%2FSkE%3D","updated":"2013-08-02 16:10:59.000000000","message":"Per discussion in IRC today, I\u0027ll try to re-work this to use mox instead.","commit_id":"5230ba7e48100c6007f6a986712a09f88f67747e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"815ea16b00cdb400bccf60c4431b0413e96c363c","unresolved":false,"context_lines":[{"line_number":3362,"context_line":"        self.stubs.Set(conn, \u0027_lookup_by_name\u0027, fake_lookup_by_name)"},{"line_number":3363,"context_line":"        self.stubs.Set(conn, \u0027_hard_reboot\u0027, fake_hard_reboot)"},{"line_number":3364,"context_line":"        conn.reboot(None, instance, [])"},{"line_number":3365,"context_line":"        self.assertTrue(self.reboot_hard_reboot_called)"},{"line_number":3366,"context_line":""},{"line_number":3367,"context_line":"    def test_destroy_undefines(self):"},{"line_number":3368,"context_line":"        mock \u003d self.mox.CreateMock(libvirt.virDomain)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAQn%2F%2FSEo%3D","line":3365,"in_reply_to":"AAAAQn%2F%2FSkE%3D","updated":"2013-08-02 16:00:13.000000000","message":"We discussed this on IRC, and I\u0027m documenting here for the record. I think the assertTrue is fine here in terms of usage validity. However, mock is the right way to make sure this method got called, IMHO, and I think Matt is going to retool it thusly.","commit_id":"5230ba7e48100c6007f6a986712a09f88f67747e"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"64443b728123a181d1f7d9cb2e2f10489507d0f3","unresolved":false,"context_lines":[{"line_number":1353,"context_line":"            try:"},{"line_number":1354,"context_line":"                soft_reboot_success \u003d self._soft_reboot(instance)"},{"line_number":1355,"context_line":"            except libvirt.libvirtError as e:"},{"line_number":1356,"context_line":"                LOG.exception(_(\"Instance soft reboot failed: %s\"), e)"},{"line_number":1357,"context_line":"                soft_reboot_success \u003d False"},{"line_number":1358,"context_line":""},{"line_number":1359,"context_line":"            if soft_reboot_success:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAQn%2F%2FShs%3D","line":1356,"updated":"2013-08-02 14:16:16.000000000","message":"LOG.exception comes in as an ERROR, we have recovery so we shouldn\u0027t do that.\n\nWe should LOG.warn that soft reboot failed, trying hard reboot\n\nThen we should LOG.debug the exception, so if you are in debug mode (like the gate) we\u0027ll get the stack trace.","commit_id":"5230ba7e48100c6007f6a986712a09f88f67747e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"884d8e4f65d482c0eff0d5294963bc4e2f82a987","unresolved":false,"context_lines":[{"line_number":1353,"context_line":"            try:"},{"line_number":1354,"context_line":"                soft_reboot_success \u003d self._soft_reboot(instance)"},{"line_number":1355,"context_line":"            except libvirt.libvirtError as e:"},{"line_number":1356,"context_line":"                LOG.exception(_(\"Instance soft reboot failed: %s\"), e)"},{"line_number":1357,"context_line":"                soft_reboot_success \u003d False"},{"line_number":1358,"context_line":""},{"line_number":1359,"context_line":"            if soft_reboot_success:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAQn%2F%2FSB4%3D","line":1356,"in_reply_to":"AAAAQn%2F%2FShs%3D","updated":"2013-08-02 16:10:59.000000000","message":"Will do.","commit_id":"5230ba7e48100c6007f6a986712a09f88f67747e"},{"author":{"_account_id":7138,"name":"David Jia","email":"jiataotj@cn.ibm.com","username":"jiataotj"},"change_message_id":"e2c3bdc43ab81dcd7206482ea7df9a7383d8e3ac","unresolved":false,"context_lines":[{"line_number":1354,"context_line":"                soft_reboot_success \u003d self._soft_reboot(instance)"},{"line_number":1355,"context_line":"            except libvirt.libvirtError as e:"},{"line_number":1356,"context_line":"                LOG.exception(_(\"Instance soft reboot failed: %s\"), e)"},{"line_number":1357,"context_line":"                soft_reboot_success \u003d False"},{"line_number":1358,"context_line":""},{"line_number":1359,"context_line":"            if soft_reboot_success:"},{"line_number":1360,"context_line":"                LOG.info(_(\"Instance soft rebooted successfully.\"),"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAQn%2F%2Fy%2Bo%3D","line":1357,"updated":"2013-07-23 03:36:50.000000000","message":"Line 1357 is unnecessary","commit_id":"5230ba7e48100c6007f6a986712a09f88f67747e"},{"author":{"_account_id":1994,"name":"Zhongyue Luo","email":"zhongyue.luo@gmail.com","username":"zyluo"},"change_message_id":"756286fc69af6190a6211502907fa27a7d6ffc26","unresolved":false,"context_lines":[{"line_number":1354,"context_line":"                soft_reboot_success \u003d self._soft_reboot(instance)"},{"line_number":1355,"context_line":"            except libvirt.libvirtError as e:"},{"line_number":1356,"context_line":"                LOG.exception(_(\"Instance soft reboot failed: %s\"), e)"},{"line_number":1357,"context_line":"                soft_reboot_success \u003d False"},{"line_number":1358,"context_line":""},{"line_number":1359,"context_line":"            if soft_reboot_success:"},{"line_number":1360,"context_line":"                LOG.info(_(\"Instance soft rebooted successfully.\"),"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAQn%2F%2Fy4I%3D","line":1357,"in_reply_to":"AAAAQn%2F%2Fy%2Bo%3D","updated":"2013-07-23 04:51:33.000000000","message":"Actually this line is needed.","commit_id":"5230ba7e48100c6007f6a986712a09f88f67747e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"43895027009f65947469dc26d13b806db6534b30","unresolved":false,"context_lines":[{"line_number":1354,"context_line":"                soft_reboot_success \u003d self._soft_reboot(instance)"},{"line_number":1355,"context_line":"            except libvirt.libvirtError as e:"},{"line_number":1356,"context_line":"                LOG.exception(_(\"Instance soft reboot failed: %s\"), e)"},{"line_number":1357,"context_line":"                soft_reboot_success \u003d False"},{"line_number":1358,"context_line":""},{"line_number":1359,"context_line":"            if soft_reboot_success:"},{"line_number":1360,"context_line":"                LOG.info(_(\"Instance soft rebooted successfully.\"),"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAQn%2F%2Fxwg%3D","line":1357,"in_reply_to":"AAAAQn%2F%2Fy4I%3D","updated":"2013-07-23 13:06:59.000000000","message":"Correct. If we don\u0027t set soft_reboot_success to False on line 1357 when an exception is raised, the variable isn\u0027t defined when we get to line 1359 and we\u0027ll have an error.","commit_id":"5230ba7e48100c6007f6a986712a09f88f67747e"},{"author":{"_account_id":1994,"name":"Zhongyue Luo","email":"zhongyue.luo@gmail.com","username":"zyluo"},"change_message_id":"ad3115a391011083a0259986979366601fb9d71d","unresolved":false,"context_lines":[{"line_number":1362,"context_line":"                return"},{"line_number":1363,"context_line":"            else:"},{"line_number":1364,"context_line":"                LOG.warn(_(\"Failed to soft reboot instance.\"),"},{"line_number":1365,"context_line":"                         instance\u003dinstance)"},{"line_number":1366,"context_line":"        return self._hard_reboot(context, instance, network_info,"},{"line_number":1367,"context_line":"                                 block_device_info)"},{"line_number":1368,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAQn%2F%2F2dA%3D","line":1365,"updated":"2013-07-22 05:30:00.000000000","message":"Can\u0027t we merge try/except and the if/else here? The log here seems redundant.","commit_id":"5230ba7e48100c6007f6a986712a09f88f67747e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0a3fb667220850fdf745361df50b1167cd8562ee","unresolved":false,"context_lines":[{"line_number":1362,"context_line":"                return"},{"line_number":1363,"context_line":"            else:"},{"line_number":1364,"context_line":"                LOG.warn(_(\"Failed to soft reboot instance.\"),"},{"line_number":1365,"context_line":"                         instance\u003dinstance)"},{"line_number":1366,"context_line":"        return self._hard_reboot(context, instance, network_info,"},{"line_number":1367,"context_line":"                                 block_device_info)"},{"line_number":1368,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAQn%2F%2F1a0%3D","line":1365,"in_reply_to":"AAAAQn%2F%2F1ik%3D","updated":"2013-07-22 13:51:38.000000000","message":"I agree with Phil here.  I left the warning in there in the case that _soft_reboot returns False rather than throws an exception.  And because I left the warning in, I didn\u0027t feel the need to bloat the exception log message with the instance information.  All the new code does is catch the exception, log it and set the flag for the rest of the code.  I didn\u0027t want to get any fancier than that.","commit_id":"5230ba7e48100c6007f6a986712a09f88f67747e"},{"author":{"_account_id":1501,"name":"Phil Day","email":"philip.day@hp.com","username":"philip-day"},"change_message_id":"458704d376bf537efce12e85959989a3e39e1c68","unresolved":false,"context_lines":[{"line_number":1362,"context_line":"                return"},{"line_number":1363,"context_line":"            else:"},{"line_number":1364,"context_line":"                LOG.warn(_(\"Failed to soft reboot instance.\"),"},{"line_number":1365,"context_line":"                         instance\u003dinstance)"},{"line_number":1366,"context_line":"        return self._hard_reboot(context, instance, network_info,"},{"line_number":1367,"context_line":"                                 block_device_info)"},{"line_number":1368,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAQn%2F%2F1ik%3D","line":1365,"in_reply_to":"AAAAQn%2F%2F2Sg%3D","updated":"2013-07-22 12:48:49.000000000","message":"I prefer the original code - I think always having the LOG.warn is better than only having it if there is no excpetion","commit_id":"5230ba7e48100c6007f6a986712a09f88f67747e"},{"author":{"_account_id":4468,"name":"Wangpan","email":"hzwangpan@corp.netease.com","username":"hzwangpan"},"change_message_id":"5d49c34ef008585aa5f26cd1a2700bdb8fda3e1b","unresolved":false,"context_lines":[{"line_number":1362,"context_line":"                return"},{"line_number":1363,"context_line":"            else:"},{"line_number":1364,"context_line":"                LOG.warn(_(\"Failed to soft reboot instance.\"),"},{"line_number":1365,"context_line":"                         instance\u003dinstance)"},{"line_number":1366,"context_line":"        return self._hard_reboot(context, instance, network_info,"},{"line_number":1367,"context_line":"                                 block_device_info)"},{"line_number":1368,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAQn%2F%2F2Sg%3D","line":1365,"in_reply_to":"AAAAQn%2F%2F2U0%3D","updated":"2013-07-22 07:00:55.000000000","message":"it is ok, too. so let others to judge it.","commit_id":"5230ba7e48100c6007f6a986712a09f88f67747e"},{"author":{"_account_id":1994,"name":"Zhongyue Luo","email":"zhongyue.luo@gmail.com","username":"zyluo"},"change_message_id":"c97828b77a505dbcf68ff2c175e16c57a8f69c6b","unresolved":false,"context_lines":[{"line_number":1362,"context_line":"                return"},{"line_number":1363,"context_line":"            else:"},{"line_number":1364,"context_line":"                LOG.warn(_(\"Failed to soft reboot instance.\"),"},{"line_number":1365,"context_line":"                         instance\u003dinstance)"},{"line_number":1366,"context_line":"        return self._hard_reboot(context, instance, network_info,"},{"line_number":1367,"context_line":"                                 block_device_info)"},{"line_number":1368,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAQn%2F%2F2U0%3D","line":1365,"in_reply_to":"AAAAQn%2F%2F2Ws%3D","updated":"2013-07-22 06:36:28.000000000","message":"Makes sense. Although it would be better if only one of the two logs (1356 and 1364) were printed. How about adding \"else:\" on line 1358 and do something like:\n\n if reboot_type \u003d\u003d \u0027SOFT\u0027:\n     # NOTE(vish): This will attempt to do a graceful shutdown/restart.\n     try:\n         soft_reboot_success \u003d self._soft_reboot(instance)\n     except libvirt.libvirtError as e:\n         LOG.exception(_(\"Instance soft reboot failed: %s\"), e)\n     else:\n         if soft_reboot_success:\n             LOG.info(_(\"Instance soft rebooted successfully.\"),\n                      instance\u003dinstance)\n             return\n         else:\n             LOG.warn(_(\"Failed to soft reboot instance.\"),\n                      instance\u003dinstance)","commit_id":"5230ba7e48100c6007f6a986712a09f88f67747e"},{"author":{"_account_id":4468,"name":"Wangpan","email":"hzwangpan@corp.netease.com","username":"hzwangpan"},"change_message_id":"480d8674e083c10420a6b05c4caaa6ba6f73e4d8","unresolved":false,"context_lines":[{"line_number":1362,"context_line":"                return"},{"line_number":1363,"context_line":"            else:"},{"line_number":1364,"context_line":"                LOG.warn(_(\"Failed to soft reboot instance.\"),"},{"line_number":1365,"context_line":"                         instance\u003dinstance)"},{"line_number":1366,"context_line":"        return self._hard_reboot(context, instance, network_info,"},{"line_number":1367,"context_line":"                                 block_device_info)"},{"line_number":1368,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAQn%2F%2F2Ws%3D","line":1365,"in_reply_to":"AAAAQn%2F%2F2cw%3D","updated":"2013-07-22 06:21:16.000000000","message":"I think this is OK, line 1356 logs the exception situation, and line 1364 logs the fail situation(such as non-exception).","commit_id":"5230ba7e48100c6007f6a986712a09f88f67747e"},{"author":{"_account_id":1994,"name":"Zhongyue Luo","email":"zhongyue.luo@gmail.com","username":"zyluo"},"change_message_id":"437543b0ef2329e02c405ddc6f21272caeb6fa4d","unresolved":false,"context_lines":[{"line_number":1362,"context_line":"                return"},{"line_number":1363,"context_line":"            else:"},{"line_number":1364,"context_line":"                LOG.warn(_(\"Failed to soft reboot instance.\"),"},{"line_number":1365,"context_line":"                         instance\u003dinstance)"},{"line_number":1366,"context_line":"        return self._hard_reboot(context, instance, network_info,"},{"line_number":1367,"context_line":"                                 block_device_info)"},{"line_number":1368,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAQn%2F%2F2cw%3D","line":1365,"in_reply_to":"AAAAQn%2F%2F2dA%3D","updated":"2013-07-22 05:31:43.000000000","message":"What I mean is lines 1359~1362 can go below line 1354 and add \"instance\" to the log on line 1356. Delete line 1357","commit_id":"5230ba7e48100c6007f6a986712a09f88f67747e"}]}