)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4aa415a501d3df557a5aee23e68e25c2b3e1bd82","unresolved":false,"context_lines":[{"line_number":4887,"context_line":""},{"line_number":4888,"context_line":"        try:"},{"line_number":4889,"context_line":"            self.driver.detach_interface(instance, condemned)"},{"line_number":4890,"context_line":"        except exception.InterfaceDetachFailed:"},{"line_number":4891,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4892,"context_line":"                LOG.warning(_LW(\"Detach interface failed, vif\u003d%(vif)s\"),"},{"line_number":4893,"context_line":"                            {\u0027vif\u0027: condemned}, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a80dd14_a0eca2f4","line":4890,"updated":"2015-03-12 14:14:03.000000000","message":"Since I just went through https://review.openstack.org/#/c/109178/ I have the same comments here about error handling.  The libvirt driver alone raises multiple errors possibly, including NovaException, NetworkMissingPhysicalNetwork, ProcessExecutionError.  The vmware driver can raise NotFound (for the instance or the device).\n\nSo rather than just handle InterfaceDetachFailed here, we should probably handle NovaException.","commit_id":"5d3be645215c25399506b227e4d495c43e809af4"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"db29a20a4d28405983f1898c461c2f8174e176bb","unresolved":false,"context_lines":[{"line_number":4887,"context_line":""},{"line_number":4888,"context_line":"        try:"},{"line_number":4889,"context_line":"            self.driver.detach_interface(instance, condemned)"},{"line_number":4890,"context_line":"        except exception.InterfaceDetachFailed:"},{"line_number":4891,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4892,"context_line":"                LOG.warning(_LW(\"Detach interface failed, vif\u003d%(vif)s\"),"},{"line_number":4893,"context_line":"                            {\u0027vif\u0027: condemned}, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a80dd14_3f74e066","line":4890,"in_reply_to":"9a80dd14_a0eca2f4","updated":"2015-03-16 02:28:57.000000000","message":"thanks matt, will update it soon.","commit_id":"5d3be645215c25399506b227e4d495c43e809af4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4aa415a501d3df557a5aee23e68e25c2b3e1bd82","unresolved":false,"context_lines":[{"line_number":4888,"context_line":"        try:"},{"line_number":4889,"context_line":"            self.driver.detach_interface(instance, condemned)"},{"line_number":4890,"context_line":"        except exception.InterfaceDetachFailed:"},{"line_number":4891,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4892,"context_line":"                LOG.warning(_LW(\"Detach interface failed, vif\u003d%(vif)s\"),"},{"line_number":4893,"context_line":"                            {\u0027vif\u0027: condemned}, instance\u003dinstance)"},{"line_number":4894,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a80dd14_a0a82278","line":4891,"updated":"2015-03-12 14:14:03.000000000","message":"Similar comment as in https://review.openstack.org/#/c/109178/, it\u0027s probably better if we log the original exception and then always raise InterfaceDetachFailed back to the API extension.  Granted, this is an RPC cast as mentioned in the commit message, so it\u0027s not as big a deal, but it\u0027d be nice to have consistent interfaces with expected exceptions in the attach/detach cases so callers wouldn\u0027t have to potentially handle half a dozen different types of exceptions.","commit_id":"5d3be645215c25399506b227e4d495c43e809af4"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"db29a20a4d28405983f1898c461c2f8174e176bb","unresolved":false,"context_lines":[{"line_number":4888,"context_line":"        try:"},{"line_number":4889,"context_line":"            self.driver.detach_interface(instance, condemned)"},{"line_number":4890,"context_line":"        except exception.InterfaceDetachFailed:"},{"line_number":4891,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4892,"context_line":"                LOG.warning(_LW(\"Detach interface failed, vif\u003d%(vif)s\"),"},{"line_number":4893,"context_line":"                            {\u0027vif\u0027: condemned}, instance\u003dinstance)"},{"line_number":4894,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a80dd14_7f6ad881","line":4891,"in_reply_to":"9a80dd14_a0a82278","updated":"2015-03-16 02:28:57.000000000","message":"yeah, compute api should handler all this kinds exception except just raise up to their callers.","commit_id":"5d3be645215c25399506b227e4d495c43e809af4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0620b8a7858b4f0c739d01fc277636cf181a7ce7","unresolved":false,"context_lines":[{"line_number":4941,"context_line":"        try:"},{"line_number":4942,"context_line":"            self.driver.detach_interface(instance, condemned)"},{"line_number":4943,"context_line":"        except exception.NovaException as ex:"},{"line_number":4944,"context_line":"            LOG.warning(_LW(\"Detach interface failed, vif\u003d%(vif)s,\""},{"line_number":4945,"context_line":"                            \"reason: %(msg)s\"),"},{"line_number":4946,"context_line":"                        {\u0027vif\u0027: condemned, \u0027msg\u0027: ex}, instance\u003dinstance)"},{"line_number":4947,"context_line":"            raise exception.InterfaceDetachFailed(instance_uuid\u003dinstance.uuid)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a80dd14_021eceba","line":4944,"updated":"2015-03-16 14:55:27.000000000","message":"This could use a space after the comma, before reason is logged, this is how it looks currently in the logs:\n\n2015-03-16 02:58:37,013 WARNING [nova.compute.manager] Detach interface failed, vif\u003dVIF({\u0027profile\u0027: None, \u0027ovs_interfaceid\u0027: None, \u0027preserve_on_delete\u0027: False, \u0027network\u0027: Network({\u0027bridge\u0027: \u0027br0\u0027, \u0027subnets\u0027: [Subnet({\u0027ips\u0027: [FixedIP({\u0027meta\u0027: {}, \u0027version\u0027: 4, \u0027type\u0027: \u0027fixed\u0027, \u0027floating_ips\u0027: [], \u0027address\u0027: \u002710.10.0.2\u0027}), FixedIP({\u0027meta\u0027: {}, \u0027version\u0027: 4, \u0027type\u0027: \u0027fixed\u0027, \u0027floating_ips\u0027: [], \u0027address\u0027: \u002710.10.0.3\u0027})], \u0027version\u0027: 4, \u0027meta\u0027: {}, \u0027dns\u0027: [IP({\u0027meta\u0027: {}, \u0027version\u0027: 4, \u0027type\u0027: None, \u0027address\u0027: \u00271.2.3.4\u0027}), IP({\u0027meta\u0027: {}, \u0027version\u0027: 4, \u0027type\u0027: None, \u0027address\u0027: \u00272.3.4.5\u0027})], \u0027routes\u0027: [Route({\u0027interface\u0027: \u0027eth0\u0027, \u0027cidr\u0027: \u00270.0.0.0/24\u0027, \u0027meta\u0027: {}, \u0027gateway\u0027: IP({\u0027meta\u0027: {}, \u0027version\u0027: 4, \u0027type\u0027: None, \u0027address\u0027: \u0027192.168.1.1\u0027})})], \u0027cidr\u0027: \u002710.10.0.0/24\u0027, \u0027gateway\u0027: IP({\u0027meta\u0027: {}, \u0027version\u0027: 4, \u0027type\u0027: None, \u0027address\u0027: \u002710.10.0.1\u0027})}), Subnet({\u0027ips\u0027: [FixedIP({\u0027meta\u0027: {}, \u0027version\u0027: 4, \u0027type\u0027: \u0027fixed\u0027, \u0027floating_ips\u0027: [], \u0027address\u0027: \u002710.10.0.2\u0027}), FixedIP({\u0027meta\u0027: {}, \u0027version\u0027: 4, \u0027type\u0027: \u0027fixed\u0027, \u0027floating_ips\u0027: [], \u0027address\u0027: \u002710.10.0.3\u0027})], \u0027version\u0027: 4, \u0027meta\u0027: {}, \u0027dns\u0027: [IP({\u0027meta\u0027: {}, \u0027version\u0027: 4, \u0027type\u0027: None, \u0027address\u0027: \u00271.2.3.4\u0027}), IP({\u0027meta\u0027: {}, \u0027version\u0027: 4, \u0027type\u0027: None, \u0027address\u0027: \u00272.3.4.5\u0027})], \u0027routes\u0027: [Route({\u0027interface\u0027: \u0027eth0\u0027, \u0027cidr\u0027: \u00270.0.0.0/24\u0027, \u0027meta\u0027: {}, \u0027gateway\u0027: IP({\u0027meta\u0027: {}, \u0027version\u0027: 4, \u0027type\u0027: None, \u0027address\u0027: \u0027192.168.1.1\u0027})})], \u0027cidr\u0027: \u0027255.255.255.255\u0027, \u0027gateway\u0027: IP({\u0027meta\u0027: {}, \u0027version\u0027: 4, \u0027type\u0027: None, \u0027address\u0027: \u002710.10.0.1\u0027})})], \u0027meta\u0027: {}, \u0027id\u0027: 1, \u0027label\u0027: \u0027public\u0027}), \u0027devname\u0027: None, \u0027vnic_type\u0027: \u0027normal\u0027, \u0027qbh_params\u0027: None, \u0027meta\u0027: {}, \u0027details\u0027: {}, \u0027address\u0027: \u0027aa:aa:aa:aa:aa:aa\u0027, \u0027active\u0027: False, \u0027type\u0027: \u0027bridge\u0027, \u0027id\u0027: 1, \u0027qbg_params\u0027: None}),reason: detach_failed\n\nI\u0027m wondering if we need all of that VIF detail too, maybe port_id would suffice.","commit_id":"37cf1c877065db447097ea09ec229854a44f9051"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"2f62c6a82731baa5cbfc773f668da2c36a2fa231","unresolved":false,"context_lines":[{"line_number":4941,"context_line":"        try:"},{"line_number":4942,"context_line":"            self.driver.detach_interface(instance, condemned)"},{"line_number":4943,"context_line":"        except exception.NovaException as ex:"},{"line_number":4944,"context_line":"            LOG.warning(_LW(\"Detach interface failed, vif\u003d%(vif)s,\""},{"line_number":4945,"context_line":"                            \"reason: %(msg)s\"),"},{"line_number":4946,"context_line":"                        {\u0027vif\u0027: condemned, \u0027msg\u0027: ex}, instance\u003dinstance)"},{"line_number":4947,"context_line":"            raise exception.InterfaceDetachFailed(instance_uuid\u003dinstance.uuid)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a80dd14_39eb0414","line":4944,"in_reply_to":"9a80dd14_021eceba","updated":"2015-03-16 15:19:28.000000000","message":"agreed, port_id is enough, since line 4940 give a port_id as exception information too.","commit_id":"37cf1c877065db447097ea09ec229854a44f9051"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"3c89b973cdf634d223efba7f5643f4c2774dd030","unresolved":false,"context_lines":[{"line_number":4941,"context_line":"        try:"},{"line_number":4942,"context_line":"            self.driver.detach_interface(instance, condemned)"},{"line_number":4943,"context_line":"        except exception.NovaException as ex:"},{"line_number":4944,"context_line":"            LOG.warning(_LW(\"Detach interface failed, vif\u003d%(vif)s,\""},{"line_number":4945,"context_line":"                            \"reason: %(msg)s\"),"},{"line_number":4946,"context_line":"                        {\u0027vif\u0027: condemned, \u0027msg\u0027: ex}, instance\u003dinstance)"},{"line_number":4947,"context_line":"            raise exception.InterfaceDetachFailed(instance_uuid\u003dinstance.uuid)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a80dd14_766bcbf0","line":4944,"in_reply_to":"9a80dd14_021eceba","updated":"2015-03-16 15:13:04.000000000","message":"hi matt, thanks for the reviewing, will switch port_id.\nbtw, can you kindly tell where can I find the warning log ? from unit test or a really test environment?\n\nthanks, Eli","commit_id":"37cf1c877065db447097ea09ec229854a44f9051"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"f3fe91db4be3fd14cff3df5ef482c12e9b6d1582","unresolved":false,"context_lines":[{"line_number":4941,"context_line":"        try:"},{"line_number":4942,"context_line":"            self.driver.detach_interface(instance, condemned)"},{"line_number":4943,"context_line":"        except exception.NovaException as ex:"},{"line_number":4944,"context_line":"            LOG.warning(_LW(\"Detach interface failed, vif\u003d%(vif)s,\""},{"line_number":4945,"context_line":"                            \"reason: %(msg)s\"),"},{"line_number":4946,"context_line":"                        {\u0027vif\u0027: condemned, \u0027msg\u0027: ex}, instance\u003dinstance)"},{"line_number":4947,"context_line":"            raise exception.InterfaceDetachFailed(instance_uuid\u003dinstance.uuid)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a80dd14_7f29c588","line":4944,"in_reply_to":"9a80dd14_593198a0","updated":"2015-03-16 23:14:54.000000000","message":"nice, thank you very much, good to know it.","commit_id":"37cf1c877065db447097ea09ec229854a44f9051"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cf3f18f1e2cad522c524e8803edf3dbd98549c5b","unresolved":false,"context_lines":[{"line_number":4941,"context_line":"        try:"},{"line_number":4942,"context_line":"            self.driver.detach_interface(instance, condemned)"},{"line_number":4943,"context_line":"        except exception.NovaException as ex:"},{"line_number":4944,"context_line":"            LOG.warning(_LW(\"Detach interface failed, vif\u003d%(vif)s,\""},{"line_number":4945,"context_line":"                            \"reason: %(msg)s\"),"},{"line_number":4946,"context_line":"                        {\u0027vif\u0027: condemned, \u0027msg\u0027: ex}, instance\u003dinstance)"},{"line_number":4947,"context_line":"            raise exception.InterfaceDetachFailed(instance_uuid\u003dinstance.uuid)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a80dd14_593198a0","line":4944,"in_reply_to":"9a80dd14_766bcbf0","updated":"2015-03-16 15:19:34.000000000","message":"Eli, it\u0027s in the subunit logs for the gate-nova-python27 job results on your change, i.e.:\n\nhttp://logs.openstack.org/51/130151/7/check/gate-nova-python27/8ca11f4/subunit_log.txt.gz\n\nThen I just searched for the test case name to find the trace.","commit_id":"37cf1c877065db447097ea09ec229854a44f9051"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"adc778731db650d23c6c5782b8aeb8b433362b55","unresolved":false,"context_lines":[{"line_number":9165,"context_line":"            self.assertRaises(exception.InterfaceDetachFailed,"},{"line_number":9166,"context_line":"                              self.compute.detach_interface, self.context,"},{"line_number":9167,"context_line":"                              instance, port_id)"},{"line_number":9168,"context_line":"            mock_deallocate.assert_not_called()"},{"line_number":9169,"context_line":""},{"line_number":9170,"context_line":"    def test_attach_volume(self):"},{"line_number":9171,"context_line":"        fake_bdm \u003d fake_block_device.FakeDbBlockDeviceDict("}],"source_content_type":"text/x-python","patch_set":4,"id":"1a930d6b_ad4a7952","line":9168,"updated":"2015-01-27 00:31:31.000000000","message":"assert_not_called() isn\u0027t a mock method.  Use self.assertFalse(mock_deallocate.called) instead.","commit_id":"d737cfa0537894945d665e3a362cc4ca1beae08d"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"fcb4fb2db129004f229a7449648752eb14d03ef1","unresolved":false,"context_lines":[{"line_number":9165,"context_line":"            self.assertRaises(exception.InterfaceDetachFailed,"},{"line_number":9166,"context_line":"                              self.compute.detach_interface, self.context,"},{"line_number":9167,"context_line":"                              instance, port_id)"},{"line_number":9168,"context_line":"            mock_deallocate.assert_not_called()"},{"line_number":9169,"context_line":""},{"line_number":9170,"context_line":"    def test_attach_volume(self):"},{"line_number":9171,"context_line":"        fake_bdm \u003d fake_block_device.FakeDbBlockDeviceDict("}],"source_content_type":"text/x-python","patch_set":4,"id":"fa81d914_28686e1c","line":9168,"in_reply_to":"1a930d6b_ad4a7952","updated":"2015-01-30 16:42:48.000000000","message":"+1 to Matt\u0027s suggestion","commit_id":"d737cfa0537894945d665e3a362cc4ca1beae08d"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"fd76d619661e3076843a540407e034e24f33ece5","unresolved":false,"context_lines":[{"line_number":9165,"context_line":"            self.assertRaises(exception.InterfaceDetachFailed,"},{"line_number":9166,"context_line":"                              self.compute.detach_interface, self.context,"},{"line_number":9167,"context_line":"                              instance, port_id)"},{"line_number":9168,"context_line":"            mock_deallocate.assert_not_called()"},{"line_number":9169,"context_line":""},{"line_number":9170,"context_line":"    def test_attach_volume(self):"},{"line_number":9171,"context_line":"        fake_bdm \u003d fake_block_device.FakeDbBlockDeviceDict("}],"source_content_type":"text/x-python","patch_set":4,"id":"da86d52c_d65f7ce3","line":9168,"in_reply_to":"fa81d914_28686e1c","updated":"2015-02-11 09:37:05.000000000","message":"Done","commit_id":"d737cfa0537894945d665e3a362cc4ca1beae08d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f07f3f19ffd5871927293a2ea3a4fc5db3cf4c41","unresolved":false,"context_lines":[{"line_number":9120,"context_line":"            self.assertRaises(exception.InterfaceDetachFailed,"},{"line_number":9121,"context_line":"                              self.compute.detach_interface, self.context,"},{"line_number":9122,"context_line":"                              instance, port_id)"},{"line_number":9123,"context_line":"            self.assertFalse(mock_deallocate.called)"},{"line_number":9124,"context_line":""},{"line_number":9125,"context_line":"    def test_attach_volume(self):"},{"line_number":9126,"context_line":"        fake_bdm \u003d fake_block_device.FakeDbBlockDeviceDict("}],"source_content_type":"text/x-python","patch_set":6,"id":"9a80dd14_60cc6a62","line":9123,"updated":"2015-03-12 14:16:01.000000000","message":"Use mock_deallocate.assert_called_once_with instead to be more explicit.","commit_id":"5d3be645215c25399506b227e4d495c43e809af4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b21aed5766a9ec1791e8c416a108df9453080a62","unresolved":false,"context_lines":[{"line_number":9120,"context_line":"            self.assertRaises(exception.InterfaceDetachFailed,"},{"line_number":9121,"context_line":"                              self.compute.detach_interface, self.context,"},{"line_number":9122,"context_line":"                              instance, port_id)"},{"line_number":9123,"context_line":"            self.assertFalse(mock_deallocate.called)"},{"line_number":9124,"context_line":""},{"line_number":9125,"context_line":"    def test_attach_volume(self):"},{"line_number":9126,"context_line":"        fake_bdm \u003d fake_block_device.FakeDbBlockDeviceDict("}],"source_content_type":"text/x-python","patch_set":6,"id":"9a80dd14_82a9de91","line":9123,"in_reply_to":"9a80dd14_1f28fc31","updated":"2015-03-16 14:51:11.000000000","message":"Oh nevermind, I was looking at this wrong, sorry. :).","commit_id":"5d3be645215c25399506b227e4d495c43e809af4"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"db29a20a4d28405983f1898c461c2f8174e176bb","unresolved":false,"context_lines":[{"line_number":9120,"context_line":"            self.assertRaises(exception.InterfaceDetachFailed,"},{"line_number":9121,"context_line":"                              self.compute.detach_interface, self.context,"},{"line_number":9122,"context_line":"                              instance, port_id)"},{"line_number":9123,"context_line":"            self.assertFalse(mock_deallocate.called)"},{"line_number":9124,"context_line":""},{"line_number":9125,"context_line":"    def test_attach_volume(self):"},{"line_number":9126,"context_line":"        fake_bdm \u003d fake_block_device.FakeDbBlockDeviceDict("}],"source_content_type":"text/x-python","patch_set":6,"id":"9a80dd14_1f28fc31","line":9123,"in_reply_to":"9a80dd14_60cc6a62","updated":"2015-03-16 02:28:57.000000000","message":"Hi Matt, thanks.\nbut I need to assert that mock is not called. do I still need to use assert_called_once_with?","commit_id":"5d3be645215c25399506b227e4d495c43e809af4"}]}