)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"33ef6d2b129530e8d2e434196a53d7c6f560b6b1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"af840beb_eb0fa04b","updated":"2025-08-11 10:59:03.000000000","message":"I agree that we should have a test, but this is such an obvious fix that I\u0027m inclined to just get this in.","commit_id":"cfb99acd6ada462eb93846b3c2ec65c6693eb0b3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3dcd3bc65a974e4addc67bb3cd2a22d174fbd0b0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2c8106d0_771f93e4","updated":"2025-05-27 15:11:03.000000000","message":"let see what ci says and review the coverage report","commit_id":"cfb99acd6ada462eb93846b3c2ec65c6693eb0b3"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3dcd3bc65a974e4addc67bb3cd2a22d174fbd0b0","unresolved":true,"context_lines":[{"line_number":11349,"context_line":"                self.live_migration_abort(instance)"},{"line_number":11350,"context_line":"            except libvirt.libvirtError:"},{"line_number":11351,"context_line":"                LOG.warning(\"Error occurred when trying to abort live \""},{"line_number":11352,"context_line":"                            \"migration job, ignoring it.\", instance\u003dinstance)"},{"line_number":11353,"context_line":"            raise"},{"line_number":11354,"context_line":"        finally:"},{"line_number":11355,"context_line":"            LOG.debug(\"Live migration monitoring is all done\","}],"source_content_type":"text/x-python","patch_set":1,"id":"0fe5e2d1_f68b9492","line":11352,"updated":"2025-05-27 15:11:03.000000000","message":"we probably should have unit test coverage for this.\non one hand this is a trivial change that i think we can just do.\non teh other hand we do have some logging fixture that should cature this if\nthis code is run with a unit test so i assume we are missing coverage of this?","commit_id":"cfb99acd6ada462eb93846b3c2ec65c6693eb0b3"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"d7360db9d68273f7539668d51a63cbb0b677fb93","unresolved":true,"context_lines":[{"line_number":11349,"context_line":"                self.live_migration_abort(instance)"},{"line_number":11350,"context_line":"            except libvirt.libvirtError:"},{"line_number":11351,"context_line":"                LOG.warning(\"Error occurred when trying to abort live \""},{"line_number":11352,"context_line":"                            \"migration job, ignoring it.\", instance\u003dinstance)"},{"line_number":11353,"context_line":"            raise"},{"line_number":11354,"context_line":"        finally:"},{"line_number":11355,"context_line":"            LOG.debug(\"Live migration monitoring is all done\","}],"source_content_type":"text/x-python","patch_set":1,"id":"72033a1a_68cb3db2","line":11352,"in_reply_to":"0fe5e2d1_f68b9492","updated":"2025-05-27 16:16:42.000000000","message":"So I\u0027d assume that this test should be covering it:\n\nhttps://opendev.org/openstack/nova/src/commit/221a3e89e8988bc664298106ee691a4e41ca71f9/nova/tests/unit/virt/libvirt/test_driver.py#L14930-L14937\n\nBut the problem is, that it not only raises libvirt error, but also TypeError as a collateral.","commit_id":"cfb99acd6ada462eb93846b3c2ec65c6693eb0b3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ed22495560766f4acc63511eb58a1f852c560044","unresolved":true,"context_lines":[{"line_number":11349,"context_line":"                self.live_migration_abort(instance)"},{"line_number":11350,"context_line":"            except libvirt.libvirtError:"},{"line_number":11351,"context_line":"                LOG.warning(\"Error occurred when trying to abort live \""},{"line_number":11352,"context_line":"                            \"migration job, ignoring it.\", instance\u003dinstance)"},{"line_number":11353,"context_line":"            raise"},{"line_number":11354,"context_line":"        finally:"},{"line_number":11355,"context_line":"            LOG.debug(\"Live migration monitoring is all done\","}],"source_content_type":"text/x-python","patch_set":1,"id":"8e0b9656_c853f628","line":11352,"in_reply_to":"72033a1a_68cb3db2","updated":"2025-05-28 13:19:09.000000000","message":"we have a generic custom nova fixture for logging\n\nhttps://github.com/openstack/nova/blob/master/nova/tests/fixtures/nova.py#L108-L205\n\nThose are meant to internally catch any formatting error like this and fail the relevent test.\n\nI guess we only use the NullHandler() for DEBUG when debug loging is not enabled\n\nso perhaps the generic log fixture https://github.com/testing-cabal/fixtures/blob/master/fixtures/_fixtures/logger.py is not properly validating that but \nlooking at the coverage report there is no unit test coverage for this branch.\nso i suspect its just not logging in the tests run.\n\nhttps://d9cb353d76e3da86de8a-bf4df36ee9e72417a89c120068385812.ssl.cf1.rackcdn.com/openstack/5b74fd85f2ee442fab7b5122a28b2d70/cover/z_155e0da3f29b7c55_driver_py.html#t11350\n\nthat proably because this branch only gets taken if \n\nself.live_migration_abort(instance) raises a libvirt error after\n\nself._live_migration_monitor has already rasied an excption\n\n_test_live_migration_monitor_job_stats_exception does not emulate that today.\nit only tests the intiall raise but not the raise in the excption handler code.\n\nif you have time to add a test for that that would be nice.\ni think we coudl also close that gap in a followup patch.","commit_id":"cfb99acd6ada462eb93846b3c2ec65c6693eb0b3"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"9e11d9354fe589312a0472ea40c36ceff60edcac","unresolved":true,"context_lines":[{"line_number":11349,"context_line":"                self.live_migration_abort(instance)"},{"line_number":11350,"context_line":"            except libvirt.libvirtError:"},{"line_number":11351,"context_line":"                LOG.warning(\"Error occurred when trying to abort live \""},{"line_number":11352,"context_line":"                            \"migration job, ignoring it.\", instance\u003dinstance)"},{"line_number":11353,"context_line":"            raise"},{"line_number":11354,"context_line":"        finally:"},{"line_number":11355,"context_line":"            LOG.debug(\"Live migration monitoring is all done\","}],"source_content_type":"text/x-python","patch_set":1,"id":"ff724a59_c5218e6a","line":11352,"in_reply_to":"8e0b9656_c853f628","updated":"2025-07-14 15:05:42.000000000","message":"frankly - I would appreciate if we could add test coverage over logging in a follow-up.\n\nAs I still can\u0027t figure out the root cause of this log trigger (qemu domain crash), so dunno when will be able to get back to this.","commit_id":"cfb99acd6ada462eb93846b3c2ec65c6693eb0b3"}]}