)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"e6286a8273f7c98c6703021496f6c765a29386b1","unresolved":false,"context_lines":[{"line_number":15,"context_line":"The test scenarios use objects, test_gpfs needs to use valid UUIDs"},{"line_number":16,"context_line":"instead of simple string such as \"cg-1234\" or \"123456\""},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Patrick mentioned issues during code review:"},{"line_number":19,"context_line":"I think these id\u0027s are going to end up needing to be UUID\u0027s too"},{"line_number":20,"context_line":"(if not now, in the future). Might be worthwhile to change them now."},{"line_number":21,"context_line":"those invalid UUIDs get fixed."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"dab17558_071e06ba","line":18,"updated":"2016-05-06 15:45:02.000000000","message":"This info doesn\u0027t need to be in the commit message, as it\u0027s not information directly about the change being merged that people will want to reference later.","commit_id":"1ba68b1512bec83503fc0ecd6f54ae25abad9016"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"e6286a8273f7c98c6703021496f6c765a29386b1","unresolved":false,"context_lines":[{"line_number":21,"context_line":"those invalid UUIDs get fixed."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Fixed the invalid UUIDs and wrong UUID names which xing-yang"},{"line_number":24,"context_line":"figured out during code review."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Co-Authored-By: Xinli Guan \u003cxinli@us.ibm.com\u003e"},{"line_number":27,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"dab17558_6779d2fc","line":24,"updated":"2016-05-06 15:45:02.000000000","message":"Same","commit_id":"1ba68b1512bec83503fc0ecd6f54ae25abad9016"}],"cinder/tests/unit/test_gpfs.py":[{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"d3864834c8837acd9fc84765551025cf4de5ca36","unresolved":false,"context_lines":[{"line_number":1813,"context_line":""},{"line_number":1814,"context_line":"    def _fake_volume_in_cg(self):"},{"line_number":1815,"context_line":"        volume \u003d self._fake_volume()"},{"line_number":1816,"context_line":"        volume.consistencygroup_id \u003d \u0027fakecg\u0027"},{"line_number":1817,"context_line":"        return volume"},{"line_number":1818,"context_line":""},{"line_number":1819,"context_line":"    def _fake_group(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_9774a4cf","line":1816,"updated":"2016-05-04 21:14:37.000000000","message":"I think these id\u0027s are going to end up needing to be UUID\u0027s too (if not now, in the future). Might be worthwhile to just change them now while your are fixing up the others.","commit_id":"45216ee2729bf88ed6660b9ee9f15d5d6e8bc113"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"d3864834c8837acd9fc84765551025cf4de5ca36","unresolved":false,"context_lines":[{"line_number":1824,"context_line":""},{"line_number":1825,"context_line":"    def _fake_cgsnapshot(self):"},{"line_number":1826,"context_line":"        snapshot \u003d self._fake_snapshot()"},{"line_number":1827,"context_line":"        snapshot.consistencygroup_id \u003d \u0027fakecg\u0027"},{"line_number":1828,"context_line":"        return snapshot"},{"line_number":1829,"context_line":""},{"line_number":1830,"context_line":"    def _fake_qemu_qcow2_image_info(self, path):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_374af083","line":1827,"updated":"2016-05-04 21:14:37.000000000","message":"same here","commit_id":"45216ee2729bf88ed6660b9ee9f15d5d6e8bc113"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"93aaad65f79a3acbd3b66b3f7e78d0ea0795ea6b","unresolved":false,"context_lines":[{"line_number":1819,"context_line":"    def _fake_group(self):"},{"line_number":1820,"context_line":"        group \u003d {}"},{"line_number":1821,"context_line":"        group[\u0027name\u0027] \u003d \u0027test_group\u0027"},{"line_number":1822,"context_line":"        group[\u0027id\u0027] \u003d \u0027123456\u0027"},{"line_number":1823,"context_line":"        return group"},{"line_number":1824,"context_line":""},{"line_number":1825,"context_line":"    def _fake_cgsnapshot(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_69cee084","line":1822,"updated":"2016-05-06 04:41:56.000000000","message":"Since you are changing everywhere else, this one should use UUID too.","commit_id":"2afa048a7ff1fb459e02fc4d2a390aca38dff12b"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"93aaad65f79a3acbd3b66b3f7e78d0ea0795ea6b","unresolved":false,"context_lines":[{"line_number":1824,"context_line":""},{"line_number":1825,"context_line":"    def _fake_cgsnapshot(self):"},{"line_number":1826,"context_line":"        snapshot \u003d self._fake_snapshot()"},{"line_number":1827,"context_line":"        snapshot.consistencygroup_id \u003d fake.CGSNAPSHOT_ID"},{"line_number":1828,"context_line":"        return snapshot"},{"line_number":1829,"context_line":""},{"line_number":1830,"context_line":"    def _fake_qemu_qcow2_image_info(self, path):"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_c930aca1","line":1827,"updated":"2016-05-06 04:41:56.000000000","message":"This should be fake.CONSISTENCY_GROUP_ID.","commit_id":"2afa048a7ff1fb459e02fc4d2a390aca38dff12b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4385660896927bcc56863d3f491e75c2dbaef3a3","unresolved":false,"context_lines":[{"line_number":17,"context_line":"import tempfile"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"import mock"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from oslo_concurrency import processutils"},{"line_number":22,"context_line":"from oslo_config import cfg"},{"line_number":23,"context_line":"from oslo_utils import units"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_17f15a6b","line":20,"updated":"2016-05-23 11:09:40.000000000","message":"-1: No blank line here","commit_id":"1548c0182a9dc5f3f8fe1c969f5e461ce1b52da5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4385660896927bcc56863d3f491e75c2dbaef3a3","unresolved":false,"context_lines":[{"line_number":1714,"context_line":"        model_update, snapshots \u003d self.driver.create_cgsnapshot(ctxt, cgsnap,"},{"line_number":1715,"context_line":"                                                                [snapshot1])"},{"line_number":1716,"context_line":"        self.driver.create_snapshot.assert_called_once_with(snapshot1)"},{"line_number":1717,"context_line":"        cgsnap[\u0027status\u0027] \u003d model_update[\u0027status\u0027]"},{"line_number":1718,"context_line":"        self.assertEqual({\u0027status\u0027: cgsnap.status}, model_update)"},{"line_number":1719,"context_line":"        self.assertEqual(\u0027available\u0027, snapshots[0][\u0027status\u0027])"},{"line_number":1720,"context_line":""},{"line_number":1721,"context_line":"    @mock.patch(\u0027cinder.volume.drivers.ibm.gpfs.GPFSDriver.create_snapshot\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_9a3d71c6","line":1718,"range":{"start_line":1717,"start_character":0,"end_line":1718,"end_character":65},"updated":"2016-05-23 11:09:40.000000000","message":"-1: This looks wrong, if you update your expected status with the test result status and then check that they are the same it doesn\u0027t actually test anything, besides the fact that model_update has the status key in the dictionary, right?","commit_id":"1548c0182a9dc5f3f8fe1c969f5e461ce1b52da5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4385660896927bcc56863d3f491e75c2dbaef3a3","unresolved":false,"context_lines":[{"line_number":1734,"context_line":"        snapshot1 \u003d self._fake_snapshot()"},{"line_number":1735,"context_line":"        model_update, snapshots \u003d self.driver.delete_cgsnapshot(ctxt, cgsnap,"},{"line_number":1736,"context_line":"                                                                [snapshot1])"},{"line_number":1737,"context_line":"        cgsnap[\u0027status\u0027] \u003d model_update[\u0027status\u0027]"},{"line_number":1738,"context_line":"        self.driver.delete_snapshot.assert_called_once_with(snapshot1)"},{"line_number":1739,"context_line":"        self.assertEqual({\u0027status\u0027: cgsnap.status}, model_update)"},{"line_number":1740,"context_line":"        self.assertEqual(\u0027deleted\u0027, snapshots[0][\u0027status\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_da3379f4","line":1737,"range":{"start_line":1737,"start_character":8,"end_line":1737,"end_character":49},"updated":"2016-05-23 11:09:40.000000000","message":"-1: Same here","commit_id":"1548c0182a9dc5f3f8fe1c969f5e461ce1b52da5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4385660896927bcc56863d3f491e75c2dbaef3a3","unresolved":false,"context_lines":[{"line_number":1746,"context_line":"        model_update, snapshots \u003d self.driver.delete_cgsnapshot(ctxt, cgsnap,"},{"line_number":1747,"context_line":"                                                                [])"},{"line_number":1748,"context_line":"        self.assertFalse(self.driver.delete_snapshot.called)"},{"line_number":1749,"context_line":"        cgsnap[\u0027status\u0027] \u003d model_update[\u0027status\u0027]"},{"line_number":1750,"context_line":"        self.assertEqual({\u0027status\u0027: cgsnap.status}, model_update)"},{"line_number":1751,"context_line":""},{"line_number":1752,"context_line":"    def test_local_path_volume_not_in_cg(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_fa52bd11","line":1749,"range":{"start_line":1749,"start_character":8,"end_line":1749,"end_character":49},"updated":"2016-05-23 11:09:40.000000000","message":"-1: And here","commit_id":"1548c0182a9dc5f3f8fe1c969f5e461ce1b52da5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"601c6367b455edaceb8a4993b38f80266b7a3760","unresolved":false,"context_lines":[{"line_number":1713,"context_line":"        model_update, snapshots \u003d self.driver.create_cgsnapshot(ctxt, cgsnap,"},{"line_number":1714,"context_line":"                                                                [snapshot1])"},{"line_number":1715,"context_line":"        self.driver.create_snapshot.assert_called_once_with(snapshot1)"},{"line_number":1716,"context_line":"        cgsnap[\u0027status\u0027] \u003d fields.ConsistencyGroupStatus.AVAILABLE"},{"line_number":1717,"context_line":"        self.assertEqual({\u0027status\u0027: cgsnap.status}, model_update)"},{"line_number":1718,"context_line":"        self.assertEqual(\u0027available\u0027, snapshots[0][\u0027status\u0027])"},{"line_number":1719,"context_line":""},{"line_number":1720,"context_line":"    @mock.patch(\u0027cinder.volume.drivers.ibm.gpfs.GPFSDriver.create_snapshot\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_1fba8123","line":1717,"range":{"start_line":1716,"start_character":8,"end_line":1717,"end_character":65},"updated":"2016-05-25 15:14:05.000000000","message":"nit: This looks really confusing to me, because you are using the cgsnap just to hold a variable, it would look better like this:\n\n self.assertEqual(\n     {\u0027status\u0027: fields.ConsistencyGroupStatus.AVAILABLE},\n     model_update)","commit_id":"6cb692928aec26ffd050e4e0b36094335915f4db"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"601c6367b455edaceb8a4993b38f80266b7a3760","unresolved":false,"context_lines":[{"line_number":1733,"context_line":"        snapshot1 \u003d self._fake_snapshot()"},{"line_number":1734,"context_line":"        model_update, snapshots \u003d self.driver.delete_cgsnapshot(ctxt, cgsnap,"},{"line_number":1735,"context_line":"                                                                [snapshot1])"},{"line_number":1736,"context_line":"        cgsnap[\u0027status\u0027] \u003d fields.ConsistencyGroupStatus.DELETED"},{"line_number":1737,"context_line":"        self.driver.delete_snapshot.assert_called_once_with(snapshot1)"},{"line_number":1738,"context_line":"        self.assertEqual({\u0027status\u0027: cgsnap.status}, model_update)"},{"line_number":1739,"context_line":"        self.assertEqual(\u0027deleted\u0027, snapshots[0][\u0027status\u0027])"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_5fd08953","line":1736,"range":{"start_line":1736,"start_character":8,"end_line":1736,"end_character":64},"updated":"2016-05-25 15:14:05.000000000","message":"ditto","commit_id":"6cb692928aec26ffd050e4e0b36094335915f4db"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"601c6367b455edaceb8a4993b38f80266b7a3760","unresolved":false,"context_lines":[{"line_number":1736,"context_line":"        cgsnap[\u0027status\u0027] \u003d fields.ConsistencyGroupStatus.DELETED"},{"line_number":1737,"context_line":"        self.driver.delete_snapshot.assert_called_once_with(snapshot1)"},{"line_number":1738,"context_line":"        self.assertEqual({\u0027status\u0027: cgsnap.status}, model_update)"},{"line_number":1739,"context_line":"        self.assertEqual(\u0027deleted\u0027, snapshots[0][\u0027status\u0027])"},{"line_number":1740,"context_line":""},{"line_number":1741,"context_line":"    @mock.patch(\u0027cinder.volume.drivers.ibm.gpfs.GPFSDriver.delete_snapshot\u0027)"},{"line_number":1742,"context_line":"    def test_delete_cgsnapshot_empty(self, mock_delete_snap):"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_bfd89580","line":1739,"range":{"start_line":1739,"start_character":7,"end_line":1739,"end_character":59},"updated":"2016-05-25 15:14:05.000000000","message":"nit: Better test the whole dictionary as you are doing with the model_update","commit_id":"6cb692928aec26ffd050e4e0b36094335915f4db"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"601c6367b455edaceb8a4993b38f80266b7a3760","unresolved":false,"context_lines":[{"line_number":1745,"context_line":"        model_update, snapshots \u003d self.driver.delete_cgsnapshot(ctxt, cgsnap,"},{"line_number":1746,"context_line":"                                                                [])"},{"line_number":1747,"context_line":"        self.assertFalse(self.driver.delete_snapshot.called)"},{"line_number":1748,"context_line":"        cgsnap[\u0027status\u0027] \u003d fields.ConsistencyGroupStatus.DELETED"},{"line_number":1749,"context_line":"        self.assertEqual({\u0027status\u0027: cgsnap.status}, model_update)"},{"line_number":1750,"context_line":""},{"line_number":1751,"context_line":"    def test_local_path_volume_not_in_cg(self):"},{"line_number":1752,"context_line":"        volume \u003d self._fake_volume()"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_9f7cb173","line":1749,"range":{"start_line":1748,"start_character":8,"end_line":1749,"end_character":65},"updated":"2016-05-25 15:14:05.000000000","message":"ditto","commit_id":"6cb692928aec26ffd050e4e0b36094335915f4db"}],"cinder/volume/drivers/ibm/gpfs.py":[{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"9e81ca61c6c2501d69fde9d675ee1ab3a668abad","unresolved":false,"context_lines":[{"line_number":1160,"context_line":""},{"line_number":1161,"context_line":"        model_update \u003d {}"},{"line_number":1162,"context_line":"        model_update[\u0027status\u0027] \u003d group[\u0027status\u0027]"},{"line_number":1163,"context_line":"        volumes \u003d self.db.volume_get_all_by_group(context, group[\u0027id\u0027])"},{"line_number":1164,"context_line":""},{"line_number":1165,"context_line":"        # Unlink and delete the fileset associated with the consistency group."},{"line_number":1166,"context_line":"        # All of the volumes and volume snapshot data will also be deleted."}],"source_content_type":"text/x-python","patch_set":6,"id":"dab17558_9f3cacc0","line":1163,"updated":"2016-05-15 03:52:38.000000000","message":"You should change this one too.  volumes is already passed in.  There is no need to  get them from db again.","commit_id":"33de8192c0cfe4453d1c0e90b188a53b946ff839"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"9e81ca61c6c2501d69fde9d675ee1ab3a668abad","unresolved":false,"context_lines":[{"line_number":1193,"context_line":"        \"\"\"Create snapshot of a consistency group of GPFS volumes.\"\"\""},{"line_number":1194,"context_line":"        for snapshot in snapshots:"},{"line_number":1195,"context_line":"            self.create_snapshot(snapshot)"},{"line_number":1196,"context_line":"            snapshot.status \u003d \u0027available\u0027"},{"line_number":1197,"context_line":""},{"line_number":1198,"context_line":"        model_update \u003d {\u0027status\u0027: \u0027available\u0027}"},{"line_number":1199,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"dab17558_7f574802","line":1196,"updated":"2016-05-15 03:52:38.000000000","message":"Same here as in line 1205. You should create an array of dictionaries and set \u0027id\u0027 and \u0027status\u0027 for each snapshot item in the dict.","commit_id":"33de8192c0cfe4453d1c0e90b188a53b946ff839"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"743a1404448dfe07bdf1e19074342b6770b4c870","unresolved":false,"context_lines":[{"line_number":1202,"context_line":"    def delete_cgsnapshot(self, context, cgsnapshot, snapshots):"},{"line_number":1203,"context_line":"        \"\"\"Delete snapshot of a consistency group of GPFS volumes.\"\"\""},{"line_number":1204,"context_line":"        for snapshot in snapshots:"},{"line_number":1205,"context_line":"            self.delete_snapshot(snapshot)"},{"line_number":1206,"context_line":"            snapshot.status \u003d \u0027deleted\u0027"},{"line_number":1207,"context_line":""},{"line_number":1208,"context_line":"        model_update \u003d {\u0027status\u0027: cgsnapshot[\u0027status\u0027]}"}],"source_content_type":"text/x-python","patch_set":6,"id":"dab17558_120585c1","line":1205,"updated":"2016-05-13 10:33:29.000000000","message":"?: No error handling in here?","commit_id":"33de8192c0cfe4453d1c0e90b188a53b946ff839"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"743a1404448dfe07bdf1e19074342b6770b4c870","unresolved":false,"context_lines":[{"line_number":1203,"context_line":"        \"\"\"Delete snapshot of a consistency group of GPFS volumes.\"\"\""},{"line_number":1204,"context_line":"        for snapshot in snapshots:"},{"line_number":1205,"context_line":"            self.delete_snapshot(snapshot)"},{"line_number":1206,"context_line":"            snapshot.status \u003d \u0027deleted\u0027"},{"line_number":1207,"context_line":""},{"line_number":1208,"context_line":"        model_update \u003d {\u0027status\u0027: cgsnapshot[\u0027status\u0027]}"},{"line_number":1209,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"dab17558_72bfb1fc","line":1206,"updated":"2016-05-13 10:33:29.000000000","message":"-1: This is wrong, you are changing the OVO directly here, which you shouldn\u0027t do.  You should be generating an iterable with dictionaries that have \u0027id\u0027 and \u0027status\u0027 keys.","commit_id":"33de8192c0cfe4453d1c0e90b188a53b946ff839"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"9e81ca61c6c2501d69fde9d675ee1ab3a668abad","unresolved":false,"context_lines":[{"line_number":1203,"context_line":"        \"\"\"Delete snapshot of a consistency group of GPFS volumes.\"\"\""},{"line_number":1204,"context_line":"        for snapshot in snapshots:"},{"line_number":1205,"context_line":"            self.delete_snapshot(snapshot)"},{"line_number":1206,"context_line":"            snapshot.status \u003d \u0027deleted\u0027"},{"line_number":1207,"context_line":""},{"line_number":1208,"context_line":"        model_update \u003d {\u0027status\u0027: cgsnapshot[\u0027status\u0027]}"},{"line_number":1209,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"dab17558_3f4dc072","line":1206,"in_reply_to":"dab17558_72bfb1fc","updated":"2016-05-15 03:52:38.000000000","message":"Good catch. Gorka is right.","commit_id":"33de8192c0cfe4453d1c0e90b188a53b946ff839"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"743a1404448dfe07bdf1e19074342b6770b4c870","unresolved":false,"context_lines":[{"line_number":1205,"context_line":"            self.delete_snapshot(snapshot)"},{"line_number":1206,"context_line":"            snapshot.status \u003d \u0027deleted\u0027"},{"line_number":1207,"context_line":""},{"line_number":1208,"context_line":"        model_update \u003d {\u0027status\u0027: cgsnapshot[\u0027status\u0027]}"},{"line_number":1209,"context_line":""},{"line_number":1210,"context_line":"        return model_update, snapshots"},{"line_number":1211,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"dab17558_726331de","line":1208,"updated":"2016-05-13 10:33:29.000000000","message":"?: Isn\u0027t this kind of pointless?  We are not changing the status here.","commit_id":"33de8192c0cfe4453d1c0e90b188a53b946ff839"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"743a1404448dfe07bdf1e19074342b6770b4c870","unresolved":false,"context_lines":[{"line_number":1207,"context_line":""},{"line_number":1208,"context_line":"        model_update \u003d {\u0027status\u0027: cgsnapshot[\u0027status\u0027]}"},{"line_number":1209,"context_line":""},{"line_number":1210,"context_line":"        return model_update, snapshots"},{"line_number":1211,"context_line":""},{"line_number":1212,"context_line":""},{"line_number":1213,"context_line":"class GPFSNFSDriver(GPFSDriver, nfs.NfsDriver, san.SanDriver):"}],"source_content_type":"text/x-python","patch_set":6,"id":"dab17558_123745b1","line":1210,"updated":"2016-05-13 10:33:29.000000000","message":"-1: You should return a new iterable not modified OVOs","commit_id":"33de8192c0cfe4453d1c0e90b188a53b946ff839"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4385660896927bcc56863d3f491e75c2dbaef3a3","unresolved":false,"context_lines":[{"line_number":1160,"context_line":""},{"line_number":1161,"context_line":"        model_update \u003d {}"},{"line_number":1162,"context_line":"        model_update[\u0027status\u0027] \u003d group[\u0027status\u0027]"},{"line_number":1163,"context_line":"        # volumes \u003d self.db.volume_get_all_by_group(context, group[\u0027id\u0027])"},{"line_number":1164,"context_line":""},{"line_number":1165,"context_line":"        # Unlink and delete the fileset associated with the consistency group."},{"line_number":1166,"context_line":"        # All of the volumes and volume snapshot data will also be deleted."}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_ba45d543","line":1163,"range":{"start_line":1163,"start_character":8,"end_line":1163,"end_character":73},"updated":"2016-05-23 11:09:40.000000000","message":"-1: If not needed it should be removed, not commented out","commit_id":"1548c0182a9dc5f3f8fe1c969f5e461ce1b52da5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4385660896927bcc56863d3f491e75c2dbaef3a3","unresolved":false,"context_lines":[{"line_number":1197,"context_line":""},{"line_number":1198,"context_line":"        return model_update, snapshots_model"},{"line_number":1199,"context_line":""},{"line_number":1200,"context_line":"    def create_consistgrp_snapshots(self, snapshots):"},{"line_number":1201,"context_line":"        \"\"\"Create maps and consistent group.\"\"\""},{"line_number":1202,"context_line":"        model_update \u003d {\u0027status\u0027: fields.ConsistencyGroupStatus.AVAILABLE}"},{"line_number":1203,"context_line":"        snapshots_model_update \u003d []"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_5a5789c9","line":1200,"range":{"start_line":1200,"start_character":7,"end_line":1200,"end_character":53},"updated":"2016-05-23 11:09:40.000000000","message":"-1: What\u0027s the point of creating another method that does everything if it\u0027s not called from anywhere else?  Should this just go in the create_cgsnapshot as before?","commit_id":"1548c0182a9dc5f3f8fe1c969f5e461ce1b52da5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4385660896927bcc56863d3f491e75c2dbaef3a3","unresolved":false,"context_lines":[{"line_number":1210,"context_line":"                fields.ConsistencyGroupStatus.ERROR_CREATE)"},{"line_number":1211,"context_line":"            LOG.error(_LE(\"Failed to create the snapshot %(snap)s of \""},{"line_number":1212,"context_line":"                          \"CGSnapshot. Exception: %(exception)s.\"),"},{"line_number":1213,"context_line":"                      {\u0027snap\u0027: snapshot[\u0027name\u0027], \u0027exception\u0027: err})"},{"line_number":1214,"context_line":""},{"line_number":1215,"context_line":"        for snapshot in snapshots:"},{"line_number":1216,"context_line":"            snapshots_model_update.append("}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_5a98296a","line":1213,"updated":"2016-05-23 11:09:40.000000000","message":"-1: Use attribute notation: snapshot.name","commit_id":"1548c0182a9dc5f3f8fe1c969f5e461ce1b52da5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4385660896927bcc56863d3f491e75c2dbaef3a3","unresolved":false,"context_lines":[{"line_number":1213,"context_line":"                      {\u0027snap\u0027: snapshot[\u0027name\u0027], \u0027exception\u0027: err})"},{"line_number":1214,"context_line":""},{"line_number":1215,"context_line":"        for snapshot in snapshots:"},{"line_number":1216,"context_line":"            snapshots_model_update.append("},{"line_number":1217,"context_line":"                {\u0027id\u0027: snapshot[\u0027id\u0027],"},{"line_number":1218,"context_line":"                 \u0027status\u0027: model_update[\u0027status\u0027]})"},{"line_number":1219,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_1a2ea1fa","line":1216,"range":{"start_line":1216,"start_character":12,"end_line":1216,"end_character":42},"updated":"2016-05-23 11:09:40.000000000","message":"nit: It would be better to use list comprehension instead:\n\n snapshots_model_update \u003d [{\u0027id\u0027: snapshot.id, \u0027status\u0027: model_update[\u0027status\u0027]} for snapshot in snapshots]","commit_id":"1548c0182a9dc5f3f8fe1c969f5e461ce1b52da5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4385660896927bcc56863d3f491e75c2dbaef3a3","unresolved":false,"context_lines":[{"line_number":1214,"context_line":""},{"line_number":1215,"context_line":"        for snapshot in snapshots:"},{"line_number":1216,"context_line":"            snapshots_model_update.append("},{"line_number":1217,"context_line":"                {\u0027id\u0027: snapshot[\u0027id\u0027],"},{"line_number":1218,"context_line":"                 \u0027status\u0027: model_update[\u0027status\u0027]})"},{"line_number":1219,"context_line":""},{"line_number":1220,"context_line":"        return model_update, snapshots_model_update"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_da9ed961","line":1217,"range":{"start_line":1217,"start_character":31,"end_line":1217,"end_character":38},"updated":"2016-05-23 11:09:40.000000000","message":"-1: Use attribute notation:  `snapshot.id`","commit_id":"1548c0182a9dc5f3f8fe1c969f5e461ce1b52da5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4385660896927bcc56863d3f491e75c2dbaef3a3","unresolved":false,"context_lines":[{"line_number":1226,"context_line":""},{"line_number":1227,"context_line":"        return model_update, snapshot_models"},{"line_number":1228,"context_line":""},{"line_number":1229,"context_line":"    def delete_consistgrp_snapshots(self, snapshots):"},{"line_number":1230,"context_line":"        \"\"\"Delete flashcopy maps and consistent group.\"\"\""},{"line_number":1231,"context_line":"        model_update \u003d {\u0027status\u0027: fields.ConsistencyGroupStatus.DELETED}"},{"line_number":1232,"context_line":"        snapshots_model_update \u003d []"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_babe75ab","line":1229,"range":{"start_line":1229,"start_character":7,"end_line":1229,"end_character":35},"updated":"2016-05-23 11:09:40.000000000","message":"-1: Same as with the other method","commit_id":"1548c0182a9dc5f3f8fe1c969f5e461ce1b52da5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4385660896927bcc56863d3f491e75c2dbaef3a3","unresolved":false,"context_lines":[{"line_number":1239,"context_line":"                fields.ConsistencyGroupStatus.ERROR_DELETING)"},{"line_number":1240,"context_line":"            LOG.error(_LE(\"Failed to delete the snapshot %(snap)s of \""},{"line_number":1241,"context_line":"                          \"CGSnapshot. Exception: %(exception)s.\"),"},{"line_number":1242,"context_line":"                      {\u0027snap\u0027: snapshot[\u0027name\u0027], \u0027exception\u0027: err})"},{"line_number":1243,"context_line":""},{"line_number":1244,"context_line":"        for snapshot in snapshots:"},{"line_number":1245,"context_line":"            snapshots_model_update.append("}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_fa30fd57","line":1242,"range":{"start_line":1242,"start_character":31,"end_line":1242,"end_character":47},"updated":"2016-05-23 11:09:40.000000000","message":"-1: snapshot.name","commit_id":"1548c0182a9dc5f3f8fe1c969f5e461ce1b52da5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4385660896927bcc56863d3f491e75c2dbaef3a3","unresolved":false,"context_lines":[{"line_number":1242,"context_line":"                      {\u0027snap\u0027: snapshot[\u0027name\u0027], \u0027exception\u0027: err})"},{"line_number":1243,"context_line":""},{"line_number":1244,"context_line":"        for snapshot in snapshots:"},{"line_number":1245,"context_line":"            snapshots_model_update.append("},{"line_number":1246,"context_line":"                {\u0027id\u0027: snapshot[\u0027id\u0027],"},{"line_number":1247,"context_line":"                 \u0027status\u0027: model_update[\u0027status\u0027]})"},{"line_number":1248,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_ba4315ae","line":1245,"range":{"start_line":1245,"start_character":11,"end_line":1245,"end_character":42},"updated":"2016-05-23 11:09:40.000000000","message":"nit: Better use list comprehension","commit_id":"1548c0182a9dc5f3f8fe1c969f5e461ce1b52da5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4385660896927bcc56863d3f491e75c2dbaef3a3","unresolved":false,"context_lines":[{"line_number":1243,"context_line":""},{"line_number":1244,"context_line":"        for snapshot in snapshots:"},{"line_number":1245,"context_line":"            snapshots_model_update.append("},{"line_number":1246,"context_line":"                {\u0027id\u0027: snapshot[\u0027id\u0027],"},{"line_number":1247,"context_line":"                 \u0027status\u0027: model_update[\u0027status\u0027]})"},{"line_number":1248,"context_line":""},{"line_number":1249,"context_line":"        return model_update, snapshots_model_update"}],"source_content_type":"text/x-python","patch_set":7,"id":"bab6814e_3a37254e","line":1246,"range":{"start_line":1246,"start_character":23,"end_line":1246,"end_character":37},"updated":"2016-05-23 11:09:40.000000000","message":"-1: snapshot.id","commit_id":"1548c0182a9dc5f3f8fe1c969f5e461ce1b52da5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"601c6367b455edaceb8a4993b38f80266b7a3760","unresolved":false,"context_lines":[{"line_number":1204,"context_line":"                      {\u0027snap\u0027: snapshot.name, \u0027exception\u0027: err})"},{"line_number":1205,"context_line":""},{"line_number":1206,"context_line":"        for snapshot in snapshots:"},{"line_number":1207,"context_line":"            snapshots_model_update.append("},{"line_number":1208,"context_line":"                {\u0027id\u0027: snapshot.id,"},{"line_number":1209,"context_line":"                 \u0027status\u0027: model_update[\u0027status\u0027]})"},{"line_number":1210,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_3f6145a9","line":1207,"range":{"start_line":1207,"start_character":12,"end_line":1207,"end_character":42},"updated":"2016-05-25 15:14:05.000000000","message":"nit: List comprehension would look better as mentioned in previous patch.","commit_id":"6cb692928aec26ffd050e4e0b36094335915f4db"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"601c6367b455edaceb8a4993b38f80266b7a3760","unresolved":false,"context_lines":[{"line_number":1226,"context_line":"                      {\u0027snap\u0027: snapshot.name, \u0027exception\u0027: err})"},{"line_number":1227,"context_line":""},{"line_number":1228,"context_line":"        for snapshot in snapshots:"},{"line_number":1229,"context_line":"            snapshots_model_update.append("},{"line_number":1230,"context_line":"                {\u0027id\u0027: snapshot.id,"},{"line_number":1231,"context_line":"                 \u0027status\u0027: model_update[\u0027status\u0027]})"},{"line_number":1232,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_3f0825d9","line":1229,"range":{"start_line":1229,"start_character":12,"end_line":1229,"end_character":42},"updated":"2016-05-25 15:14:05.000000000","message":"ditto","commit_id":"6cb692928aec26ffd050e4e0b36094335915f4db"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"d88aa325f58786b649dafe0ffa6a876511906d3a","unresolved":false,"context_lines":[{"line_number":1205,"context_line":"                self.create_snapshot(snapshot)"},{"line_number":1206,"context_line":"        except exception.VolumeBackendAPIException as err:"},{"line_number":1207,"context_line":"            model_update[\u0027status\u0027] \u003d ("},{"line_number":1208,"context_line":"                fields.ConsistencyGroupStatus.ERROR_CREATE)"},{"line_number":1209,"context_line":"            LOG.error(_LE(\"Failed to create the snapshot %(snap)s of \""},{"line_number":1210,"context_line":"                          \"CGSnapshot. Exception: %(exception)s.\"),"},{"line_number":1211,"context_line":"                      {\u0027snap\u0027: snapshot.name, \u0027exception\u0027: err})"}],"source_content_type":"text/x-python","patch_set":9,"id":"bab6814e_8768df22","line":1208,"updated":"2016-05-25 18:36:22.000000000","message":"It looks like there is no unit test for this case.","commit_id":"bf2be3ddb6f9c716288c70bc7fb37c229e911788"},{"author":{"_account_id":21144,"name":"Xinli Guan","email":"xinli@us.ibm.com","username":"xinli"},"change_message_id":"a04d875aaa2f4cefee32f509874e8a0fc59b3496","unresolved":false,"context_lines":[{"line_number":1205,"context_line":"                self.create_snapshot(snapshot)"},{"line_number":1206,"context_line":"        except exception.VolumeBackendAPIException as err:"},{"line_number":1207,"context_line":"            model_update[\u0027status\u0027] \u003d ("},{"line_number":1208,"context_line":"                fields.ConsistencyGroupStatus.ERROR_CREATE)"},{"line_number":1209,"context_line":"            LOG.error(_LE(\"Failed to create the snapshot %(snap)s of \""},{"line_number":1210,"context_line":"                          \"CGSnapshot. Exception: %(exception)s.\"),"},{"line_number":1211,"context_line":"                      {\u0027snap\u0027: snapshot.name, \u0027exception\u0027: err})"}],"source_content_type":"text/x-python","patch_set":9,"id":"bab6814e_088050be","line":1208,"in_reply_to":"bab6814e_819c36c1","updated":"2016-05-26 15:17:25.000000000","message":"Vipin; thank you very much for your feedback, I understand your concern, if encountering create / delete snapshot issue, the we will use log file (in GPFS driver) to indicating the issue which is consistent with other test scenario. I will consider to add more unit test scenarios in create / delete snapshort, consistent group, etc with the fields.xxx.","commit_id":"bf2be3ddb6f9c716288c70bc7fb37c229e911788"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"03e1f781e96e3c17088cd686acd93c21f3c68b20","unresolved":false,"context_lines":[{"line_number":1205,"context_line":"                self.create_snapshot(snapshot)"},{"line_number":1206,"context_line":"        except exception.VolumeBackendAPIException as err:"},{"line_number":1207,"context_line":"            model_update[\u0027status\u0027] \u003d ("},{"line_number":1208,"context_line":"                fields.ConsistencyGroupStatus.ERROR_CREATE)"},{"line_number":1209,"context_line":"            LOG.error(_LE(\"Failed to create the snapshot %(snap)s of \""},{"line_number":1210,"context_line":"                          \"CGSnapshot. Exception: %(exception)s.\"),"},{"line_number":1211,"context_line":"                      {\u0027snap\u0027: snapshot.name, \u0027exception\u0027: err})"}],"source_content_type":"text/x-python","patch_set":9,"id":"bab6814e_819c36c1","line":1208,"in_reply_to":"bab6814e_8306101a","updated":"2016-05-26 07:44:15.000000000","message":"I\u0027m not sure whether I understand you correctly. I was telling that it is good to have a negative test case test_create_cgsnapshot_error which asserts:\n\nself.assertEqual({\u0027status\u0027:fields.ConsistencyGroupStatus.ERROR_CREATE}, model_update)","commit_id":"bf2be3ddb6f9c716288c70bc7fb37c229e911788"},{"author":{"_account_id":21144,"name":"Xinli Guan","email":"xinli@us.ibm.com","username":"xinli"},"change_message_id":"78204e79cee189be90fd5abd88e71e290947f854","unresolved":false,"context_lines":[{"line_number":1205,"context_line":"                self.create_snapshot(snapshot)"},{"line_number":1206,"context_line":"        except exception.VolumeBackendAPIException as err:"},{"line_number":1207,"context_line":"            model_update[\u0027status\u0027] \u003d ("},{"line_number":1208,"context_line":"                fields.ConsistencyGroupStatus.ERROR_CREATE)"},{"line_number":1209,"context_line":"            LOG.error(_LE(\"Failed to create the snapshot %(snap)s of \""},{"line_number":1210,"context_line":"                          \"CGSnapshot. Exception: %(exception)s.\"),"},{"line_number":1211,"context_line":"                      {\u0027snap\u0027: snapshot.name, \u0027exception\u0027: err})"}],"source_content_type":"text/x-python","patch_set":9,"id":"bab6814e_8306101a","line":1208,"in_reply_to":"bab6814e_8768df22","updated":"2016-05-25 20:18:11.000000000","message":"Vipin; thank you very much for your code review, for this case,ERROR_DELETING, the error message will be posted in the log file.","commit_id":"bf2be3ddb6f9c716288c70bc7fb37c229e911788"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"d88aa325f58786b649dafe0ffa6a876511906d3a","unresolved":false,"context_lines":[{"line_number":1227,"context_line":"                self.delete_snapshot(snapshot)"},{"line_number":1228,"context_line":"        except exception.VolumeBackendAPIException as err:"},{"line_number":1229,"context_line":"            model_update[\u0027status\u0027] \u003d ("},{"line_number":1230,"context_line":"                fields.ConsistencyGroupStatus.ERROR_DELETING)"},{"line_number":1231,"context_line":"            LOG.error(_LE(\"Failed to delete the snapshot %(snap)s of \""},{"line_number":1232,"context_line":"                          \"CGSnapshot. Exception: %(exception)s.\"),"},{"line_number":1233,"context_line":"                      {\u0027snap\u0027: snapshot.name, \u0027exception\u0027: err})"}],"source_content_type":"text/x-python","patch_set":9,"id":"bab6814e_c772e710","line":1230,"updated":"2016-05-25 18:36:22.000000000","message":"Same comment.","commit_id":"bf2be3ddb6f9c716288c70bc7fb37c229e911788"},{"author":{"_account_id":21144,"name":"Xinli Guan","email":"xinli@us.ibm.com","username":"xinli"},"change_message_id":"78204e79cee189be90fd5abd88e71e290947f854","unresolved":false,"context_lines":[{"line_number":1227,"context_line":"                self.delete_snapshot(snapshot)"},{"line_number":1228,"context_line":"        except exception.VolumeBackendAPIException as err:"},{"line_number":1229,"context_line":"            model_update[\u0027status\u0027] \u003d ("},{"line_number":1230,"context_line":"                fields.ConsistencyGroupStatus.ERROR_DELETING)"},{"line_number":1231,"context_line":"            LOG.error(_LE(\"Failed to delete the snapshot %(snap)s of \""},{"line_number":1232,"context_line":"                          \"CGSnapshot. Exception: %(exception)s.\"),"},{"line_number":1233,"context_line":"                      {\u0027snap\u0027: snapshot.name, \u0027exception\u0027: err})"}],"source_content_type":"text/x-python","patch_set":9,"id":"bab6814e_63220ca5","line":1230,"in_reply_to":"bab6814e_c772e710","updated":"2016-05-25 20:18:11.000000000","message":"The case is the same as in create_cgsnapshot.","commit_id":"bf2be3ddb6f9c716288c70bc7fb37c229e911788"}]}