)]}'
{"cinder/tests/unit/test_volume.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"7d4825a914a7212ee4700da132ac0c4860ca95a2","unresolved":false,"context_lines":[{"line_number":1127,"context_line":"        self.volume_api.delete(self.context, volume)"},{"line_number":1128,"context_line":""},{"line_number":1129,"context_line":"        volume \u003d db.volume_get(self.context, volume[\u0027id\u0027])"},{"line_number":1130,"context_line":"        while volume[\u0027status\u0027] \u003d\u003d \u0027available\u0027:"},{"line_number":1131,"context_line":"            # Must wait for volume_api delete request to process enough to"},{"line_number":1132,"context_line":"            # change the volume status."},{"line_number":1133,"context_line":"            time.sleep(0.5)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ac06dbe_769d7565","line":1130,"range":{"start_line":1130,"start_character":14,"end_line":1130,"end_character":30},"updated":"2016-08-23 10:42:00.000000000","message":"-1: This is an ORM instance, we should use attribute notation\n\n volume.status","commit_id":"0074f3a6dfc2f348efac9734e4d9ebb20554b85f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"7d4825a914a7212ee4700da132ac0c4860ca95a2","unresolved":false,"context_lines":[{"line_number":1131,"context_line":"            # Must wait for volume_api delete request to process enough to"},{"line_number":1132,"context_line":"            # change the volume status."},{"line_number":1133,"context_line":"            time.sleep(0.5)"},{"line_number":1134,"context_line":"            volume \u003d db.volume_get(self.context, volume[\u0027id\u0027])"},{"line_number":1135,"context_line":""},{"line_number":1136,"context_line":"        self.assertEqual(\u0027deleting\u0027, volume[\u0027status\u0027])"},{"line_number":1137,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1ac06dbe_76f0d591","line":1134,"range":{"start_line":1134,"start_character":21,"end_line":1134,"end_character":34},"updated":"2016-08-23 10:42:00.000000000","message":"-1: New code should use OVOs instead of accessing directly DB methods:\n\n volume \u003d objects.Volume.get_by_id(self.context, volume.id)\n\nAnd if L1129 is changed with:\n\n volume \u003d objects.Volume.get_by_id(self.context, volume.id)\n\nthen we can just do the loop with:\n\n volume.refresh()","commit_id":"0074f3a6dfc2f348efac9734e4d9ebb20554b85f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"7d4825a914a7212ee4700da132ac0c4860ca95a2","unresolved":false,"context_lines":[{"line_number":1131,"context_line":"            # Must wait for volume_api delete request to process enough to"},{"line_number":1132,"context_line":"            # change the volume status."},{"line_number":1133,"context_line":"            time.sleep(0.5)"},{"line_number":1134,"context_line":"            volume \u003d db.volume_get(self.context, volume[\u0027id\u0027])"},{"line_number":1135,"context_line":""},{"line_number":1136,"context_line":"        self.assertEqual(\u0027deleting\u0027, volume[\u0027status\u0027])"},{"line_number":1137,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1ac06dbe_b689ad28","line":1134,"range":{"start_line":1134,"start_character":49,"end_line":1134,"end_character":61},"updated":"2016-08-23 10:42:00.000000000","message":"ditto","commit_id":"0074f3a6dfc2f348efac9734e4d9ebb20554b85f"},{"author":{"_account_id":23083,"name":"TommyLike","email":"tommylikehu@gmail.com","username":"TommyLike"},"change_message_id":"86115213de54b5901f70f654fee27a4cda20ccb3","unresolved":false,"context_lines":[{"line_number":1127,"context_line":"        self.volume_api.delete(self.context, volume)"},{"line_number":1128,"context_line":""},{"line_number":1129,"context_line":"        volume \u003d objects.Volume.get_by_id(self.context, volume.id)"},{"line_number":1130,"context_line":"        while volume.status \u003d\u003d \u0027available\u0027:"},{"line_number":1131,"context_line":"            # Must wait for volume_api delete request to process enough to"},{"line_number":1132,"context_line":"            # change the volume status."},{"line_number":1133,"context_line":"            time.sleep(0.5)"},{"line_number":1134,"context_line":"            volume.refresh()"},{"line_number":1135,"context_line":""},{"line_number":1136,"context_line":"        self.assertEqual(\u0027deleting\u0027, volume[\u0027status\u0027])"},{"line_number":1137,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_9900e735","line":1134,"range":{"start_line":1130,"start_character":6,"end_line":1134,"end_character":28},"updated":"2016-08-24 02:31:23.000000000","message":"IMO you should add max_wait_time in case of infinite loop if bad thing happened,and also this solution may lead your testcase unstable in time consuming","commit_id":"aabca4d746368846b05f47590a99c1c03b4469a9"},{"author":{"_account_id":23083,"name":"TommyLike","email":"tommylikehu@gmail.com","username":"TommyLike"},"change_message_id":"9a7ba275a48c5202e191992bfe4298c06409b376","unresolved":false,"context_lines":[{"line_number":1127,"context_line":"        self.volume_api.delete(self.context, volume)"},{"line_number":1128,"context_line":""},{"line_number":1129,"context_line":"        volume \u003d objects.Volume.get_by_id(self.context, volume.id)"},{"line_number":1130,"context_line":"        while volume.status \u003d\u003d \u0027available\u0027:"},{"line_number":1131,"context_line":"            # Must wait for volume_api delete request to process enough to"},{"line_number":1132,"context_line":"            # change the volume status."},{"line_number":1133,"context_line":"            time.sleep(0.5)"},{"line_number":1134,"context_line":"            volume.refresh()"},{"line_number":1135,"context_line":""},{"line_number":1136,"context_line":"        self.assertEqual(\u0027deleting\u0027, volume[\u0027status\u0027])"},{"line_number":1137,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_7af6aa5d","line":1134,"range":{"start_line":1130,"start_character":6,"end_line":1134,"end_character":28},"in_reply_to":"1ac06dbe_1a6fbbfc","updated":"2016-08-24 09:49:08.000000000","message":"That\u0027s reasonable~","commit_id":"aabca4d746368846b05f47590a99c1c03b4469a9"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ce3e262291e463531b818a667a800bf1a2d4a536","unresolved":false,"context_lines":[{"line_number":1127,"context_line":"        self.volume_api.delete(self.context, volume)"},{"line_number":1128,"context_line":""},{"line_number":1129,"context_line":"        volume \u003d objects.Volume.get_by_id(self.context, volume.id)"},{"line_number":1130,"context_line":"        while volume.status \u003d\u003d \u0027available\u0027:"},{"line_number":1131,"context_line":"            # Must wait for volume_api delete request to process enough to"},{"line_number":1132,"context_line":"            # change the volume status."},{"line_number":1133,"context_line":"            time.sleep(0.5)"},{"line_number":1134,"context_line":"            volume.refresh()"},{"line_number":1135,"context_line":""},{"line_number":1136,"context_line":"        self.assertEqual(\u0027deleting\u0027, volume[\u0027status\u0027])"},{"line_number":1137,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_1d3de3bb","line":1134,"range":{"start_line":1130,"start_character":6,"end_line":1134,"end_character":28},"in_reply_to":"1ac06dbe_7af6aa5d","updated":"2016-08-24 13:11:59.000000000","message":"\u003e IMO you should add max_wait_time in case of infinite loop if bad thing happened\n\nThis is not necessary for unit tests -- the test rig already implements a fixed timeout after which it stops the unit test.  (60s, I believe.)","commit_id":"aabca4d746368846b05f47590a99c1c03b4469a9"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"27f41ca4693ca20388741b3502a8cbe6763d1aa1","unresolved":false,"context_lines":[{"line_number":1127,"context_line":"        self.volume_api.delete(self.context, volume)"},{"line_number":1128,"context_line":""},{"line_number":1129,"context_line":"        volume \u003d objects.Volume.get_by_id(self.context, volume.id)"},{"line_number":1130,"context_line":"        while volume.status \u003d\u003d \u0027available\u0027:"},{"line_number":1131,"context_line":"            # Must wait for volume_api delete request to process enough to"},{"line_number":1132,"context_line":"            # change the volume status."},{"line_number":1133,"context_line":"            time.sleep(0.5)"},{"line_number":1134,"context_line":"            volume.refresh()"},{"line_number":1135,"context_line":""},{"line_number":1136,"context_line":"        self.assertEqual(\u0027deleting\u0027, volume[\u0027status\u0027])"},{"line_number":1137,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_1a6fbbfc","line":1134,"range":{"start_line":1130,"start_character":6,"end_line":1134,"end_character":28},"in_reply_to":"1ac06dbe_9900e735","updated":"2016-08-24 08:29:18.000000000","message":"That\u0027s a very valid point, and if this was in the Cinder code I would agree with you 100%, but this is for the unit tests, and here sqlite is not going to disappear beneath us, or any other process is going to modify the same volume while we wait, or anything like that.\n\nIn theory he could have just given a 2 second delay and be done with it, but I think he just wanted to be safe for the rare occasions where this can happen.\n\nAnd if this has to do more than 1 sleep it means the machine that is running this test is under serious load, and a delay in the tests is expected and acceptable.\n\nI believe this is an acceptable solution for a test case.","commit_id":"aabca4d746368846b05f47590a99c1c03b4469a9"}]}