)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1115fa3680a1bdf8fc54183ab47fbd88481ca1e0","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Also add a simple test ensuring that refresh_connection_info is called"},{"line_number":10,"context_line":"for each DriverVolumeBlockDevice provided in block_device_mapping."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ib1ff00e7f4f5b599317d7111c322ce9af8a9a2b1"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"bacf61ea_a2ca799f","line":11,"updated":"2016-07-28 14:07:19.000000000","message":"If this is a bug, mark it as closes-bug: #1419577","commit_id":"5cd2743c5bf32dd026601b95325d42e8c3ee1d6c"}],"nova/tests/unit/virt/test_block_device.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3a187a80a579b2ddc68ea15ab42c6b7e7b6bc4a8","unresolved":false,"context_lines":[{"line_number":1072,"context_line":"            mock.call(mock.sentinel.refresh_context,"},{"line_number":1073,"context_line":"                      mock.sentinel.refresh_instance,"},{"line_number":1074,"context_line":"                      mock.sentinel.refresh_vol_api,"},{"line_number":1075,"context_line":"                      mock.sentinel.refresh_virt_drv)])"}],"source_content_type":"text/x-python","patch_set":8,"id":"bacf61ea_f904237f","line":1075,"updated":"2016-07-28 15:12:48.000000000","message":"This doesn\u0027t really ensure that we called refresh on each of the BDMs, it just asserts that there were two calls. What about using a MagicMock instead of the real driver classes? Then you could check the calls on each of them.\n\nIf not, then maybe mock the actual test_volume_1.refresh_connection_info explicitly. But I think the MagicMock approach would be fine and simpler.","commit_id":"71e4548c249440ecb7a1c08bda64a795a6bd11e6"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"906fde54ce05c41861893c1c2c6bdecd7b7a92e0","unresolved":false,"context_lines":[{"line_number":1056,"context_line":"    def test_refresh_conn_infos(self):"},{"line_number":1057,"context_line":"        test_volume_1 \u003d mock.MagicMock()"},{"line_number":1058,"context_line":"        test_volume_2 \u003d mock.MagicMock()"},{"line_number":1059,"context_line":"        block_device_mapping \u003d [test_volume_1, test_volume_2]"},{"line_number":1060,"context_line":"        driver_block_device.refresh_conn_infos(block_device_mapping,"},{"line_number":1061,"context_line":"                                               mock.sentinel.refresh_context,"},{"line_number":1062,"context_line":"                                               mock.sentinel.refresh_instance,"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a8bc5a2_2a73914e","line":1059,"updated":"2016-09-23 20:13:12.000000000","message":"a volume that doesn\u0027t have refresh_connection_info, to cover that case, would be nice.","commit_id":"bf10cd8065662991b63546703fec2a0f3d015ea4"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"906fde54ce05c41861893c1c2c6bdecd7b7a92e0","unresolved":false,"context_lines":[{"line_number":1062,"context_line":"                                               mock.sentinel.refresh_instance,"},{"line_number":1063,"context_line":"                                               mock.sentinel.refresh_vol_api,"},{"line_number":1064,"context_line":"                                               mock.sentinel.refresh_virt_drv)"},{"line_number":1065,"context_line":"        test_volume_1.refresh_connection_info.assert_called_with("},{"line_number":1066,"context_line":"            mock.sentinel.refresh_context,"},{"line_number":1067,"context_line":"            mock.sentinel.refresh_instance,"},{"line_number":1068,"context_line":"            mock.sentinel.refresh_vol_api,"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a8bc5a2_4a89bd25","line":1065,"updated":"2016-09-23 20:13:12.000000000","message":"assert_called_once_with would be preferable.","commit_id":"bf10cd8065662991b63546703fec2a0f3d015ea4"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"906fde54ce05c41861893c1c2c6bdecd7b7a92e0","unresolved":false,"context_lines":[{"line_number":1068,"context_line":"            mock.sentinel.refresh_vol_api,"},{"line_number":1069,"context_line":"            mock.sentinel.refresh_virt_drv"},{"line_number":1070,"context_line":"        )"},{"line_number":1071,"context_line":"        test_volume_1.refresh_connection_info.assert_called_with("},{"line_number":1072,"context_line":"            mock.sentinel.refresh_context,"},{"line_number":1073,"context_line":"            mock.sentinel.refresh_instance,"},{"line_number":1074,"context_line":"            mock.sentinel.refresh_vol_api,"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a8bc5a2_6a8cb935","line":1071,"updated":"2016-09-23 20:13:12.000000000","message":"ditto.","commit_id":"bf10cd8065662991b63546703fec2a0f3d015ea4"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"5e76678cc7a2638b1b5daaa3706e311deda594ca","unresolved":false,"context_lines":[{"line_number":1098,"context_line":"            mock.sentinel.refresh_instance,"},{"line_number":1099,"context_line":"            mock.sentinel.refresh_vol_api,"},{"line_number":1100,"context_line":"            mock.sentinel.refresh_virt_drv"},{"line_number":1101,"context_line":"        )"},{"line_number":1102,"context_line":"        # NOTE(lyarwood): Can\u0027t think of a better way of testing this as we"},{"line_number":1103,"context_line":"        # can\u0027t assert_not_called if the method isn\u0027t in the spec."},{"line_number":1104,"context_line":"        self.assertFalse(hasattr(test_eph, \u0027refresh_connection_info\u0027))"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa6399be_5c947136","line":1101,"updated":"2016-10-11 12:56:45.000000000","message":"Great point, I\u0027ll change this now.","commit_id":"a375a47bfca1b09878e3c63c445014c4506ec160"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"31ec9e484022c850a3dd83b9254e2009f86d9b9a","unresolved":false,"context_lines":[{"line_number":1070,"context_line":"            spec\u003ddriver_block_device.DriverSwapBlockDevice)"},{"line_number":1071,"context_line":"        block_device_mapping \u003d [test_volume, test_image, test_eph,"},{"line_number":1072,"context_line":"                                test_snapshot, test_swap, test_blank]"},{"line_number":1073,"context_line":"        driver_block_device.refresh_conn_infos(block_device_mapping,"},{"line_number":1074,"context_line":"                                               mock.sentinel.refresh_context,"},{"line_number":1075,"context_line":"                                               mock.sentinel.refresh_instance,"},{"line_number":1076,"context_line":"                                               mock.sentinel.refresh_vol_api,"},{"line_number":1077,"context_line":"                                               mock.sentinel.refresh_virt_drv)"},{"line_number":1078,"context_line":"        test_volume.refresh_connection_info.assert_called_once_with("},{"line_number":1079,"context_line":"            mock.sentinel.refresh_context,"},{"line_number":1080,"context_line":"            mock.sentinel.refresh_instance,"},{"line_number":1081,"context_line":"            mock.sentinel.refresh_vol_api,"},{"line_number":1082,"context_line":"            mock.sentinel.refresh_virt_drv"},{"line_number":1083,"context_line":"        )"},{"line_number":1084,"context_line":"        test_image.refresh_connection_info.assert_called_once_with("},{"line_number":1085,"context_line":"            mock.sentinel.refresh_context,"},{"line_number":1086,"context_line":"            mock.sentinel.refresh_instance,"},{"line_number":1087,"context_line":"            mock.sentinel.refresh_vol_api,"},{"line_number":1088,"context_line":"            mock.sentinel.refresh_virt_drv"},{"line_number":1089,"context_line":"        )"},{"line_number":1090,"context_line":"        test_snapshot.refresh_connection_info.assert_called_once_with("},{"line_number":1091,"context_line":"            mock.sentinel.refresh_context,"},{"line_number":1092,"context_line":"            mock.sentinel.refresh_instance,"},{"line_number":1093,"context_line":"            mock.sentinel.refresh_vol_api,"},{"line_number":1094,"context_line":"            mock.sentinel.refresh_virt_drv"},{"line_number":1095,"context_line":"        )"},{"line_number":1096,"context_line":"        test_blank.refresh_connection_info.assert_called_once_with("},{"line_number":1097,"context_line":"            mock.sentinel.refresh_context,"},{"line_number":1098,"context_line":"            mock.sentinel.refresh_instance,"},{"line_number":1099,"context_line":"            mock.sentinel.refresh_vol_api,"},{"line_number":1100,"context_line":"            mock.sentinel.refresh_virt_drv"},{"line_number":1101,"context_line":"        )"},{"line_number":1102,"context_line":"        # NOTE(lyarwood): Can\u0027t think of a better way of testing this as we"},{"line_number":1103,"context_line":"        # can\u0027t assert_not_called if the method isn\u0027t in the spec."},{"line_number":1104,"context_line":"        self.assertFalse(hasattr(test_eph, \u0027refresh_connection_info\u0027))"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa6399be_993a9b35","line":1101,"range":{"start_line":1073,"start_character":0,"end_line":1101,"end_character":9},"updated":"2016-10-11 12:41:02.000000000","message":"While we do favor explicitness in tests, this really seems like needless duplication and could be easily replaced by a loop:\n\n    for test_mock in [test_volume, test_image, ...]:\n        test_mock.refresh_connection...","commit_id":"a375a47bfca1b09878e3c63c445014c4506ec160"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f55eef7a648d1e5e291ce0b99c8ef35b1d552a26","unresolved":false,"context_lines":[{"line_number":1084,"context_line":"        # NOTE(lyarwood): Can\u0027t think of a better way of testing this as we"},{"line_number":1085,"context_line":"        # can\u0027t assert_not_called if the method isn\u0027t in the spec."},{"line_number":1086,"context_line":"        self.assertFalse(hasattr(test_eph, \u0027refresh_connection_info\u0027))"},{"line_number":1087,"context_line":"        self.assertFalse(hasattr(test_swap, \u0027refresh_connection_info\u0027))"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa6399be_47e07496","line":1087,"updated":"2016-10-14 15:31:50.000000000","message":"The lack of an attribute not found error seems good. This test would break if you deleted those lines you modified, so I guess thats good.","commit_id":"b83cae02ece4c338e09c3606c6ae69b715bd6f8c"}],"nova/virt/block_device.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7c5cd9728017ed2567ec99e6e10c702c8829fc42","unresolved":false,"context_lines":[{"line_number":522,"context_line":"def refresh_conn_infos(block_device_mapping, *refresh_args, **refresh_kwargs):"},{"line_number":523,"context_line":"    refresh_func \u003d operator.methodcaller(\u0027refresh_connection_info\u0027,"},{"line_number":524,"context_line":"                                         *refresh_args, **refresh_kwargs)"},{"line_number":525,"context_line":"    list(map(refresh_func, block_device_mapping))"},{"line_number":526,"context_line":"    return block_device_mapping"},{"line_number":527,"context_line":""},{"line_number":528,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_d8186078","line":525,"updated":"2016-07-14 11:18:27.000000000","message":"A for loop would be a better fit here, tbh. This is kinda an abuse of map(). This obviously works, though, so is undoubtedly an improvement :)","commit_id":"fa18b3c6ee01a7b68b0af49c449341aed7aa3127"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d139d3036a004457df1e467edcfe59538bf91d68","unresolved":false,"context_lines":[{"line_number":518,"context_line":"def refresh_conn_infos(block_device_mapping, *refresh_args, **refresh_kwargs):"},{"line_number":519,"context_line":"    refresh_func \u003d operator.methodcaller(\u0027refresh_connection_info\u0027,"},{"line_number":520,"context_line":"                                         *refresh_args, **refresh_kwargs)"},{"line_number":521,"context_line":"    list(map(refresh_func, block_device_mapping))"},{"line_number":522,"context_line":"    return block_device_mapping"},{"line_number":523,"context_line":""},{"line_number":524,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bacf61ea_2c5682d2","line":521,"updated":"2016-07-28 13:37:39.000000000","message":"Like mdbooth said, this is a rather obscure abuse of map. On py3 it will only execute the function because you coerce to a list, which is why it broke from py2-\u003epy3 in the first place.\n\nWhy are you not just doing a for loop on block_device_mapping?","commit_id":"4b4b0f266d25b12b4de9b0073c4924d5acea7574"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3a187a80a579b2ddc68ea15ab42c6b7e7b6bc4a8","unresolved":false,"context_lines":[{"line_number":517,"context_line":"def refresh_conn_infos(block_device_mapping, *refresh_args, **refresh_kwargs):"},{"line_number":518,"context_line":"    for device in block_device_mapping:"},{"line_number":519,"context_line":"        refresh_func \u003d getattr(device, \u0027refresh_connection_info\u0027)"},{"line_number":520,"context_line":"        if callable(refresh_func):"},{"line_number":521,"context_line":"            device.refresh_connection_info(*refresh_args, **refresh_kwargs)"},{"line_number":522,"context_line":"    return block_device_mapping"},{"line_number":523,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"bacf61ea_59c0cf7c","line":520,"updated":"2016-07-28 15:12:48.000000000","message":"When would this happen? I don\u0027t think this is strictly necessary.\n\nIf you\u0027re going to keep it, you should test for it, but I think it\u0027d be better to just drop it.","commit_id":"71e4548c249440ecb7a1c08bda64a795a6bd11e6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"77e52bfccbf76ad3273f05ab163340fe32046f75","unresolved":false,"context_lines":[{"line_number":517,"context_line":"def refresh_conn_infos(block_device_mapping, *refresh_args, **refresh_kwargs):"},{"line_number":518,"context_line":"    for device in block_device_mapping:"},{"line_number":519,"context_line":"        refresh_func \u003d getattr(device, \u0027refresh_connection_info\u0027)"},{"line_number":520,"context_line":"        if callable(refresh_func):"},{"line_number":521,"context_line":"            device.refresh_connection_info(*refresh_args, **refresh_kwargs)"},{"line_number":522,"context_line":"    return block_device_mapping"},{"line_number":523,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"bacf61ea_c4821694","line":520,"in_reply_to":"bacf61ea_59c0cf7c","updated":"2016-07-28 15:30:55.000000000","message":"Okay, this is because non-volume BDMs don\u0027t have this. I think it\u0027s more than sufficient to just do a hasattr(\u0027refresh_connection_info\u0027) here, which is more compact.\n\nAlso, can you add a comment above this to indicate why you need to do this check?","commit_id":"71e4548c249440ecb7a1c08bda64a795a6bd11e6"}]}