)]}'
{"cinder/tests/unit/test_nfs.py":[{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"dbb28511cc3b18a3c961d7534191c2ab75a541e4","unresolved":false,"context_lines":[{"line_number":31,"context_line":"from cinder.volume.drivers import nfs"},{"line_number":32,"context_line":"from cinder.volume.drivers import remotefs"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"CONF \u003d cfg.CONF"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"class DumbVolume(object):"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a1a9d01_c14c8872","line":34,"updated":"2015-09-28 16:12:27.000000000","message":"Do we really need CONF for this test? If yes, we should to mock it instead of using real config","commit_id":"4a0ebde26b9a1417ca065114895f47d5971c8216"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"17a3b71c3008bcf1f69d56eab4936be0c5ddd1eb","unresolved":false,"context_lines":[{"line_number":1323,"context_line":"    @mock.patch.object(os, \u0027rename\u0027)"},{"line_number":1324,"context_line":"    def test_update_migrated_volume(self, rename_volume):"},{"line_number":1325,"context_line":"        drv \u003d nfs.NfsDriver(configuration\u003dself.configuration)"},{"line_number":1326,"context_line":"        context \u003d mock.Mock()"},{"line_number":1327,"context_line":"        fake_volume_id \u003d \u0027vol1\u0027"},{"line_number":1328,"context_line":"        fake_new_volume_id \u003d \u0027vol2\u0027"},{"line_number":1329,"context_line":"        fake_provider \u003d \u0027fake_provider\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a1a9d01_4d6eb9ca","line":1326,"updated":"2015-09-25 09:52:05.000000000","message":"nit: You can use `self.context` instead","commit_id":"4a0ebde26b9a1417ca065114895f47d5971c8216"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"17a3b71c3008bcf1f69d56eab4936be0c5ddd1eb","unresolved":false,"context_lines":[{"line_number":1336,"context_line":"                       \u0027provider_location\u0027: fake_provider,"},{"line_number":1337,"context_line":"                       \u0027_name_id\u0027: None}"},{"line_number":1338,"context_line":"        fake_new_volume \u003d {\u0027size\u0027: 1, \u0027id\u0027: fake_new_volume_id,"},{"line_number":1339,"context_line":"                           \u0027provider_location\u0027: fake_provider,"},{"line_number":1340,"context_line":"                           \u0027_name_id\u0027: None}"},{"line_number":1341,"context_line":""},{"line_number":1342,"context_line":"        with mock.patch.object(drv, \u0027local_path\u0027) as local_path:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a1a9d01_8dd05155","line":1339,"updated":"2015-09-25 09:52:05.000000000","message":"-1: This should be different than the one in `fake_volume`","commit_id":"4a0ebde26b9a1417ca065114895f47d5971c8216"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"17a3b71c3008bcf1f69d56eab4936be0c5ddd1eb","unresolved":false,"context_lines":[{"line_number":1352,"context_line":""},{"line_number":1353,"context_line":"            rename_volume.reset_mock()"},{"line_number":1354,"context_line":"            rename_volume.side_effect \u003d OSError"},{"line_number":1355,"context_line":"            update \u003d drv.update_migrated_volume(context,"},{"line_number":1356,"context_line":"                                                fake_volume,"},{"line_number":1357,"context_line":"                                                fake_new_volume,"},{"line_number":1358,"context_line":"                                                \u0027available\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a1a9d01_4d41d90a","line":1355,"updated":"2015-09-25 09:52:05.000000000","message":"nit: Is always better to only test 1 thing in each test, so this should go in another test.","commit_id":"4a0ebde26b9a1417ca065114895f47d5971c8216"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"17a3b71c3008bcf1f69d56eab4936be0c5ddd1eb","unresolved":false,"context_lines":[{"line_number":1360,"context_line":"                                                  original_volume_path)"},{"line_number":1361,"context_line":"            self.assertEqual({\u0027_name_id\u0027: fake_new_volume_id,"},{"line_number":1362,"context_line":"                              \u0027provider_location\u0027: fake_provider}, update)"},{"line_number":1363,"context_line":""},{"line_number":1364,"context_line":"    def test_retype_is_there(self):"},{"line_number":1365,"context_line":"        \"Ensure that driver.retype() is there.\"\"\""},{"line_number":1366,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9a1a9d01_8d5e11e8","line":1363,"updated":"2015-09-25 09:52:05.000000000","message":"-1: Missing \u0027in-use\u0027 test case.","commit_id":"4a0ebde26b9a1417ca065114895f47d5971c8216"}],"cinder/volume/drivers/nfs.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"17a3b71c3008bcf1f69d56eab4936be0c5ddd1eb","unresolved":false,"context_lines":[{"line_number":380,"context_line":""},{"line_number":381,"context_line":"    def update_migrated_volume(self, ctxt, volume, new_volume,"},{"line_number":382,"context_line":"                               original_volume_status):"},{"line_number":383,"context_line":"        \"\"\"Return model update from NFS for migrated volume."},{"line_number":384,"context_line":""},{"line_number":385,"context_line":"        This method should rename the back-end volume name(id) on the"},{"line_number":386,"context_line":"        destination host back to its original name(id) on the source host."}],"source_content_type":"text/x-python","patch_set":5,"id":"9a1a9d01_87827251","line":383,"updated":"2015-09-25 09:52:05.000000000","message":"nit: That\u0027s not a good description of what the method does, since primary function is to rename the volume.  Although not that important considering the rest of the docstring explains everything in detail.","commit_id":"4a0ebde26b9a1417ca065114895f47d5971c8216"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"17a3b71c3008bcf1f69d56eab4936be0c5ddd1eb","unresolved":false,"context_lines":[{"line_number":395,"context_line":"        name_id \u003d None"},{"line_number":396,"context_line":"        if original_volume_status \u003d\u003d \u0027available\u0027:"},{"line_number":397,"context_line":"            current_name \u003d CONF.volume_name_template % new_volume[\u0027id\u0027]"},{"line_number":398,"context_line":"            original_volume_name \u003d CONF.volume_name_template % volume[\u0027id\u0027]"},{"line_number":399,"context_line":"            current_path \u003d self.local_path(new_volume)"},{"line_number":400,"context_line":"            # Replace the volume name with the original volume name"},{"line_number":401,"context_line":"            original_path \u003d current_path.replace(current_name,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a1a9d01_07e8a2e1","line":398,"updated":"2015-09-25 09:52:05.000000000","message":"-1: This would not be correct if original volume had been migrated at least once before this code is in place, you should take into consideration `_name_id`.","commit_id":"4a0ebde26b9a1417ca065114895f47d5971c8216"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"ef4b4c5826b1695ab072cef50248ac1cc9427a01","unresolved":false,"context_lines":[{"line_number":395,"context_line":"        name_id \u003d None"},{"line_number":396,"context_line":"        if original_volume_status \u003d\u003d \u0027available\u0027:"},{"line_number":397,"context_line":"            current_name \u003d CONF.volume_name_template % new_volume[\u0027id\u0027]"},{"line_number":398,"context_line":"            original_volume_name \u003d CONF.volume_name_template % volume[\u0027id\u0027]"},{"line_number":399,"context_line":"            current_path \u003d self.local_path(new_volume)"},{"line_number":400,"context_line":"            # Replace the volume name with the original volume name"},{"line_number":401,"context_line":"            original_path \u003d current_path.replace(current_name,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a1a9d01_54007434","line":398,"in_reply_to":"9a1a9d01_07e8a2e1","updated":"2015-09-25 16:20:03.000000000","message":"Due to some reasons, it is possible that the volume does not have an empty value for the name Id, meaning that volume Id differs from the backend I\u0027d even before migration. \nI am actually intentional to consider volume Id instead of name Id here, because it is just a good chance here to make the backend Id the same as the volume Id again.","commit_id":"4a0ebde26b9a1417ca065114895f47d5971c8216"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9ae5a7a6bf8aad2858cdf65de6a3dd822cba8179","unresolved":false,"context_lines":[{"line_number":395,"context_line":"        name_id \u003d None"},{"line_number":396,"context_line":"        if original_volume_status \u003d\u003d \u0027available\u0027:"},{"line_number":397,"context_line":"            current_name \u003d CONF.volume_name_template % new_volume[\u0027id\u0027]"},{"line_number":398,"context_line":"            original_volume_name \u003d CONF.volume_name_template % volume[\u0027id\u0027]"},{"line_number":399,"context_line":"            current_path \u003d self.local_path(new_volume)"},{"line_number":400,"context_line":"            # Replace the volume name with the original volume name"},{"line_number":401,"context_line":"            original_path \u003d current_path.replace(current_name,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a1a9d01_47cbb572","line":398,"in_reply_to":"9a1a9d01_54007434","updated":"2015-09-28 13:16:38.000000000","message":"That actually is the smart thing to do.  :-)","commit_id":"4a0ebde26b9a1417ca065114895f47d5971c8216"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"17a3b71c3008bcf1f69d56eab4936be0c5ddd1eb","unresolved":false,"context_lines":[{"line_number":401,"context_line":"            original_path \u003d current_path.replace(current_name,"},{"line_number":402,"context_line":"                                                 original_volume_name)"},{"line_number":403,"context_line":"            try:"},{"line_number":404,"context_line":"                os.rename(current_path, original_path)"},{"line_number":405,"context_line":"            except OSError:"},{"line_number":406,"context_line":"                LOG.error(_LE(\u0027Unable to rename the logical volume \u0027"},{"line_number":407,"context_line":"                              \u0027for volume: %s\u0027), volume[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a1a9d01_67ecfec7","line":404,"updated":"2015-09-25 09:52:05.000000000","message":"-1: You should just add a `return` here, that way we don\u0027t do unnecessary DB updates just to leave the same contents it already has.","commit_id":"4a0ebde26b9a1417ca065114895f47d5971c8216"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9ae5a7a6bf8aad2858cdf65de6a3dd822cba8179","unresolved":false,"context_lines":[{"line_number":401,"context_line":"            original_path \u003d current_path.replace(current_name,"},{"line_number":402,"context_line":"                                                 original_volume_name)"},{"line_number":403,"context_line":"            try:"},{"line_number":404,"context_line":"                os.rename(current_path, original_path)"},{"line_number":405,"context_line":"            except OSError:"},{"line_number":406,"context_line":"                LOG.error(_LE(\u0027Unable to rename the logical volume \u0027"},{"line_number":407,"context_line":"                              \u0027for volume: %s\u0027), volume[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a1a9d01_1a22f052","line":404,"in_reply_to":"9a1a9d01_67ecfec7","updated":"2015-09-28 13:16:38.000000000","message":"My comment is no longer valid.","commit_id":"4a0ebde26b9a1417ca065114895f47d5971c8216"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"17a3b71c3008bcf1f69d56eab4936be0c5ddd1eb","unresolved":false,"context_lines":[{"line_number":408,"context_line":"                # If the rename fails, _name_id should be set to the new"},{"line_number":409,"context_line":"                # volume id and provider_location should be set to the"},{"line_number":410,"context_line":"                # one from the new volume as well."},{"line_number":411,"context_line":"                name_id \u003d new_volume[\u0027_name_id\u0027] or new_volume[\u0027id\u0027]"},{"line_number":412,"context_line":"        else:"},{"line_number":413,"context_line":"            # The back-end will not be renamed."},{"line_number":414,"context_line":"            name_id \u003d new_volume[\u0027_name_id\u0027] or new_volume[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a1a9d01_a730d657","line":411,"updated":"2015-09-25 09:52:05.000000000","message":"nit: If you add the `return` on success you can remove line 395, 411 and 412 and lower the indent on lines 413 and 414","commit_id":"4a0ebde26b9a1417ca065114895f47d5971c8216"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9ae5a7a6bf8aad2858cdf65de6a3dd822cba8179","unresolved":false,"context_lines":[{"line_number":408,"context_line":"                # If the rename fails, _name_id should be set to the new"},{"line_number":409,"context_line":"                # volume id and provider_location should be set to the"},{"line_number":410,"context_line":"                # one from the new volume as well."},{"line_number":411,"context_line":"                name_id \u003d new_volume[\u0027_name_id\u0027] or new_volume[\u0027id\u0027]"},{"line_number":412,"context_line":"        else:"},{"line_number":413,"context_line":"            # The back-end will not be renamed."},{"line_number":414,"context_line":"            name_id \u003d new_volume[\u0027_name_id\u0027] or new_volume[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a1a9d01_daa0f8a1","line":411,"in_reply_to":"9a1a9d01_a730d657","updated":"2015-09-28 13:16:38.000000000","message":"My comment is no longer valid.","commit_id":"4a0ebde26b9a1417ca065114895f47d5971c8216"}]}