)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"1d5fddf5aa69b684575082b0705c12ee545419e3","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"During live migration of instance, if the live migration fails then"},{"line_number":10,"context_line":"\u0027_rollback_live_migration\u0027 method will be called. Here if the"},{"line_number":11,"context_line":"\u0027remove_volume_connection\u0027 method throw an error, see [1], method"},{"line_number":12,"context_line":"\u0027rollback_live_migration_at_destination\u0027 will not be called, see [2]."},{"line_number":13,"context_line":"This could leave resources not cleaned up on the destination, any"},{"line_number":14,"context_line":"exception from \u0027remove_volume_connection\u0027 method should be handled and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9a68dd71_aa5c2fee","line":11,"updated":"2016-01-20 12:10:53.000000000","message":"Can you describe scenario when such exception might occur?","commit_id":"549580090e288f6b70f16c323d8f2f2bb7518bef"}],"nova/compute/manager.py":[{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"1d5fddf5aa69b684575082b0705c12ee545419e3","unresolved":false,"context_lines":[{"line_number":5497,"context_line":"                try:"},{"line_number":5498,"context_line":"                    self.compute_rpcapi.remove_volume_connection("},{"line_number":5499,"context_line":"                        context, instance, bdm.volume_id, dest)"},{"line_number":5500,"context_line":"                except Exception:"},{"line_number":5501,"context_line":"                    LOG.exception(_LE(\u0027\u0027\u0027An Error occurred while removing"},{"line_number":5502,"context_line":"                         volume connection\u0027\u0027\u0027),"},{"line_number":5503,"context_line":"                                  instance\u003dinstance, volume_id\u003dbdm.volume_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_6ac15780","line":5500,"updated":"2016-01-20 12:10:53.000000000","message":"You need to add note there why we don\u0027t reraise exception. Someone might try to \"fix\" that in the future.","commit_id":"549580090e288f6b70f16c323d8f2f2bb7518bef"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"00fda7846582a6989ec96235d2293cb5b252a4b8","unresolved":false,"context_lines":[{"line_number":5503,"context_line":"                    # to handle them to make sure the cleanup work on"},{"line_number":5504,"context_line":"                    # destination can be done as expected."},{"line_number":5505,"context_line":"                    LOG.exception(_LE(\u0027\u0027\u0027An Error occurred while removing"},{"line_number":5506,"context_line":"                         volume connection\u0027\u0027\u0027),"},{"line_number":5507,"context_line":"                                  instance\u003dinstance, volume_id\u003dbdm.volume_id)"},{"line_number":5508,"context_line":""},{"line_number":5509,"context_line":"        self._notify_about_instance_usage(context, instance,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a68dd71_3fc87274","line":5506,"updated":"2016-01-20 15:37:55.000000000","message":"Using triple quotes will result in all the whitespace between \u0027removing\u0027 and \u0027volume\u0027 being added to the log message. Close the quote on the first line, and open it again on the next. Python will merge them together without the extra whitespace.","commit_id":"b30cc81bfa4cd3a6226a19dc86e59ab14cee9fad"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e341eea1fd82cf25ee92235baa082289a959126c","unresolved":false,"context_lines":[{"line_number":5493,"context_line":"                try:"},{"line_number":5494,"context_line":"                    self.compute_rpcapi.remove_volume_connection("},{"line_number":5495,"context_line":"                        context, instance, bdm.volume_id, dest)"},{"line_number":5496,"context_line":"                except Exception:"},{"line_number":5497,"context_line":"                    # Note(yuywz): In case the RPC layer or the underlying"},{"line_number":5498,"context_line":"                    # compute driver throws unexpected exception, we need"},{"line_number":5499,"context_line":"                    # to handle them to make sure the cleanup work on"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a5ae5dd_4d033bb9","line":5496,"range":{"start_line":5496,"start_character":23,"end_line":5496,"end_character":32},"updated":"2016-02-03 10:16:49.000000000","message":"In general we don\u0027t like catch-all type except block, could you be more specific what kind of exceptions are expected here?","commit_id":"f31acb50ec2725e7c57981c5d2b7e9b15160fae3"},{"author":{"_account_id":13248,"name":"Wenzhi Yu","email":"wenzhi_yu@163.com","username":"yuywz"},"change_message_id":"3c48dd4bad2e67774e2bbc0d1a12d7f21b1fe3d5","unresolved":false,"context_lines":[{"line_number":5493,"context_line":"                try:"},{"line_number":5494,"context_line":"                    self.compute_rpcapi.remove_volume_connection("},{"line_number":5495,"context_line":"                        context, instance, bdm.volume_id, dest)"},{"line_number":5496,"context_line":"                except Exception:"},{"line_number":5497,"context_line":"                    # Note(yuywz): In case the RPC layer or the underlying"},{"line_number":5498,"context_line":"                    # compute driver throws unexpected exception, we need"},{"line_number":5499,"context_line":"                    # to handle them to make sure the cleanup work on"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a5ae5dd_b24b2b2b","line":5496,"range":{"start_line":5496,"start_character":23,"end_line":5496,"end_character":32},"in_reply_to":"5a5ae5dd_4d033bb9","updated":"2016-02-03 11:33:40.000000000","message":"I understand \u0027catch-all\u0027 block is not cool, however, process of removing volume connection is kind of complicated, \u0027remove_volume_connection\u0027 method will invoke compute driver and cinder api. So I think \u0027catch_all\u0027 is the best way to make sure any exception from remove_volume_connection will be handled.","commit_id":"f31acb50ec2725e7c57981c5d2b7e9b15160fae3"},{"author":{"_account_id":10439,"name":"Xi Yang","email":"yang.xi@99cloud.net","username":"phenom"},"change_message_id":"647626a5ddced01b14104cdced3bbf6be14a8a6d","unresolved":false,"context_lines":[{"line_number":5547,"context_line":"                    # to handle them to make sure the cleanup work on"},{"line_number":5548,"context_line":"                    # destination can be done as expected."},{"line_number":5549,"context_line":"                    LOG.exception(_LE(\u0027An Error occurred while removing \u0027"},{"line_number":5550,"context_line":"                                      \u0027volume connection\u0027),"},{"line_number":5551,"context_line":"                                  instance\u003dinstance, volume_id\u003dbdm.volume_id)"},{"line_number":5552,"context_line":""},{"line_number":5553,"context_line":"        self._notify_about_instance_usage(context, instance,"}],"source_content_type":"text/x-python","patch_set":6,"id":"bae84128_1873e481","line":5550,"range":{"start_line":5550,"start_character":0,"end_line":5550,"end_character":38},"updated":"2016-02-20 09:59:07.000000000","message":"Suggest to log the detail message of the exception.","commit_id":"3ffc05f3ffc60fede930613b82b2ccdb40352155"},{"author":{"_account_id":13248,"name":"Wenzhi Yu","email":"wenzhi_yu@163.com","username":"yuywz"},"change_message_id":"224b544ba23aaa626ced8b347225427745052dd4","unresolved":false,"context_lines":[{"line_number":5547,"context_line":"                    # to handle them to make sure the cleanup work on"},{"line_number":5548,"context_line":"                    # destination can be done as expected."},{"line_number":5549,"context_line":"                    LOG.exception(_LE(\u0027An Error occurred while removing \u0027"},{"line_number":5550,"context_line":"                                      \u0027volume connection\u0027),"},{"line_number":5551,"context_line":"                                  instance\u003dinstance, volume_id\u003dbdm.volume_id)"},{"line_number":5552,"context_line":""},{"line_number":5553,"context_line":"        self._notify_about_instance_usage(context, instance,"}],"source_content_type":"text/x-python","patch_set":6,"id":"bae84128_e4cda0b8","line":5550,"range":{"start_line":5550,"start_character":0,"end_line":5550,"end_character":38},"in_reply_to":"bae84128_1873e481","updated":"2016-02-22 01:10:21.000000000","message":"Done, thanks for your comment.","commit_id":"3ffc05f3ffc60fede930613b82b2ccdb40352155"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"41c939b8feb08b77b1beafcbd3963058be868c98","unresolved":false,"context_lines":[{"line_number":5572,"context_line":"                try:"},{"line_number":5573,"context_line":"                    self.compute_rpcapi.remove_volume_connection("},{"line_number":5574,"context_line":"                        context, bdm.volume_id, instance, dest)"},{"line_number":5575,"context_line":"                except Exception as error:"},{"line_number":5576,"context_line":"                    # Note(yuywz): In case the RPC layer or the underlying"},{"line_number":5577,"context_line":"                    # compute driver throws unexpected exception, we need"},{"line_number":5578,"context_line":"                    # to handle them to make sure the cleanup work on"}],"source_content_type":"text/x-python","patch_set":10,"id":"9aed3d3a_7bb3bae9","line":5575,"updated":"2016-02-29 08:06:02.000000000","message":"You don\u0027t reraise here so this will skip @wrap_instance_fault which is not very good. I don\u0027t have any idea yet how you can fix it, i\u0027m thinking of putting this is in another method and adding finally clause.","commit_id":"86564a4f5b9e3f890211eb71190fd33dedcab58b"},{"author":{"_account_id":13248,"name":"Wenzhi Yu","email":"wenzhi_yu@163.com","username":"yuywz"},"change_message_id":"9bfd64ceffd6234565829aa4fcfa780efa035377","unresolved":false,"context_lines":[{"line_number":5572,"context_line":"                try:"},{"line_number":5573,"context_line":"                    self.compute_rpcapi.remove_volume_connection("},{"line_number":5574,"context_line":"                        context, bdm.volume_id, instance, dest)"},{"line_number":5575,"context_line":"                except Exception as error:"},{"line_number":5576,"context_line":"                    # Note(yuywz): In case the RPC layer or the underlying"},{"line_number":5577,"context_line":"                    # compute driver throws unexpected exception, we need"},{"line_number":5578,"context_line":"                    # to handle them to make sure the cleanup work on"}],"source_content_type":"text/x-python","patch_set":10,"id":"9aed3d3a_ae94e242","line":5575,"in_reply_to":"9aed3d3a_7bb3bae9","updated":"2016-02-29 08:30:19.000000000","message":"Hi Pawel, thanks for your comment! \nI\u0027m considering put the whole \u0027for loop\u0027 into a new function \u0027remove_volume_connection_for_bdms()\u0027, then add try-except- finally block around the \u0027remove_volume_connection_for_bdms()\u0027 method to handle the clean up work and reraise the exception.\nDo you have any better suggestion?","commit_id":"86564a4f5b9e3f890211eb71190fd33dedcab58b"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"517353de8dd1646378f91fd76026f762a158bb95","unresolved":false,"context_lines":[{"line_number":5572,"context_line":"                try:"},{"line_number":5573,"context_line":"                    self.compute_rpcapi.remove_volume_connection("},{"line_number":5574,"context_line":"                        context, bdm.volume_id, instance, dest)"},{"line_number":5575,"context_line":"                except Exception as error:"},{"line_number":5576,"context_line":"                    # Note(yuywz): In case the RPC layer or the underlying"},{"line_number":5577,"context_line":"                    # compute driver throws unexpected exception, we need"},{"line_number":5578,"context_line":"                    # to handle them to make sure the cleanup work on"}],"source_content_type":"text/x-python","patch_set":10,"id":"9aed3d3a_11bd2f02","line":5575,"in_reply_to":"9aed3d3a_ae94e242","updated":"2016-02-29 08:50:15.000000000","message":"This probably is the best we can do here. I\u0027m only worrying that rollback_live_migration_at_destination might fail and raise exception, this will cause an exception in finally clause and we will hide one instance fault, but let\u0027s try it","commit_id":"86564a4f5b9e3f890211eb71190fd33dedcab58b"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"a78751ea810bfd45bee476ac60b1368b5dee874e","unresolved":false,"context_lines":[{"line_number":5576,"context_line":""},{"line_number":5577,"context_line":"        try:"},{"line_number":5578,"context_line":"            _remove_volume_connection_for_bdms(context, bdms, instance, dest)"},{"line_number":5579,"context_line":"        except Exception as error:"},{"line_number":5580,"context_line":"            # Note(yuywz): In case the RPC layer or the underlying compute"},{"line_number":5581,"context_line":"            # driver throws unexpected exception when removing volume"},{"line_number":5582,"context_line":"            # connection, we need to handle them to make sure the cleanup work"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aed3d3a_0ffb367a","line":5579,"updated":"2016-02-29 14:52:20.000000000","message":"I will leave open question here. At this point migration_status can be \u0027failed\u0027 or \u0027aborted\u0027. In case exception is caught there, should it force migration status to be \u0027failed\u0027? I think having this distinction between \u0027aborted\u0027 and \u0027failed\u0027 is good, but someone can have other opinion.","commit_id":"33d9bb5a0593f4d9ba5debaff81d9bf4cca0816a"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"00fda7846582a6989ec96235d2293cb5b252a4b8","unresolved":false,"context_lines":[{"line_number":5934,"context_line":"        @mock.patch.object(self.compute, \u0027_live_migration_cleanup_flags\u0027)"},{"line_number":5935,"context_line":"        @mock.patch.object(self.compute, \u0027network_api\u0027)"},{"line_number":5936,"context_line":"        def _test(mock_nw_api, mock_lmcf, mock_cpt_api, mock_roll_at_dest):"},{"line_number":5937,"context_line":"            mock_lmcf.return_value \u003d True, False"},{"line_number":5938,"context_line":"            mock_cpt_api.remove_volume_connection.side_effect \u003d exception"},{"line_number":5939,"context_line":"            self.compute._rollback_live_migration(c, instance, \u0027foo\u0027,"},{"line_number":5940,"context_line":"                                                  False,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a68dd71_3fe5d2a7","line":5937,"updated":"2016-01-20 15:37:55.000000000","message":"Please add parentheses around the assigned values:\n\n    mock_lmcf.return_value \u003d (True, False)","commit_id":"b30cc81bfa4cd3a6226a19dc86e59ab14cee9fad"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"00fda7846582a6989ec96235d2293cb5b252a4b8","unresolved":false,"context_lines":[{"line_number":5941,"context_line":"                                                  migrate_data\u003dmigrate_data)"},{"line_number":5942,"context_line":"            mock_nw_api.setup_networks_on_host.assert_called_once_with("},{"line_number":5943,"context_line":"                c, instance, self.compute.host)"},{"line_number":5944,"context_line":"            mock_cpt_api.rollback_live_migration_at_destination.\\"},{"line_number":5945,"context_line":"                assert_called_once_with("},{"line_number":5946,"context_line":"                    c, instance, \u0027foo\u0027,"},{"line_number":5947,"context_line":"                    destroy_disks\u003dFalse, migrate_data\u003dmigrate_data)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a68dd71_ff309a09","line":5944,"updated":"2016-01-20 15:37:55.000000000","message":"You can remove the need for the line continuation backslash as follows:\n\n    rollback \u003d mock_cpt_api.rollback_live_migration_at_destination\n    rollback.assert_called_once_with(c, instance, ...","commit_id":"b30cc81bfa4cd3a6226a19dc86e59ab14cee9fad"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e341eea1fd82cf25ee92235baa082289a959126c","unresolved":false,"context_lines":[{"line_number":5963,"context_line":"        @mock.patch.object(self.compute, \u0027network_api\u0027)"},{"line_number":5964,"context_line":"        def _test(mock_nw_api, mock_lmcf, mock_cpt_api, mock_roll_at_dest):"},{"line_number":5965,"context_line":"            mock_lmcf.return_value \u003d (True, False)"},{"line_number":5966,"context_line":"            mock_cpt_api.remove_volume_connection.side_effect \u003d exception"},{"line_number":5967,"context_line":"            self.compute._rollback_live_migration(c, instance, \u0027foo\u0027,"},{"line_number":5968,"context_line":"                                                  False,"},{"line_number":5969,"context_line":"                                                  migrate_data\u003dmigrate_data)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a5ae5dd_6def17b1","line":5966,"range":{"start_line":5966,"start_character":64,"end_line":5966,"end_character":73},"updated":"2016-02-03 10:16:49.000000000","message":"this means that your test throws the nova.exception module as an exception object. I think this is not what you want.","commit_id":"f31acb50ec2725e7c57981c5d2b7e9b15160fae3"},{"author":{"_account_id":13248,"name":"Wenzhi Yu","email":"wenzhi_yu@163.com","username":"yuywz"},"change_message_id":"3c48dd4bad2e67774e2bbc0d1a12d7f21b1fe3d5","unresolved":false,"context_lines":[{"line_number":5963,"context_line":"        @mock.patch.object(self.compute, \u0027network_api\u0027)"},{"line_number":5964,"context_line":"        def _test(mock_nw_api, mock_lmcf, mock_cpt_api, mock_roll_at_dest):"},{"line_number":5965,"context_line":"            mock_lmcf.return_value \u003d (True, False)"},{"line_number":5966,"context_line":"            mock_cpt_api.remove_volume_connection.side_effect \u003d exception"},{"line_number":5967,"context_line":"            self.compute._rollback_live_migration(c, instance, \u0027foo\u0027,"},{"line_number":5968,"context_line":"                                                  False,"},{"line_number":5969,"context_line":"                                                  migrate_data\u003dmigrate_data)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a5ae5dd_5263d7b1","line":5966,"range":{"start_line":5966,"start_character":64,"end_line":5966,"end_character":73},"in_reply_to":"5a5ae5dd_6def17b1","updated":"2016-02-03 11:33:40.000000000","message":"Done","commit_id":"f31acb50ec2725e7c57981c5d2b7e9b15160fae3"},{"author":{"_account_id":18337,"name":"Sujitha","email":"sujitha.neti@intel.com","username":"Sujitha"},"change_message_id":"817a00fac60a7d4fdbec9f14ba53bea5765862aa","unresolved":false,"context_lines":[{"line_number":5877,"context_line":"        @mock.patch.object(self.compute, \u0027compute_rpcapi\u0027)"},{"line_number":5878,"context_line":"        @mock.patch.object(self.compute, \u0027_live_migration_cleanup_flags\u0027)"},{"line_number":5879,"context_line":"        @mock.patch.object(self.compute, \u0027network_api\u0027)"},{"line_number":5880,"context_line":"        def _test(mock_nw_api, mock_lmcf, mock_cpt_api, mock_roll_at_dest):"},{"line_number":5881,"context_line":"            mock_lmcf.return_value \u003d (True, False)"},{"line_number":5882,"context_line":"            rm_vol \u003d mock_cpt_api.remove_volume_connection"},{"line_number":5883,"context_line":"            rm_vol.side_effect \u003d test.TestingException"}],"source_content_type":"text/x-python","patch_set":7,"id":"bae84128_c532b7f1","line":5880,"updated":"2016-02-23 17:52:15.000000000","message":"mock_roll_at_dest is not being used anywhere in the test. Test runs successfully even when @mock.patch.object(self.compute,                     \u0027rollback_live_migration_at_destination\u0027) and mock_roll_at_dest are removed.","commit_id":"bab31e54833183c6d50f5dd0c46553a478ec16d2"},{"author":{"_account_id":13248,"name":"Wenzhi Yu","email":"wenzhi_yu@163.com","username":"yuywz"},"change_message_id":"cbce1276b2a551b382f1e8e0bb1b73f678fe2e19","unresolved":false,"context_lines":[{"line_number":5877,"context_line":"        @mock.patch.object(self.compute, \u0027compute_rpcapi\u0027)"},{"line_number":5878,"context_line":"        @mock.patch.object(self.compute, \u0027_live_migration_cleanup_flags\u0027)"},{"line_number":5879,"context_line":"        @mock.patch.object(self.compute, \u0027network_api\u0027)"},{"line_number":5880,"context_line":"        def _test(mock_nw_api, mock_lmcf, mock_cpt_api, mock_roll_at_dest):"},{"line_number":5881,"context_line":"            mock_lmcf.return_value \u003d (True, False)"},{"line_number":5882,"context_line":"            rm_vol \u003d mock_cpt_api.remove_volume_connection"},{"line_number":5883,"context_line":"            rm_vol.side_effect \u003d test.TestingException"}],"source_content_type":"text/x-python","patch_set":7,"id":"bae84128_d9d0cccb","line":5880,"in_reply_to":"bae84128_c532b7f1","updated":"2016-02-24 03:00:48.000000000","message":"Done, thanks for your comment!","commit_id":"bab31e54833183c6d50f5dd0c46553a478ec16d2"},{"author":{"_account_id":20758,"name":"He Xin","email":"hxhehx@cn.ibm.com","username":"hexin"},"change_message_id":"ad1b5286987f916a6b66ccd348cdfba3c1812104","unresolved":false,"context_lines":[{"line_number":5872,"context_line":"        self.assertEqual(\u0027fake\u0027, migration.status)"},{"line_number":5873,"context_line":"        migration.save.assert_called_once_with()"},{"line_number":5874,"context_line":""},{"line_number":5875,"context_line":"    @mock.patch(\u0027nova.objects.BlockDeviceMappingList.get_by_instance_uuid\u0027)"},{"line_number":5876,"context_line":"    def test_rollback_live_migration_remove_volume_exception(self, mock_bdms):"},{"line_number":5877,"context_line":"        c \u003d context.get_admin_context()"},{"line_number":5878,"context_line":"        instance \u003d mock.MagicMock()"}],"source_content_type":"text/x-python","patch_set":9,"id":"9aed3d3a_95dad4e6","line":5875,"updated":"2016-02-29 02:27:33.000000000","message":"To improve readability,I suggest to move the mock decrator to _test method","commit_id":"8f8f55683d074d2b4b4c259ef7b5061085b69e6c"},{"author":{"_account_id":13248,"name":"Wenzhi Yu","email":"wenzhi_yu@163.com","username":"yuywz"},"change_message_id":"8f86ad1b6ea4d6200c9e726fc3354603261fa73a","unresolved":false,"context_lines":[{"line_number":5872,"context_line":"        self.assertEqual(\u0027fake\u0027, migration.status)"},{"line_number":5873,"context_line":"        migration.save.assert_called_once_with()"},{"line_number":5874,"context_line":""},{"line_number":5875,"context_line":"    @mock.patch(\u0027nova.objects.BlockDeviceMappingList.get_by_instance_uuid\u0027)"},{"line_number":5876,"context_line":"    def test_rollback_live_migration_remove_volume_exception(self, mock_bdms):"},{"line_number":5877,"context_line":"        c \u003d context.get_admin_context()"},{"line_number":5878,"context_line":"        instance \u003d mock.MagicMock()"}],"source_content_type":"text/x-python","patch_set":9,"id":"9aed3d3a_d89ad16d","line":5875,"in_reply_to":"9aed3d3a_95dad4e6","updated":"2016-02-29 02:54:06.000000000","message":"Done, thanks for your comment, hexin.","commit_id":"8f8f55683d074d2b4b4c259ef7b5061085b69e6c"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"8cb809d6649bed5a3716e76066310624a0775acd","unresolved":false,"context_lines":[{"line_number":5885,"context_line":"        @mock.patch.object(self.compute, \u0027network_api\u0027)"},{"line_number":5886,"context_line":"        @mock.patch(\u0027nova.objects.BlockDeviceMappingList.get_by_instance_uuid\u0027)"},{"line_number":5887,"context_line":"        def _test(mock_bdms, mock_nw_api, mock_lmcf, mock_cpt_api):"},{"line_number":5888,"context_line":"            mock_bdms.return_value \u003d []"},{"line_number":5889,"context_line":"            mock_lmcf.return_value \u003d (True, False)"},{"line_number":5890,"context_line":"            rm_vol \u003d mock_cpt_api.remove_volume_connection"},{"line_number":5891,"context_line":"            rm_vol.side_effect \u003d test.TestingException"}],"source_content_type":"text/x-python","patch_set":15,"id":"5a18252c_3c680287","line":5888,"range":{"start_line":5888,"start_character":37,"end_line":5888,"end_character":39},"updated":"2016-04-14 11:23:55.000000000","message":"did this really test the flow you added ? \n\nreturn [] means \u0027_remove_volume_connection_for_bdms\u0027 not called?","commit_id":"628d78ab8e44c5afbd1560dae646e25446b2d05f"},{"author":{"_account_id":13248,"name":"Wenzhi Yu","email":"wenzhi_yu@163.com","username":"yuywz"},"change_message_id":"d36f7eb6e7af16f78c9314575bf436d0a23dac2d","unresolved":false,"context_lines":[{"line_number":5885,"context_line":"        @mock.patch.object(self.compute, \u0027network_api\u0027)"},{"line_number":5886,"context_line":"        @mock.patch(\u0027nova.objects.BlockDeviceMappingList.get_by_instance_uuid\u0027)"},{"line_number":5887,"context_line":"        def _test(mock_bdms, mock_nw_api, mock_lmcf, mock_cpt_api):"},{"line_number":5888,"context_line":"            mock_bdms.return_value \u003d []"},{"line_number":5889,"context_line":"            mock_lmcf.return_value \u003d (True, False)"},{"line_number":5890,"context_line":"            rm_vol \u003d mock_cpt_api.remove_volume_connection"},{"line_number":5891,"context_line":"            rm_vol.side_effect \u003d test.TestingException"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a122d0e_340ffce2","line":5888,"range":{"start_line":5888,"start_character":37,"end_line":5888,"end_character":39},"in_reply_to":"5a18252c_3c680287","updated":"2016-04-15 16:32:36.000000000","message":"oh yes, thx for pointing this out.","commit_id":"628d78ab8e44c5afbd1560dae646e25446b2d05f"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"8cb809d6649bed5a3716e76066310624a0775acd","unresolved":false,"context_lines":[{"line_number":5888,"context_line":"            mock_bdms.return_value \u003d []"},{"line_number":5889,"context_line":"            mock_lmcf.return_value \u003d (True, False)"},{"line_number":5890,"context_line":"            rm_vol \u003d mock_cpt_api.remove_volume_connection"},{"line_number":5891,"context_line":"            rm_vol.side_effect \u003d test.TestingException"},{"line_number":5892,"context_line":"            self.compute._rollback_live_migration(c, instance, \u0027foo\u0027,"},{"line_number":5893,"context_line":"                                                  False,"},{"line_number":5894,"context_line":"                                                  migrate_data\u003dmigrate_data)"}],"source_content_type":"text/x-python","patch_set":15,"id":"5a18252c_5c9e9e63","line":5891,"range":{"start_line":5891,"start_character":38,"end_line":5891,"end_character":54},"updated":"2016-04-14 11:23:55.000000000","message":"this side effect should makes the function raise Exception? but test didn\u0027t show that","commit_id":"628d78ab8e44c5afbd1560dae646e25446b2d05f"},{"author":{"_account_id":13248,"name":"Wenzhi Yu","email":"wenzhi_yu@163.com","username":"yuywz"},"change_message_id":"d36f7eb6e7af16f78c9314575bf436d0a23dac2d","unresolved":false,"context_lines":[{"line_number":5888,"context_line":"            mock_bdms.return_value \u003d []"},{"line_number":5889,"context_line":"            mock_lmcf.return_value \u003d (True, False)"},{"line_number":5890,"context_line":"            rm_vol \u003d mock_cpt_api.remove_volume_connection"},{"line_number":5891,"context_line":"            rm_vol.side_effect \u003d test.TestingException"},{"line_number":5892,"context_line":"            self.compute._rollback_live_migration(c, instance, \u0027foo\u0027,"},{"line_number":5893,"context_line":"                                                  False,"},{"line_number":5894,"context_line":"                                                  migrate_data\u003dmigrate_data)"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a122d0e_141480f3","line":5891,"range":{"start_line":5891,"start_character":38,"end_line":5891,"end_character":54},"in_reply_to":"5a18252c_5c9e9e63","updated":"2016-04-15 16:32:36.000000000","message":"Done","commit_id":"628d78ab8e44c5afbd1560dae646e25446b2d05f"}]}