)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"bd1ce8800c610e697934f5bbdb4195fc796e413c","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"libvirt: Rework \u0027EBUSY\u0027 (SIGKILL) error handling code path"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Nova commit 3907867 (libvirt: handle code\u003d38 + sigkill (ebusy) in"},{"line_number":10,"context_line":"destroY()) handled the case where a QEMU process \"refuses to die\" within"},{"line_number":11,"context_line":"a given timeout period set by libvirt."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"ffb9cba7_ab53ad16","line":9,"range":{"start_line":9,"start_character":12,"end_line":9,"end_character":19},"updated":"2019-04-30 09:49:38.000000000","message":"supernit - Would be super if this could be a clickable reference in the UI, IIRC that requires something like \u003e\u003d8 characters and also accepts the change id.","commit_id":"362045bf2bd9d9a34d7307ce0c07aec0c325cfbb"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"bd1ce8800c610e697934f5bbdb4195fc796e413c","unresolved":false,"context_lines":[{"line_number":7,"context_line":"libvirt: Rework \u0027EBUSY\u0027 (SIGKILL) error handling code path"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Nova commit 3907867 (libvirt: handle code\u003d38 + sigkill (ebusy) in"},{"line_number":10,"context_line":"destroY()) handled the case where a QEMU process \"refuses to die\" within"},{"line_number":11,"context_line":"a given timeout period set by libvirt."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Originally, libvirt sent SIGTERM (allowing the process to clean-up"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"ffb9cba7_6b593533","line":10,"range":{"start_line":10,"start_character":6,"end_line":10,"end_character":7},"updated":"2019-04-30 09:49:38.000000000","message":"y","commit_id":"362045bf2bd9d9a34d7307ce0c07aec0c325cfbb"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"bd1ce8800c610e697934f5bbdb4195fc796e413c","unresolved":false,"context_lines":[{"line_number":24,"context_line":"In this change:"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":" - Increment the counter to retry the destroy() call from 3 to 6, thus"},{"line_number":27,"context_line":"   increasing the total time from 15 to 30 seconds, before SIKILL takes"},{"line_number":28,"context_line":"   effect.  And it matches the (more graceful) behaviour of libvirt"},{"line_number":29,"context_line":"   v4.7.0.  This also gives breathing room for Nova instances running in"},{"line_number":30,"context_line":"   environments with large Compute nodes (3TB of RAM) with high instance"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"ffb9cba7_8b29098b","line":27,"range":{"start_line":27,"start_character":59,"end_line":27,"end_character":65},"updated":"2019-04-30 09:49:38.000000000","message":"SIGKILL","commit_id":"362045bf2bd9d9a34d7307ce0c07aec0c325cfbb"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"bd1ce8800c610e697934f5bbdb4195fc796e413c","unresolved":false,"context_lines":[{"line_number":27,"context_line":"   increasing the total time from 15 to 30 seconds, before SIKILL takes"},{"line_number":28,"context_line":"   effect.  And it matches the (more graceful) behaviour of libvirt"},{"line_number":29,"context_line":"   v4.7.0.  This also gives breathing room for Nova instances running in"},{"line_number":30,"context_line":"   environments with large Compute nodes (3TB of RAM) with high instance"},{"line_number":31,"context_line":"   creation or delete churn, where the current timout may not be"},{"line_number":32,"context_line":"   sufficient."},{"line_number":33,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"ffb9cba7_2b3a5dcb","line":30,"range":{"start_line":30,"start_character":27,"end_line":30,"end_character":34},"updated":"2019-04-30 09:49:38.000000000","message":"compute","commit_id":"362045bf2bd9d9a34d7307ce0c07aec0c325cfbb"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"bd1ce8800c610e697934f5bbdb4195fc796e413c","unresolved":false,"context_lines":[{"line_number":27,"context_line":"   increasing the total time from 15 to 30 seconds, before SIKILL takes"},{"line_number":28,"context_line":"   effect.  And it matches the (more graceful) behaviour of libvirt"},{"line_number":29,"context_line":"   v4.7.0.  This also gives breathing room for Nova instances running in"},{"line_number":30,"context_line":"   environments with large Compute nodes (3TB of RAM) with high instance"},{"line_number":31,"context_line":"   creation or delete churn, where the current timout may not be"},{"line_number":32,"context_line":"   sufficient."},{"line_number":33,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"ffb9cba7_6b24d5af","line":30,"range":{"start_line":30,"start_character":41,"end_line":30,"end_character":53},"updated":"2019-04-30 09:49:38.000000000","message":"supernit - You don\u0027t need to reference this IMHO.","commit_id":"362045bf2bd9d9a34d7307ce0c07aec0c325cfbb"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2c323bffe9017a281611caae63ba985c36ae7f0d","unresolved":false,"context_lines":[{"line_number":7,"context_line":"libvirt: Rework \u0027EBUSY\u0027 (SIGKILL) error handling code path"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change ID I128bf6b939 (libvirt: handle code\u003d38 + sigkill (ebusy) in"},{"line_number":10,"context_line":"destroy()) handled the case where a QEMU process \"refuses to die\" within"},{"line_number":11,"context_line":"a given timeout period set by libvirt."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Originally, libvirt sent SIGTERM (allowing the process to clean-up"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"bfb3d3c7_e0444a76","line":10,"range":{"start_line":10,"start_character":0,"end_line":10,"end_character":7},"updated":"2019-05-29 10:43:27.000000000","message":"_destroy()","commit_id":"dd5b19b3f90ed91154d4ef5dacec06f8f8b930be"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2c323bffe9017a281611caae63ba985c36ae7f0d","unresolved":false,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":"In this change:"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":" - Increment the counter to retry the destroy() call from 3 to 6, thus"},{"line_number":27,"context_line":"   increasing the total time from 15 to 30 seconds, before SIGKILL takes"},{"line_number":28,"context_line":"   effect.  And it matches the (more graceful) behaviour of libvirt"},{"line_number":29,"context_line":"   v4.7.0.  This also gives breathing room for Nova instances running in"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"bfb3d3c7_a062d208","line":26,"range":{"start_line":26,"start_character":38,"end_line":26,"end_character":47},"updated":"2019-05-29 10:43:27.000000000","message":"_destroy()","commit_id":"dd5b19b3f90ed91154d4ef5dacec06f8f8b930be"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2c323bffe9017a281611caae63ba985c36ae7f0d","unresolved":false,"context_lines":[{"line_number":30,"context_line":"   environments with large compute nodes with high instance creation or"},{"line_number":31,"context_line":"   delete churn, where the current timout may not be sufficient."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":" - Retry the destroy() API call _only_ if MIN_LIBVIRT_VERSION is"},{"line_number":34,"context_line":"   lower than 4.7.0."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"[1] https://libvirt.org/git/?p\u003dlibvirt.git;a\u003dcommitdiff;h\u003d9a4e4b9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"bfb3d3c7_c05d8647","line":33,"range":{"start_line":33,"start_character":13,"end_line":33,"end_character":22},"updated":"2019-05-29 10:43:27.000000000","message":"_destroy()","commit_id":"dd5b19b3f90ed91154d4ef5dacec06f8f8b930be"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b3dcb3e46701934633f7553b66889c4ce46b6672","unresolved":false,"context_lines":[{"line_number":14752,"context_line":"            self.assertRaises(fakelibvirt.libvirtError, drvr._destroy,"},{"line_number":14753,"context_line":"                              instance)"},{"line_number":14754,"context_line":""},{"line_number":14755,"context_line":"        self.assertEqual(6, mock_guest.poweroff.call_count)"},{"line_number":14756,"context_line":""},{"line_number":14757,"context_line":"    def test_private_destroy_ebusy_multiple_attempt_ok(self):"},{"line_number":14758,"context_line":"        # Tests that the _destroy attempt loop is broken when EBUSY is no"}],"source_content_type":"text/x-python","patch_set":6,"id":"ffb9cba7_2c3ef08a","line":14755,"updated":"2019-04-30 17:56:25.000000000","message":"shouldn\u0027t we have a test case for when the libvirt version is \u003e\u003d 4.7.0 (which IIUC means poweroff will only be called once before raising)?","commit_id":"dc6ca1110fceec036e2b2056e45a19c367005f70"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"e54bf61102eaf29e75c31f842978b6355862949e","unresolved":false,"context_lines":[{"line_number":14752,"context_line":"            self.assertRaises(fakelibvirt.libvirtError, drvr._destroy,"},{"line_number":14753,"context_line":"                              instance)"},{"line_number":14754,"context_line":""},{"line_number":14755,"context_line":"        self.assertEqual(6, mock_guest.poweroff.call_count)"},{"line_number":14756,"context_line":""},{"line_number":14757,"context_line":"    def test_private_destroy_ebusy_multiple_attempt_ok(self):"},{"line_number":14758,"context_line":"        # Tests that the _destroy attempt loop is broken when EBUSY is no"}],"source_content_type":"text/x-python","patch_set":6,"id":"ffb9cba7_02541352","line":14755,"in_reply_to":"ffb9cba7_2c3ef08a","updated":"2019-04-30 21:06:15.000000000","message":"A test is on the plan (discussed on #openstack-nova IRC this morning), I was first ironing out the core change itself.\n\nThanks for the review!","commit_id":"dc6ca1110fceec036e2b2056e45a19c367005f70"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"67c162524d1567649e36849b917fa9e78e31c57b","unresolved":false,"context_lines":[{"line_number":14929,"context_line":""},{"line_number":14930,"context_line":"        self.assertEqual(2, mock_guest.poweroff.call_count)"},{"line_number":14931,"context_line":""},{"line_number":14932,"context_line":"    # FIXME"},{"line_number":14933,"context_line":"    @mock.patch.object(libvirt_driver.LOG, \u0027warning\u0027)"},{"line_number":14934,"context_line":"    @mock.patch.object(fakelibvirt.Connection, \u0027getLibVersion\u0027,"},{"line_number":14935,"context_line":"                       return_value\u003dversionutils.convert_version_to_int("}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_202ee2fa","line":14932,"range":{"start_line":14932,"start_character":6,"end_line":14932,"end_character":11},"updated":"2019-05-29 10:02:13.000000000","message":"Will nix this on next iteration","commit_id":"dd5b19b3f90ed91154d4ef5dacec06f8f8b930be"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"23db0a6d227212045e866df04d3da7c435046b69","unresolved":false,"context_lines":[{"line_number":14929,"context_line":""},{"line_number":14930,"context_line":"        self.assertEqual(2, mock_guest.poweroff.call_count)"},{"line_number":14931,"context_line":""},{"line_number":14932,"context_line":"    # FIXME"},{"line_number":14933,"context_line":"    @mock.patch.object(libvirt_driver.LOG, \u0027warning\u0027)"},{"line_number":14934,"context_line":"    @mock.patch.object(fakelibvirt.Connection, \u0027getLibVersion\u0027,"},{"line_number":14935,"context_line":"                       return_value\u003dversionutils.convert_version_to_int("}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_43e3e0d9","line":14932,"range":{"start_line":14932,"start_character":6,"end_line":14932,"end_character":11},"in_reply_to":"bfb3d3c7_202ee2fa","updated":"2019-05-29 10:50:54.000000000","message":"Yes, please.","commit_id":"dd5b19b3f90ed91154d4ef5dacec06f8f8b930be"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"3552624833cbf2714aac2ae7509cbde087a4f055","unresolved":false,"context_lines":[{"line_number":14939,"context_line":"                                                 mock_get_libversion):"},{"line_number":14940,"context_line":"        ex \u003d fakelibvirt.make_libvirtError("},{"line_number":14941,"context_line":"                fakelibvirt.libvirtError,"},{"line_number":14942,"context_line":"                (\"Failed to terminate process 26425 with SIGKILL: \""},{"line_number":14943,"context_line":"                 \"Device or resource busy\"),"},{"line_number":14944,"context_line":"                error_code\u003dfakelibvirt.VIR_ERR_SYSTEM_ERROR,"},{"line_number":14945,"context_line":"                int1\u003derrno.EBUSY)"},{"line_number":14946,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_00ecfe8c","line":14943,"range":{"start_line":14942,"start_character":17,"end_line":14943,"end_character":42},"updated":"2019-05-29 10:13:04.000000000","message":"Maybe first assign this to a temporary variable, so that you can test it below.","commit_id":"dd5b19b3f90ed91154d4ef5dacec06f8f8b930be"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"3552624833cbf2714aac2ae7509cbde087a4f055","unresolved":false,"context_lines":[{"line_number":14952,"context_line":""},{"line_number":14953,"context_line":"        with mock.patch.object(drvr._host, \u0027get_guest\u0027,"},{"line_number":14954,"context_line":"                               return_value\u003dmock_guest):"},{"line_number":14955,"context_line":"            self.assertRaises(fakelibvirt.libvirtError, drvr._destroy,"},{"line_number":14956,"context_line":"                              instance)"},{"line_number":14957,"context_line":""},{"line_number":14958,"context_line":"        self.assertTrue(mock_warning.called)"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_a08bb2c0","line":14955,"range":{"start_line":14955,"start_character":17,"end_line":14955,"end_character":29},"updated":"2019-05-29 10:13:04.000000000","message":"You could use assertRaisesRegexp() here, to ensure that the right libvirtError was raised.  However this was renamed to assertRaisesRegex() in Python 3.2, so you\u0027d need to use https://six.readthedocs.io/#six.assertRaisesRegex","commit_id":"dd5b19b3f90ed91154d4ef5dacec06f8f8b930be"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"23db0a6d227212045e866df04d3da7c435046b69","unresolved":false,"context_lines":[{"line_number":14952,"context_line":""},{"line_number":14953,"context_line":"        with mock.patch.object(drvr._host, \u0027get_guest\u0027,"},{"line_number":14954,"context_line":"                               return_value\u003dmock_guest):"},{"line_number":14955,"context_line":"            self.assertRaises(fakelibvirt.libvirtError, drvr._destroy,"},{"line_number":14956,"context_line":"                              instance)"},{"line_number":14957,"context_line":""},{"line_number":14958,"context_line":"        self.assertTrue(mock_warning.called)"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_a3a5fca3","line":14955,"range":{"start_line":14955,"start_character":17,"end_line":14955,"end_character":29},"in_reply_to":"bfb3d3c7_a08bb2c0","updated":"2019-05-29 10:50:54.000000000","message":"Or just: self.assertRaises(ex, ...)\n\nto assert that it raises your fake exception object specifically.","commit_id":"dd5b19b3f90ed91154d4ef5dacec06f8f8b930be"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"63964535c4263d8604b72ab02be5f174841911c3","unresolved":false,"context_lines":[{"line_number":14952,"context_line":""},{"line_number":14953,"context_line":"        with mock.patch.object(drvr._host, \u0027get_guest\u0027,"},{"line_number":14954,"context_line":"                               return_value\u003dmock_guest):"},{"line_number":14955,"context_line":"            self.assertRaises(fakelibvirt.libvirtError, drvr._destroy,"},{"line_number":14956,"context_line":"                              instance)"},{"line_number":14957,"context_line":""},{"line_number":14958,"context_line":"        self.assertTrue(mock_warning.called)"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_83531839","line":14955,"range":{"start_line":14955,"start_character":17,"end_line":14955,"end_character":29},"in_reply_to":"bfb3d3c7_a3a5fca3","updated":"2019-05-29 11:09:42.000000000","message":"Note to future generations: ^^^ doesn\u0027t work. I fat fingered my test.","commit_id":"dd5b19b3f90ed91154d4ef5dacec06f8f8b930be"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2c323bffe9017a281611caae63ba985c36ae7f0d","unresolved":false,"context_lines":[{"line_number":14955,"context_line":"            self.assertRaises(fakelibvirt.libvirtError, drvr._destroy,"},{"line_number":14956,"context_line":"                              instance)"},{"line_number":14957,"context_line":""},{"line_number":14958,"context_line":"        self.assertTrue(mock_warning.called)"},{"line_number":14959,"context_line":"        self.assertEqual(1, mock_guest.poweroff.call_count)"},{"line_number":14960,"context_line":""},{"line_number":14961,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_63ef04e9","line":14958,"range":{"start_line":14958,"start_character":8,"end_line":14958,"end_character":44},"updated":"2019-05-29 10:43:27.000000000","message":"mock_warning.assert_called_once()","commit_id":"dd5b19b3f90ed91154d4ef5dacec06f8f8b930be"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"3fcd0809b002fcaf9503a0dbc535f80b2827d0b2","unresolved":false,"context_lines":[{"line_number":14955,"context_line":"            self.assertRaises(fakelibvirt.libvirtError, drvr._destroy,"},{"line_number":14956,"context_line":"                              instance)"},{"line_number":14957,"context_line":""},{"line_number":14958,"context_line":"        self.assertTrue(mock_warning.called)"},{"line_number":14959,"context_line":"        self.assertEqual(1, mock_guest.poweroff.call_count)"},{"line_number":14960,"context_line":""},{"line_number":14961,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_46ef6e1d","line":14958,"range":{"start_line":14958,"start_character":8,"end_line":14958,"end_character":44},"in_reply_to":"bfb3d3c7_63ef04e9","updated":"2019-05-29 11:37:12.000000000","message":"Heh, I actually _started_ with what you suggested; but went with \u0027assertTrue\u0027 after some `grep`ing around:\n\n    $\u003e git grep self.assertTrue\\(mock | grep warn | wc -l\n    12\n\nVersus:\n\n    $\u003e git grep mock_warning.assert_called_once | wc -l\n    3\n\nBut can change; as I mock_warning.assert_called_once is easier to read.","commit_id":"dd5b19b3f90ed91154d4ef5dacec06f8f8b930be"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7ef08767f4c228ce23c1e011771866506ca9e884","unresolved":false,"context_lines":[{"line_number":14952,"context_line":"        with mock.patch.object(drvr._host, \u0027get_guest\u0027,"},{"line_number":14953,"context_line":"                               return_value\u003dmock_guest):"},{"line_number":14954,"context_line":"            raised \u003d self.assertRaises(fakelibvirt.libvirtError,"},{"line_number":14955,"context_line":"                                       drvr._destroy,"},{"line_number":14956,"context_line":"                                       instance)"},{"line_number":14957,"context_line":"            self.assertEqual(fakelibvirt.VIR_ERR_SYSTEM_ERROR,"},{"line_number":14958,"context_line":"                    raised.get_error_code())"},{"line_number":14959,"context_line":""},{"line_number":14960,"context_line":"        mock_warning.assert_called_once()"},{"line_number":14961,"context_line":"        self.assertEqual(1, mock_guest.poweroff.call_count)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bfb3d3c7_492ea192","line":14958,"range":{"start_line":14955,"start_character":0,"end_line":14958,"end_character":44},"updated":"2019-05-29 12:26:11.000000000","message":"nit: Indentation.","commit_id":"9903a0b6f615cbae4c23264a7cd4ef0a7303ff39"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"24c1e8b2eba1f96d38efc7c911621a6490be3482","unresolved":false,"context_lines":[{"line_number":14952,"context_line":"        with mock.patch.object(drvr._host, \u0027get_guest\u0027,"},{"line_number":14953,"context_line":"                               return_value\u003dmock_guest):"},{"line_number":14954,"context_line":"            raised \u003d self.assertRaises(fakelibvirt.libvirtError,"},{"line_number":14955,"context_line":"                                       drvr._destroy,"},{"line_number":14956,"context_line":"                                       instance)"},{"line_number":14957,"context_line":"            self.assertEqual(fakelibvirt.VIR_ERR_SYSTEM_ERROR,"},{"line_number":14958,"context_line":"                    raised.get_error_code())"},{"line_number":14959,"context_line":""},{"line_number":14960,"context_line":"        mock_warning.assert_called_once()"},{"line_number":14961,"context_line":"        self.assertEqual(1, mock_guest.poweroff.call_count)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bfb3d3c7_09b8a9e2","line":14958,"range":{"start_line":14955,"start_character":0,"end_line":14958,"end_character":44},"in_reply_to":"bfb3d3c7_492ea192","updated":"2019-05-29 12:51:09.000000000","message":"Fixed in v9","commit_id":"9903a0b6f615cbae4c23264a7cd4ef0a7303ff39"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7ef08767f4c228ce23c1e011771866506ca9e884","unresolved":false,"context_lines":[{"line_number":14958,"context_line":"                    raised.get_error_code())"},{"line_number":14959,"context_line":""},{"line_number":14960,"context_line":"        mock_warning.assert_called_once()"},{"line_number":14961,"context_line":"        self.assertEqual(1, mock_guest.poweroff.call_count)"},{"line_number":14962,"context_line":""},{"line_number":14963,"context_line":"    @mock.patch.object(fakelibvirt.libvirtError, \u0027get_error_code\u0027)"},{"line_number":14964,"context_line":"    @mock.patch.object(host.Host, \u0027_get_domain\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"bfb3d3c7_c93931d7","line":14961,"updated":"2019-05-29 12:26:11.000000000","message":"nit: mock_guest.poweroff.assert_called_once()","commit_id":"9903a0b6f615cbae4c23264a7cd4ef0a7303ff39"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"24c1e8b2eba1f96d38efc7c911621a6490be3482","unresolved":false,"context_lines":[{"line_number":14958,"context_line":"                    raised.get_error_code())"},{"line_number":14959,"context_line":""},{"line_number":14960,"context_line":"        mock_warning.assert_called_once()"},{"line_number":14961,"context_line":"        self.assertEqual(1, mock_guest.poweroff.call_count)"},{"line_number":14962,"context_line":""},{"line_number":14963,"context_line":"    @mock.patch.object(fakelibvirt.libvirtError, \u0027get_error_code\u0027)"},{"line_number":14964,"context_line":"    @mock.patch.object(host.Host, \u0027_get_domain\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"bfb3d3c7_49c2215b","line":14961,"in_reply_to":"bfb3d3c7_c93931d7","updated":"2019-05-29 12:51:09.000000000","message":"Fixed in v9","commit_id":"9903a0b6f615cbae4c23264a7cd4ef0a7303ff39"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"07fd2993001cd393cd8ce5fef1ca7411d5f5b3b5","unresolved":false,"context_lines":[{"line_number":14938,"context_line":"                                                 mock_get_libversion):"},{"line_number":14939,"context_line":"        ex \u003d fakelibvirt.make_libvirtError("},{"line_number":14940,"context_line":"                fakelibvirt.libvirtError,"},{"line_number":14941,"context_line":"                (\"Failed to terminate process \\d+ with SIGKILL: \""},{"line_number":14942,"context_line":"                                     \"Device or resource busy\"),"},{"line_number":14943,"context_line":"                error_code\u003dfakelibvirt.VIR_ERR_SYSTEM_ERROR,"},{"line_number":14944,"context_line":"                int1\u003derrno.EBUSY)"}],"source_content_type":"text/x-python","patch_set":9,"id":"bfb3d3c7_09a18946","line":14941,"range":{"start_line":14941,"start_character":46,"end_line":14941,"end_character":49},"updated":"2019-05-29 13:04:50.000000000","message":"nit: This looks a bit weird now as you didn\u0027t go with the regex match. Doesn\u0027t really matter, though.","commit_id":"41c7881a635d4b21bb5f0eb8e8a7e0d66c0a4998"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"1809f868be75b4ef8e90a29a8f2d7574e0aab50e","unresolved":false,"context_lines":[{"line_number":14938,"context_line":"                                                 mock_get_libversion):"},{"line_number":14939,"context_line":"        ex \u003d fakelibvirt.make_libvirtError("},{"line_number":14940,"context_line":"                fakelibvirt.libvirtError,"},{"line_number":14941,"context_line":"                (\"Failed to terminate process \\d+ with SIGKILL: \""},{"line_number":14942,"context_line":"                                     \"Device or resource busy\"),"},{"line_number":14943,"context_line":"                error_code\u003dfakelibvirt.VIR_ERR_SYSTEM_ERROR,"},{"line_number":14944,"context_line":"                int1\u003derrno.EBUSY)"}],"source_content_type":"text/x-python","patch_set":9,"id":"bfb3d3c7_34d5c044","line":14941,"range":{"start_line":14941,"start_character":46,"end_line":14941,"end_character":49},"in_reply_to":"bfb3d3c7_09a18946","updated":"2019-05-29 13:34:18.000000000","message":"Yeah, if you\u0027re not doing a regexp match it would be better to hardcode a fake pid.  And if you did, a template approach would be required:\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-05-29.log.html#t2019-05-29T10:55:03","commit_id":"41c7881a635d4b21bb5f0eb8e8a7e0d66c0a4998"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"196a0854cc4507cd43a3560e43de45543e7897eb","unresolved":false,"context_lines":[{"line_number":14938,"context_line":"                                                 mock_get_libversion):"},{"line_number":14939,"context_line":"        ex \u003d fakelibvirt.make_libvirtError("},{"line_number":14940,"context_line":"                fakelibvirt.libvirtError,"},{"line_number":14941,"context_line":"                (\"Failed to terminate process \\d+ with SIGKILL: \""},{"line_number":14942,"context_line":"                                     \"Device or resource busy\"),"},{"line_number":14943,"context_line":"                error_code\u003dfakelibvirt.VIR_ERR_SYSTEM_ERROR,"},{"line_number":14944,"context_line":"                int1\u003derrno.EBUSY)"}],"source_content_type":"text/x-python","patch_set":9,"id":"bfb3d3c7_55538e17","line":14941,"range":{"start_line":14941,"start_character":46,"end_line":14941,"end_character":49},"in_reply_to":"bfb3d3c7_34d5c044","updated":"2019-05-29 15:11:15.000000000","message":"Right; no reg-exp match; I changed it only for \"fun\", but it doesn\u0027t matter.  I\u0027ll put back the fake PID.","commit_id":"41c7881a635d4b21bb5f0eb8e8a7e0d66c0a4998"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"1809f868be75b4ef8e90a29a8f2d7574e0aab50e","unresolved":false,"context_lines":[{"line_number":14954,"context_line":"            raised \u003d self.assertRaises(fakelibvirt.libvirtError,"},{"line_number":14955,"context_line":"                                       drvr._destroy,"},{"line_number":14956,"context_line":"                                       instance)"},{"line_number":14957,"context_line":"            self.assertEqual(fakelibvirt.VIR_ERR_SYSTEM_ERROR,"},{"line_number":14958,"context_line":"                             raised.get_error_code())"},{"line_number":14959,"context_line":""},{"line_number":14960,"context_line":"        mock_warning.assert_called_once()"}],"source_content_type":"text/x-python","patch_set":9,"id":"bfb3d3c7_14ce7cb3","line":14957,"updated":"2019-05-29 13:34:18.000000000","message":"Nice.","commit_id":"41c7881a635d4b21bb5f0eb8e8a7e0d66c0a4998"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"deebc5c9dccfa0865ac58c2fe9ddc9ffdf24d324","unresolved":false,"context_lines":[{"line_number":979,"context_line":"                                        {\u0027errcode\u0027: errcode, \u0027e\u0027: e,"},{"line_number":980,"context_line":"                                         \u0027attempt\u0027: attempt},"},{"line_number":981,"context_line":"                                        instance\u003dinstance)"},{"line_number":982,"context_line":"                            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":983,"context_line":"                                # Try up to 6 times before giving up."},{"line_number":984,"context_line":"                                if attempt \u003c 6:"},{"line_number":985,"context_line":"                                    ctxt.reraise \u003d False"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_6d9b8cc8","line":982,"range":{"start_line":982,"start_character":28,"end_line":982,"end_character":78},"updated":"2019-03-25 23:37:41.000000000","message":"(mostly) unrelated: this should be the first thing under except: -- otherwise you\u0027re risking any intervening code resetting/overwriting the exception context.","commit_id":"e3c3e56a36f410870598ac8f7f9c976621e6f5df"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"059cf2cdbc540d1a4734390c78417e41a87a4b49","unresolved":false,"context_lines":[{"line_number":979,"context_line":"                                        {\u0027errcode\u0027: errcode, \u0027e\u0027: e,"},{"line_number":980,"context_line":"                                         \u0027attempt\u0027: attempt},"},{"line_number":981,"context_line":"                                        instance\u003dinstance)"},{"line_number":982,"context_line":"                            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":983,"context_line":"                                # Try up to 6 times before giving up."},{"line_number":984,"context_line":"                                if attempt \u003c 6:"},{"line_number":985,"context_line":"                                    ctxt.reraise \u003d False"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fce034c_04c0d54f","line":982,"range":{"start_line":982,"start_character":28,"end_line":982,"end_character":78},"in_reply_to":"5fc1f717_6d9b8cc8","updated":"2019-04-17 14:57:44.000000000","message":"Hmm.  That should be done in a separate change.  (\"One logical change per patch\")","commit_id":"e3c3e56a36f410870598ac8f7f9c976621e6f5df"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"deebc5c9dccfa0865ac58c2fe9ddc9ffdf24d324","unresolved":false,"context_lines":[{"line_number":985,"context_line":"                                    ctxt.reraise \u003d False"},{"line_number":986,"context_line":"                                    self._destroy(instance, attempt + 1)"},{"line_number":987,"context_line":"                                    return"},{"line_number":988,"context_line":"                        else:"},{"line_number":989,"context_line":"                            return"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"                if not is_okay:"},{"line_number":992,"context_line":"                    with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_ad8df47c","line":989,"range":{"start_line":988,"start_character":0,"end_line":989,"end_character":34},"updated":"2019-03-25 23:37:41.000000000","message":"? I would have thought we would want to fall through to the warn/raise here","commit_id":"e3c3e56a36f410870598ac8f7f9c976621e6f5df"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"78d851525518f9e0bc8ef909747f7680a34a2eac","unresolved":false,"context_lines":[{"line_number":985,"context_line":"                                    ctxt.reraise \u003d False"},{"line_number":986,"context_line":"                                    self._destroy(instance, attempt + 1)"},{"line_number":987,"context_line":"                                    return"},{"line_number":988,"context_line":"                        else:"},{"line_number":989,"context_line":"                            return"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"                if not is_okay:"},{"line_number":992,"context_line":"                    with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":4,"id":"ffb9cba7_bd2305bd","line":989,"range":{"start_line":988,"start_character":0,"end_line":989,"end_character":34},"in_reply_to":"3fce034c_64951940","updated":"2019-04-29 11:46:13.000000000","message":"s/\"raise an exception\"/\"raise\"/\n\nAs in, the exception we handled under \"except libvirt.libvirtError as e [...]\" will kick in.","commit_id":"e3c3e56a36f410870598ac8f7f9c976621e6f5df"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"059cf2cdbc540d1a4734390c78417e41a87a4b49","unresolved":false,"context_lines":[{"line_number":985,"context_line":"                                    ctxt.reraise \u003d False"},{"line_number":986,"context_line":"                                    self._destroy(instance, attempt + 1)"},{"line_number":987,"context_line":"                                    return"},{"line_number":988,"context_line":"                        else:"},{"line_number":989,"context_line":"                            return"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"                if not is_okay:"},{"line_number":992,"context_line":"                    with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fce034c_64951940","line":989,"range":{"start_line":988,"start_character":0,"end_line":989,"end_character":34},"in_reply_to":"5fc1f717_ad8df47c","updated":"2019-04-17 14:57:44.000000000","message":"Yes, I should raise an exception here.  Let me rework...","commit_id":"e3c3e56a36f410870598ac8f7f9c976621e6f5df"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"bd1ce8800c610e697934f5bbdb4195fc796e413c","unresolved":false,"context_lines":[{"line_number":1007,"context_line":"                                        {\u0027errcode\u0027: errcode, \u0027e\u0027: e,"},{"line_number":1008,"context_line":"                                         \u0027attempt\u0027: attempt},"},{"line_number":1009,"context_line":"                                        instance\u003dinstance)"},{"line_number":1010,"context_line":"                            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":1011,"context_line":"                                # Try up to 6 times before giving up."},{"line_number":1012,"context_line":"                                if attempt \u003c 6:"},{"line_number":1013,"context_line":"                                    ctxt.reraise \u003d False"}],"source_content_type":"text/x-python","patch_set":5,"id":"ffb9cba7_0bc75998","line":1010,"range":{"start_line":1010,"start_character":0,"end_line":1010,"end_character":79},"updated":"2019-04-30 09:49:38.000000000","message":"Could we move this up above L1002 and remove the else raise below:\n\n    with excutils.save_and_reraise_exception() as ctxt:\n        if not self._host.has_min_version(\n                MIN_LIBVIRT_BETTER_SIGKILL_HANDLING):","commit_id":"362045bf2bd9d9a34d7307ce0c07aec0c325cfbb"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"c92f9c877e1d06929554e8c0fed4c972aaaeaa60","unresolved":false,"context_lines":[{"line_number":1007,"context_line":"                                        {\u0027errcode\u0027: errcode, \u0027e\u0027: e,"},{"line_number":1008,"context_line":"                                         \u0027attempt\u0027: attempt},"},{"line_number":1009,"context_line":"                                        instance\u003dinstance)"},{"line_number":1010,"context_line":"                            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":1011,"context_line":"                                # Try up to 6 times before giving up."},{"line_number":1012,"context_line":"                                if attempt \u003c 6:"},{"line_number":1013,"context_line":"                                    ctxt.reraise \u003d False"}],"source_content_type":"text/x-python","patch_set":5,"id":"ffb9cba7_4bb5517d","line":1010,"range":{"start_line":1010,"start_character":0,"end_line":1010,"end_character":79},"in_reply_to":"ffb9cba7_0bc75998","updated":"2019-04-30 11:03:12.000000000","message":"Yep, we can do that; and indeed we can do away with the \"else: raise\" below.","commit_id":"362045bf2bd9d9a34d7307ce0c07aec0c325cfbb"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"fdda800d5c2ca9d20151bd4ce229cb0e8ab6c54d","unresolved":false,"context_lines":[{"line_number":985,"context_line":"                        # steal time from the cloud host. ie 15 wallclock"},{"line_number":986,"context_line":"                        # seconds may have passed, but the VM might have only"},{"line_number":987,"context_line":"                        # have a few seconds of scheduled run time."},{"line_number":988,"context_line":"                        #"},{"line_number":989,"context_line":"                        # TODO(kchamart): Rewrite the above note, and"},{"line_number":990,"context_line":"                        # remove following code that retries destroy()"},{"line_number":991,"context_line":"                        # API call (to give SIGKILL 30 seconds to take"}],"source_content_type":"text/x-python","patch_set":6,"id":"bfb3d3c7_970b72d4","line":988,"range":{"start_line":988,"start_character":0,"end_line":988,"end_character":25},"updated":"2019-05-27 15:42:22.000000000","message":"supernit - We don\u0027t need this blank line.","commit_id":"dc6ca1110fceec036e2b2056e45a19c367005f70"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"99b025a4102cd4cf71d3067c3bba08743e7ad65d","unresolved":false,"context_lines":[{"line_number":985,"context_line":"                        # steal time from the cloud host. ie 15 wallclock"},{"line_number":986,"context_line":"                        # seconds may have passed, but the VM might have only"},{"line_number":987,"context_line":"                        # have a few seconds of scheduled run time."},{"line_number":988,"context_line":"                        #"},{"line_number":989,"context_line":"                        # TODO(kchamart): Rewrite the above note, and"},{"line_number":990,"context_line":"                        # remove following code that retries destroy()"},{"line_number":991,"context_line":"                        # API call (to give SIGKILL 30 seconds to take"}],"source_content_type":"text/x-python","patch_set":6,"id":"bfb3d3c7_e9c447f7","line":988,"range":{"start_line":988,"start_character":0,"end_line":988,"end_character":25},"in_reply_to":"bfb3d3c7_970b72d4","updated":"2019-05-28 15:55:22.000000000","message":"I intentionally added it for visual clarity and \"breathing space\".  Otherwise, both notes combined will render as one large blob of text, which hinders readability.","commit_id":"dc6ca1110fceec036e2b2056e45a19c367005f70"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b3dcb3e46701934633f7553b66889c4ce46b6672","unresolved":false,"context_lines":[{"line_number":999,"context_line":"                        #"},{"line_number":1000,"context_line":"                        #   - be2ca0444 (process: wait longer on kill"},{"line_number":1001,"context_line":"                        #     per assigned Hostdev)"},{"line_number":1002,"context_line":"                        with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":1003,"context_line":"                            if not self._host.has_min_version("},{"line_number":1004,"context_line":"                                    MIN_LIBVIRT_BETTER_SIGKILL_HANDLING):"},{"line_number":1005,"context_line":"                                LOG.warning(\u0027Error from libvirt during \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"ffb9cba7_0cc44c90","line":1002,"range":{"start_line":1002,"start_character":24,"end_line":1002,"end_character":75},"updated":"2019-04-30 17:56:25.000000000","message":"ugh, this should be the first thing after the except: line. As written, it\u0027s theoretically possible that e.gett_error_code() and/or e.get_intl() will rewrite the exception context.\n\nHowever, this was broken before, so can be fixed in a separate change.","commit_id":"dc6ca1110fceec036e2b2056e45a19c367005f70"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"e54bf61102eaf29e75c31f842978b6355862949e","unresolved":false,"context_lines":[{"line_number":999,"context_line":"                        #"},{"line_number":1000,"context_line":"                        #   - be2ca0444 (process: wait longer on kill"},{"line_number":1001,"context_line":"                        #     per assigned Hostdev)"},{"line_number":1002,"context_line":"                        with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":1003,"context_line":"                            if not self._host.has_min_version("},{"line_number":1004,"context_line":"                                    MIN_LIBVIRT_BETTER_SIGKILL_HANDLING):"},{"line_number":1005,"context_line":"                                LOG.warning(\u0027Error from libvirt during \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"ffb9cba7_a273c715","line":1002,"range":{"start_line":1002,"start_character":24,"end_line":1002,"end_character":75},"in_reply_to":"ffb9cba7_0cc44c90","updated":"2019-04-30 21:06:15.000000000","message":"Yeah, that\u0027s what I mentioned earlier, will be done as a separate change.","commit_id":"dc6ca1110fceec036e2b2056e45a19c367005f70"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2c323bffe9017a281611caae63ba985c36ae7f0d","unresolved":false,"context_lines":[{"line_number":960,"context_line":"        # If the instance is already terminated, we\u0027re still happy"},{"line_number":961,"context_line":"        # Otherwise, destroy it"},{"line_number":962,"context_line":"        old_domid \u003d -1"},{"line_number":963,"context_line":"        if guest is not None:"},{"line_number":964,"context_line":"            try:"},{"line_number":965,"context_line":"                old_domid \u003d guest.id"},{"line_number":966,"context_line":"                guest.poweroff()"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_432e8092","line":963,"updated":"2019-05-29 10:43:27.000000000","message":"Perhaps you could use this opportunity as an excuse for some refactoring of this horribly long method?  For a start, you could either invert this conditional:\n\n    if guest is None:\n        return\n\nand drop one level of indentation, or even better, just split everything below it into a separate _poweroff() method or similar (probably as a separate commit).","commit_id":"dd5b19b3f90ed91154d4ef5dacec06f8f8b930be"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"3fcd0809b002fcaf9503a0dbc535f80b2827d0b2","unresolved":false,"context_lines":[{"line_number":960,"context_line":"        # If the instance is already terminated, we\u0027re still happy"},{"line_number":961,"context_line":"        # Otherwise, destroy it"},{"line_number":962,"context_line":"        old_domid \u003d -1"},{"line_number":963,"context_line":"        if guest is not None:"},{"line_number":964,"context_line":"            try:"},{"line_number":965,"context_line":"                old_domid \u003d guest.id"},{"line_number":966,"context_line":"                guest.poweroff()"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_0643b614","line":963,"in_reply_to":"bfb3d3c7_432e8092","updated":"2019-05-29 11:37:12.000000000","message":"Yeah, I\u0027d to tackle all the refactoring _after_ addressing this existing problem.  But your point duly noted.","commit_id":"dd5b19b3f90ed91154d4ef5dacec06f8f8b930be"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2c323bffe9017a281611caae63ba985c36ae7f0d","unresolved":false,"context_lines":[{"line_number":995,"context_line":"                    reason \u003d _(\"operation time out\")"},{"line_number":996,"context_line":"                    raise exception.InstancePowerOffFailure(reason\u003dreason)"},{"line_number":997,"context_line":"                elif errcode \u003d\u003d libvirt.VIR_ERR_SYSTEM_ERROR:"},{"line_number":998,"context_line":"                    if e.get_int1() \u003d\u003d errno.EBUSY:"},{"line_number":999,"context_line":"                        # NOTE(danpb): When libvirt kills a process it sends it"},{"line_number":1000,"context_line":"                        # SIGTERM first and waits 10 seconds. If it hasn\u0027t gone"},{"line_number":1001,"context_line":"                        # it sends SIGKILL and waits another 5 seconds. If it"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_c3acb0ee","line":998,"updated":"2019-05-29 10:43:27.000000000","message":"Again I would split this whole block into a separate method (probably as a separate commit).\n\n\u003csoapbox mode\u003d\u0027apologetic\u0027\u003e\nIMHO long methods are hugely harmful for readability (e.g. clear control flow and variable scoping) and hence maintainability.  Also it triggers the broken windows effect: once a method gets to a certain level of complexity, it\u0027s too hard for people to quickly understand the whole thing. So when they need to change it, they just inject more complexity into the portion which they do understand.  So they get longer, and longer, and longer ...  Best to nip this in the bud as early as possible ;-)\n\u003c/soapbox\u003e","commit_id":"dd5b19b3f90ed91154d4ef5dacec06f8f8b930be"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"3fcd0809b002fcaf9503a0dbc535f80b2827d0b2","unresolved":false,"context_lines":[{"line_number":995,"context_line":"                    reason \u003d _(\"operation time out\")"},{"line_number":996,"context_line":"                    raise exception.InstancePowerOffFailure(reason\u003dreason)"},{"line_number":997,"context_line":"                elif errcode \u003d\u003d libvirt.VIR_ERR_SYSTEM_ERROR:"},{"line_number":998,"context_line":"                    if e.get_int1() \u003d\u003d errno.EBUSY:"},{"line_number":999,"context_line":"                        # NOTE(danpb): When libvirt kills a process it sends it"},{"line_number":1000,"context_line":"                        # SIGTERM first and waits 10 seconds. If it hasn\u0027t gone"},{"line_number":1001,"context_line":"                        # it sends SIGKILL and waits another 5 seconds. If it"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_6649525e","line":998,"in_reply_to":"bfb3d3c7_c3acb0ee","updated":"2019-05-29 11:37:12.000000000","message":"Yes, as noted earlier, I\u0027d like to handle the refactoring separately.  (And indeed, as you allude to, not least because of the cardinal rule: \"One logical change per commit\")\n\nAnd, hehe I do agree with the soapbox apologetic .","commit_id":"dd5b19b3f90ed91154d4ef5dacec06f8f8b930be"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"23db0a6d227212045e866df04d3da7c435046b69","unresolved":false,"context_lines":[{"line_number":1027,"context_line":"                        with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":1028,"context_line":"                            if not self._host.has_min_version("},{"line_number":1029,"context_line":"                                    MIN_LIBVIRT_BETTER_SIGKILL_HANDLING):"},{"line_number":1030,"context_line":"                                LOG.warning(\u0027Error from libvirt during \u0027"},{"line_number":1031,"context_line":"                                            \u0027destroy. Code\u003d%(errcode)s \u0027"},{"line_number":1032,"context_line":"                                            \u0027Error\u003d%(e)s; attempt \u0027"},{"line_number":1033,"context_line":"                                            \u0027%(attempt)d of 6 \u0027,"},{"line_number":1034,"context_line":"                                            {\u0027errcode\u0027: errcode, \u0027e\u0027: e,"},{"line_number":1035,"context_line":"                                             \u0027attempt\u0027: attempt},"},{"line_number":1036,"context_line":"                                            instance\u003dinstance)"},{"line_number":1037,"context_line":"                                # Try up to 6 times before giving up."},{"line_number":1038,"context_line":"                                if attempt \u003c 6:"},{"line_number":1039,"context_line":"                                    ctxt.reraise \u003d False"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_63c86473","line":1036,"range":{"start_line":1030,"start_character":0,"end_line":1036,"end_character":62},"updated":"2019-05-29 10:50:54.000000000","message":"Reviewer note: This log message is now guarded by MIN_LIBVIRT_BETTER_SIGKILL_HANDLING, meaning that if we\u0027ve got libvirt \u003e\u003d 4.7.0 we *won\u0027t* generate this message. This is correct, because we\u0027ll generate the message on line 1045 below instead which doesn\u0027t reference an attempt number, because we\u0027re not going to make multiple attempts.","commit_id":"dd5b19b3f90ed91154d4ef5dacec06f8f8b930be"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"1809f868be75b4ef8e90a29a8f2d7574e0aab50e","unresolved":false,"context_lines":[{"line_number":1011,"context_line":"                        # seconds may have passed, but the VM might have only"},{"line_number":1012,"context_line":"                        # have a few seconds of scheduled run time."},{"line_number":1013,"context_line":"                        #"},{"line_number":1014,"context_line":"                        # TODO(kchamart): Rewrite the above note, and"},{"line_number":1015,"context_line":"                        # remove following code that retries destroy()"},{"line_number":1016,"context_line":"                        # API call (to give SIGKILL 30 seconds to take"},{"line_number":1017,"context_line":"                        # effect) once MIN_LIBVIRT_VERSION reaches"}],"source_content_type":"text/x-python","patch_set":9,"id":"bfb3d3c7_b44d303f","line":1014,"updated":"2019-05-29 13:34:18.000000000","message":"Maybe you\u0027re already planning this (in which case forgive me for stating the obvious) but I really think this TODO should be done as part of this review, otherwise it\u0027s just introducing more debt.","commit_id":"41c7881a635d4b21bb5f0eb8e8a7e0d66c0a4998"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"ffc6942637f4b25e1e696bbcfeb957b0522416ec","unresolved":false,"context_lines":[{"line_number":1011,"context_line":"                        # seconds may have passed, but the VM might have only"},{"line_number":1012,"context_line":"                        # have a few seconds of scheduled run time."},{"line_number":1013,"context_line":"                        #"},{"line_number":1014,"context_line":"                        # TODO(kchamart): Rewrite the above note, and"},{"line_number":1015,"context_line":"                        # remove following code that retries destroy()"},{"line_number":1016,"context_line":"                        # API call (to give SIGKILL 30 seconds to take"},{"line_number":1017,"context_line":"                        # effect) once MIN_LIBVIRT_VERSION reaches"}],"source_content_type":"text/x-python","patch_set":9,"id":"bfb3d3c7_ef3995f1","line":1014,"in_reply_to":"bfb3d3c7_b44d303f","updated":"2019-05-29 14:32:15.000000000","message":"This was my initial thought, but Kashyap is explicit that this is a TODO once MIN_LIBVIRT_VERSION reaches v4.7.0. Until then the workaround remains so the above shouldn\u0027t be deleted.","commit_id":"41c7881a635d4b21bb5f0eb8e8a7e0d66c0a4998"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"8e1c50b530d9c711b5e71d3010779cba4c9c2c5b","unresolved":false,"context_lines":[{"line_number":1011,"context_line":"                        # seconds may have passed, but the VM might have only"},{"line_number":1012,"context_line":"                        # have a few seconds of scheduled run time."},{"line_number":1013,"context_line":"                        #"},{"line_number":1014,"context_line":"                        # TODO(kchamart): Rewrite the above note, and"},{"line_number":1015,"context_line":"                        # remove following code that retries destroy()"},{"line_number":1016,"context_line":"                        # API call (to give SIGKILL 30 seconds to take"},{"line_number":1017,"context_line":"                        # effect) once MIN_LIBVIRT_VERSION reaches"}],"source_content_type":"text/x-python","patch_set":9,"id":"bfb3d3c7_b5dd2ae1","line":1014,"in_reply_to":"bfb3d3c7_bab43981","updated":"2019-05-29 15:29:41.000000000","message":"Yeah, I misread this.  Helping Kashyap to clarify the wording in IRC :-)","commit_id":"41c7881a635d4b21bb5f0eb8e8a7e0d66c0a4998"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"196a0854cc4507cd43a3560e43de45543e7897eb","unresolved":false,"context_lines":[{"line_number":1011,"context_line":"                        # seconds may have passed, but the VM might have only"},{"line_number":1012,"context_line":"                        # have a few seconds of scheduled run time."},{"line_number":1013,"context_line":"                        #"},{"line_number":1014,"context_line":"                        # TODO(kchamart): Rewrite the above note, and"},{"line_number":1015,"context_line":"                        # remove following code that retries destroy()"},{"line_number":1016,"context_line":"                        # API call (to give SIGKILL 30 seconds to take"},{"line_number":1017,"context_line":"                        # effect) once MIN_LIBVIRT_VERSION reaches"}],"source_content_type":"text/x-python","patch_set":9,"id":"bfb3d3c7_bab43981","line":1014,"in_reply_to":"bfb3d3c7_ef3995f1","updated":"2019-05-29 15:11:15.000000000","message":"Indeed, what Matt said.  I was careful to write in the note that we should rewrite the note and \"remove [the] following code ... once MIN_LIBVIRT_VERSION reaches v4.7.0\"","commit_id":"41c7881a635d4b21bb5f0eb8e8a7e0d66c0a4998"}]}