)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":32036,"name":"katari manoj kumar","email":"katkumar@in.ibm.com","username":"katarimanojkumar"},"change_message_id":"62c98a260135110ba27d4ebe6b379764994195dc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"d4ebe399_3abf4897","updated":"2021-12-21 13:52:51.000000000","message":"CI passed. LGTM","commit_id":"e9f7006c9189f4bfe667c58291319d08dd59cb6f"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"a552cca348480dcfa420663a406af9e29f6a0c9d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"377ee04a_8d6d5ad6","updated":"2022-01-06 16:49:10.000000000","message":"CI passed.  I think this looks ok.  ","commit_id":"bb9ba48bb0e121cc6a14a10510bc6f45c48e3413"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"05a68ea03c5585ec6a3ee44ed62dc4bf954b7680","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"52441a2d_14d2ca2b","updated":"2022-01-21 15:08:59.000000000","message":"Comments inline.  I don\u0027t have an opinion at the moment, so not voting.","commit_id":"bb9ba48bb0e121cc6a14a10510bc6f45c48e3413"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"7c067c3291121fcc3f0e6aaec499a603a8b6a209","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a18c7768_f5a2c25d","updated":"2022-01-28 13:39:06.000000000","message":"Just want to respond to some questions on IRC: I\u0027m not voting on this patch because I haven\u0027t had time to work through the logic here.  The reason I feel like i need to work through it is that I\u0027m suspicious of a strategy that does some stuff, does some more stuff, and then goes back and fixes some properties that were not set properly in phase 2.  But that\u0027s just a suspicion, everything may be fine if I understood more about the backend and the circumstances of the bug and understood exactly what\u0027s going on here.  And this isn\u0027t to say that I didn\u0027t spend time looking at the patch--that\u0027s why I left some comments in the first place.  But I must respect the project\u0027s review priorities, and there are other higher-priority patches to review, I do not feel comfortable voting on this patch until I understand what\u0027s going on.\n\nAnd it\u0027s possible I\u0027m just being a dope.  That\u0027s why we have multiple core reviewers.","commit_id":"bb9ba48bb0e121cc6a14a10510bc6f45c48e3413"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"8bf86b7532dd9e9947a9b9647bcadf65a2885ca7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f020e562_48ab021e","updated":"2022-01-10 08:15:25.000000000","message":"LGTM","commit_id":"bb9ba48bb0e121cc6a14a10510bc6f45c48e3413"},{"author":{"_account_id":33431,"name":"Fábio Oliveira","email":"fabioaurelio1269@gmail.com","username":"fabiooliveira1"},"change_message_id":"21036c199d349e0e282248fbf8c09db8d20fe7dd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"21465de8_679cd77d","updated":"2022-01-20 20:02:37.000000000","message":"LGTM\nThank you, Venkata","commit_id":"bb9ba48bb0e121cc6a14a10510bc6f45c48e3413"},{"author":{"_account_id":32266,"name":"Venkata krishna Thumu","display_name":"VenkataKrishna","email":"venkata.krishna.reddy@ibm.com","username":"venkatakrishnathumu","status":"Active"},"change_message_id":"278674523e400e8c2eed34c7b13fa79ed5faa6df","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7528215c_76aff864","updated":"2021-12-28 10:42:26.000000000","message":"run-IBM Storage CI","commit_id":"bb9ba48bb0e121cc6a14a10510bc6f45c48e3413"},{"author":{"_account_id":32266,"name":"Venkata krishna Thumu","display_name":"VenkataKrishna","email":"venkata.krishna.reddy@ibm.com","username":"venkatakrishnathumu","status":"Active"},"change_message_id":"b51b4f84cbdd2305dce6b81dfd7f58634f8154d2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7c4be675_f8b2ace8","updated":"2021-12-29 15:04:10.000000000","message":"run-IBM Storage CI","commit_id":"bb9ba48bb0e121cc6a14a10510bc6f45c48e3413"},{"author":{"_account_id":32266,"name":"Venkata krishna Thumu","display_name":"VenkataKrishna","email":"venkata.krishna.reddy@ibm.com","username":"venkatakrishnathumu","status":"Active"},"change_message_id":"5737dbafe3c2c9035e67012c512098f233f7f785","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b3d4a276_5967395c","updated":"2021-12-31 09:53:45.000000000","message":"run-IBM Storage CI","commit_id":"bb9ba48bb0e121cc6a14a10510bc6f45c48e3413"},{"author":{"_account_id":32266,"name":"Venkata krishna Thumu","display_name":"VenkataKrishna","email":"venkata.krishna.reddy@ibm.com","username":"venkatakrishnathumu","status":"Active"},"change_message_id":"03fec6d1db6c1d137322e8cb6f9d615a9a3f1aba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f54ea317_f237fc48","updated":"2021-12-28 05:01:28.000000000","message":"run-IBM Storage CI","commit_id":"bb9ba48bb0e121cc6a14a10510bc6f45c48e3413"},{"author":{"_account_id":34201,"name":"Mounika Sreeram","email":"sreeram.mounika@ibm.com","username":"sreerammounika"},"change_message_id":"9a317d77fe287b0807e50ba6461f8fe464a07c86","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"12dbf8a7_0509143f","in_reply_to":"a18c7768_f5a2c25d","updated":"2022-02-07 17:55:39.000000000","message":"Thanks for the review. I have addressed the previous comment.","commit_id":"bb9ba48bb0e121cc6a14a10510bc6f45c48e3413"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"843bcf5ed73fd40fd099b116a8bdbcab33f76df6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8b90ca0b_d0e722ed","updated":"2022-02-08 15:46:04.000000000","message":"Sorry, unless I\u0027m understanding something wrong there are 2 assignments to the `volume_model` variable that shouldn\u0027t affect the result of running the code, so by having them they are kind of misleading.","commit_id":"fd610c7aa9214388a1c46b3fe8c9e1c710fcfda8"},{"author":{"_account_id":34201,"name":"Mounika Sreeram","email":"sreeram.mounika@ibm.com","username":"sreerammounika"},"change_message_id":"17e70597cbf36caf2c42ae569e85cd089f1fd726","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"fa03e949_80a6048c","updated":"2022-02-07 16:05:04.000000000","message":"run-IBM Storage CI","commit_id":"fd610c7aa9214388a1c46b3fe8c9e1c710fcfda8"},{"author":{"_account_id":34201,"name":"Mounika Sreeram","email":"sreeram.mounika@ibm.com","username":"sreerammounika"},"change_message_id":"7b50228f95778cb001982a243d7bed419614f547","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"859d53ab_e596f76a","updated":"2022-02-09 14:38:14.000000000","message":"run-IBM Storage CI","commit_id":"ece9a904d0f239ba0c32867b85f7dfc9604f3007"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4853c2516619a201389694e2c171c69e4efbcfa3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b32b2995_b6f6283b","updated":"2022-02-14 12:29:10.000000000","message":"LGTM","commit_id":"88d3005df43e00ac2d93b70be5bc696b1ad3a275"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"7316bb20cd441700c8a2d2b8fdbde90888da3e56","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3fb0baea_6e67d926","updated":"2022-02-14 15:18:24.000000000","message":"Passing CI and it appears that the comments have been addressed.  Thanks!","commit_id":"88d3005df43e00ac2d93b70be5bc696b1ad3a275"},{"author":{"_account_id":34201,"name":"Mounika Sreeram","email":"sreeram.mounika@ibm.com","username":"sreerammounika"},"change_message_id":"97d598b6b83fc4c8cbfedd432993eb95aef232f0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"23e32dec_9468683e","updated":"2022-02-11 15:37:48.000000000","message":"run-IBM Storage CI","commit_id":"88d3005df43e00ac2d93b70be5bc696b1ad3a275"}],"cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c86705f4a5d73f60cd2219645f8bfbd2462912ab","unresolved":true,"context_lines":[{"line_number":13952,"context_line":"                self.assertEqual(1, create_rccg.call_count)"},{"line_number":13953,"context_line":"                update_rep_group.assert_called()"},{"line_number":13954,"context_line":"                self.assertEqual(1, update_rep_group.call_count)"},{"line_number":13955,"context_line":""},{"line_number":13956,"context_line":"                model_update \u003d ("},{"line_number":13957,"context_line":"                    self.driver.delete_group(self.ctxt, clone_group,"},{"line_number":13958,"context_line":"                                             [clone_vol1, clone_vol2]))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1554f932_393ce458","line":13955,"updated":"2021-12-23 16:03:23.000000000","message":"I don\u0027t see any test that checks that the returned volumes_model value actually changes the names.","commit_id":"e9f7006c9189f4bfe667c58291319d08dd59cb6f"},{"author":{"_account_id":32266,"name":"Venkata krishna Thumu","display_name":"VenkataKrishna","email":"venkata.krishna.reddy@ibm.com","username":"venkatakrishnathumu","status":"Active"},"change_message_id":"1e542d3030f5ad733c54bfe3684d76f7d621f58a","unresolved":true,"context_lines":[{"line_number":13952,"context_line":"                self.assertEqual(1, create_rccg.call_count)"},{"line_number":13953,"context_line":"                update_rep_group.assert_called()"},{"line_number":13954,"context_line":"                self.assertEqual(1, update_rep_group.call_count)"},{"line_number":13955,"context_line":""},{"line_number":13956,"context_line":"                model_update \u003d ("},{"line_number":13957,"context_line":"                    self.driver.delete_group(self.ctxt, clone_group,"},{"line_number":13958,"context_line":"                                             [clone_vol1, clone_vol2]))"}],"source_content_type":"text/x-python","patch_set":1,"id":"8604f890_56b8736e","line":13955,"in_reply_to":"1554f932_393ce458","updated":"2021-12-27 06:02:29.000000000","message":"thanks for the comment. Addressed it.","commit_id":"e9f7006c9189f4bfe667c58291319d08dd59cb6f"}],"cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c86705f4a5d73f60cd2219645f8bfbd2462912ab","unresolved":true,"context_lines":[{"line_number":6122,"context_line":"            if model_update.get(\u0027status\u0027) !\u003d fields.GroupStatus.ERROR:"},{"line_number":6123,"context_line":"                # Updating RCCG property to volume metadata"},{"line_number":6124,"context_line":"                for vol in volumes:"},{"line_number":6125,"context_line":"                    volumes_model[volumes.index(vol)]["},{"line_number":6126,"context_line":"                        \u0027metadata\u0027][\u0027Consistency Group Name\u0027] \u003d rccg_name"},{"line_number":6127,"context_line":""},{"line_number":6128,"context_line":"        LOG.debug(\"Leave: create_group_from_src.\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"359f54c9_abed7d21","line":6125,"range":{"start_line":6125,"start_character":19,"end_line":6125,"end_character":54},"updated":"2021-12-23 16:03:23.000000000","message":"this looks weird (and dangerous) to me, we are using the index from one iterable as the index for a different one.\n\nif I\u0027m understanding the code correctly all volumes will be present in the volumes_model and we can just change the name of the CG in all the volumes in volumes_model:\n\n  for model in volumes_model:\n      model[\u0027metadata\u0027][\u0027Consistency Group Name\u0027] \u003d rccg_name\n\nOr is there a case where the volumes_model could have entries that are not present in volumes?","commit_id":"e9f7006c9189f4bfe667c58291319d08dd59cb6f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"843bcf5ed73fd40fd099b116a8bdbcab33f76df6","unresolved":false,"context_lines":[{"line_number":6122,"context_line":"            if model_update.get(\u0027status\u0027) !\u003d fields.GroupStatus.ERROR:"},{"line_number":6123,"context_line":"                # Updating RCCG property to volume metadata"},{"line_number":6124,"context_line":"                for vol in volumes:"},{"line_number":6125,"context_line":"                    volumes_model[volumes.index(vol)]["},{"line_number":6126,"context_line":"                        \u0027metadata\u0027][\u0027Consistency Group Name\u0027] \u003d rccg_name"},{"line_number":6127,"context_line":""},{"line_number":6128,"context_line":"        LOG.debug(\"Leave: create_group_from_src.\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"49dba201_61972b21","line":6125,"range":{"start_line":6125,"start_character":19,"end_line":6125,"end_character":54},"in_reply_to":"32aac825_cf97f26f","updated":"2022-02-08 15:46:04.000000000","message":"Ack","commit_id":"e9f7006c9189f4bfe667c58291319d08dd59cb6f"},{"author":{"_account_id":32266,"name":"Venkata krishna Thumu","display_name":"VenkataKrishna","email":"venkata.krishna.reddy@ibm.com","username":"venkatakrishnathumu","status":"Active"},"change_message_id":"1e542d3030f5ad733c54bfe3684d76f7d621f58a","unresolved":true,"context_lines":[{"line_number":6122,"context_line":"            if model_update.get(\u0027status\u0027) !\u003d fields.GroupStatus.ERROR:"},{"line_number":6123,"context_line":"                # Updating RCCG property to volume metadata"},{"line_number":6124,"context_line":"                for vol in volumes:"},{"line_number":6125,"context_line":"                    volumes_model[volumes.index(vol)]["},{"line_number":6126,"context_line":"                        \u0027metadata\u0027][\u0027Consistency Group Name\u0027] \u003d rccg_name"},{"line_number":6127,"context_line":""},{"line_number":6128,"context_line":"        LOG.debug(\"Leave: create_group_from_src.\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"32aac825_cf97f26f","line":6125,"range":{"start_line":6125,"start_character":19,"end_line":6125,"end_character":54},"in_reply_to":"359f54c9_abed7d21","updated":"2021-12-27 06:02:29.000000000","message":"thanks for the comment. Addressed","commit_id":"e9f7006c9189f4bfe667c58291319d08dd59cb6f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"05a68ea03c5585ec6a3ee44ed62dc4bf954b7680","unresolved":true,"context_lines":[{"line_number":6117,"context_line":"        if volume_utils.is_group_a_type("},{"line_number":6118,"context_line":"                group, \"consistent_group_replication_enabled\"):"},{"line_number":6119,"context_line":"            model_update, added_vols, removed_vols \u003d ("},{"line_number":6120,"context_line":"                self._update_replication_grp(context, group, volumes, []))"},{"line_number":6121,"context_line":"            if model_update.get(\u0027status\u0027) !\u003d fields.GroupStatus.ERROR:"},{"line_number":6122,"context_line":"                # Updating RCCG property to volume metadata"},{"line_number":6123,"context_line":"                for model in volumes_model:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5c6a8af2_bcc24120","line":6120,"range":{"start_line":6120,"start_character":61,"end_line":6120,"end_character":72},"updated":"2022-01-21 15:08:59.000000000","message":"nit: use the named parameters here, it makes it much more obvious that you\u0027re doing the right thing when making this call.","commit_id":"bb9ba48bb0e121cc6a14a10510bc6f45c48e3413"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"721189fca13578e222a7d50440949274c618d4e1","unresolved":true,"context_lines":[{"line_number":6118,"context_line":"                group, \"consistent_group_replication_enabled\"):"},{"line_number":6119,"context_line":"            model_update, added_vols, removed_vols \u003d ("},{"line_number":6120,"context_line":"                self._update_replication_grp(context, group, volumes, []))"},{"line_number":6121,"context_line":"            if model_update.get(\u0027status\u0027) !\u003d fields.GroupStatus.ERROR:"},{"line_number":6122,"context_line":"                # Updating RCCG property to volume metadata"},{"line_number":6123,"context_line":"                for model in volumes_model:"},{"line_number":6124,"context_line":"                    model[\u0027metadata\u0027][\u0027Consistency Group Name\u0027] \u003d rccg_name"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff69b21b_e686d73e","line":6121,"updated":"2022-01-04 21:21:33.000000000","message":"I understand what this code is doing, but I\u0027m not sure why you are adding it here, and not in the self._update_replication_grp() function, or maybe near L6073.","commit_id":"bb9ba48bb0e121cc6a14a10510bc6f45c48e3413"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"05a68ea03c5585ec6a3ee44ed62dc4bf954b7680","unresolved":true,"context_lines":[{"line_number":6118,"context_line":"                group, \"consistent_group_replication_enabled\"):"},{"line_number":6119,"context_line":"            model_update, added_vols, removed_vols \u003d ("},{"line_number":6120,"context_line":"                self._update_replication_grp(context, group, volumes, []))"},{"line_number":6121,"context_line":"            if model_update.get(\u0027status\u0027) !\u003d fields.GroupStatus.ERROR:"},{"line_number":6122,"context_line":"                # Updating RCCG property to volume metadata"},{"line_number":6123,"context_line":"                for model in volumes_model:"},{"line_number":6124,"context_line":"                    model[\u0027metadata\u0027][\u0027Consistency Group Name\u0027] \u003d rccg_name"}],"source_content_type":"text/x-python","patch_set":2,"id":"160e76f6_4d23961c","line":6121,"in_reply_to":"03e7367f_1f45ccbb","updated":"2022-01-21 15:08:59.000000000","message":"Note that the code at line 6092 is using the anti-pattern that Gorka pointed out on PS 1 (using the index from one iterable as the index for a different one).","commit_id":"bb9ba48bb0e121cc6a14a10510bc6f45c48e3413"},{"author":{"_account_id":32036,"name":"katari manoj kumar","email":"katkumar@in.ibm.com","username":"katarimanojkumar"},"change_message_id":"eb44240079b19b064da7a19e38059ecf73156a3e","unresolved":true,"context_lines":[{"line_number":6118,"context_line":"                group, \"consistent_group_replication_enabled\"):"},{"line_number":6119,"context_line":"            model_update, added_vols, removed_vols \u003d ("},{"line_number":6120,"context_line":"                self._update_replication_grp(context, group, volumes, []))"},{"line_number":6121,"context_line":"            if model_update.get(\u0027status\u0027) !\u003d fields.GroupStatus.ERROR:"},{"line_number":6122,"context_line":"                # Updating RCCG property to volume metadata"},{"line_number":6123,"context_line":"                for model in volumes_model:"},{"line_number":6124,"context_line":"                    model[\u0027metadata\u0027][\u0027Consistency Group Name\u0027] \u003d rccg_name"}],"source_content_type":"text/x-python","patch_set":2,"id":"b9191bc1_01a524c5","line":6121,"in_reply_to":"160e76f6_4d23961c","updated":"2022-01-24 11:52:57.000000000","message":"Hi Brian, Thanks for your review. \n\nat L6122 which Gorka pointed out, we just need to update rccg_name for all the volumes models in a loop whereas at L6092 we loop through each volume replication properties and then update volume model_update one by one, using indexes we could achieve it in simple steps. I don\u0027t see any issue there. Pls correct me if i am wrong.\n\nIn case, if we have to change the approach, we might have to add few lines of code to explicitly cross-check and find the correct model update (in volumes_model) and use/update it.","commit_id":"bb9ba48bb0e121cc6a14a10510bc6f45c48e3413"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"54f22c8d8aed747eceb97d7aaefa49a4a5635bcf","unresolved":false,"context_lines":[{"line_number":6118,"context_line":"                group, \"consistent_group_replication_enabled\"):"},{"line_number":6119,"context_line":"            model_update, added_vols, removed_vols \u003d ("},{"line_number":6120,"context_line":"                self._update_replication_grp(context, group, volumes, []))"},{"line_number":6121,"context_line":"            if model_update.get(\u0027status\u0027) !\u003d fields.GroupStatus.ERROR:"},{"line_number":6122,"context_line":"                # Updating RCCG property to volume metadata"},{"line_number":6123,"context_line":"                for model in volumes_model:"},{"line_number":6124,"context_line":"                    model[\u0027metadata\u0027][\u0027Consistency Group Name\u0027] \u003d rccg_name"}],"source_content_type":"text/x-python","patch_set":2,"id":"03e7367f_1f45ccbb","line":6121,"in_reply_to":"bc258874_00a4d6cc","updated":"2022-01-06 17:06:39.000000000","message":"I was initially concerned to see two functions (upgrade_group and _update_replication_grp)  that, according to their names, I thought should handle everything.\n\nBut this patch is in code that seems to *create* the group (L6012 \"create_group_from_src\") and so I see how creating a group may be a little different than updating an existing group.\n\nThis is a long way of saying I\u0027m OK with your explanation! Thanks!","commit_id":"bb9ba48bb0e121cc6a14a10510bc6f45c48e3413"},{"author":{"_account_id":32266,"name":"Venkata krishna Thumu","display_name":"VenkataKrishna","email":"venkata.krishna.reddy@ibm.com","username":"venkatakrishnathumu","status":"Active"},"change_message_id":"8755b68ddc4c2ea981171cc2ab4493ba4a171d81","unresolved":false,"context_lines":[{"line_number":6118,"context_line":"                group, \"consistent_group_replication_enabled\"):"},{"line_number":6119,"context_line":"            model_update, added_vols, removed_vols \u003d ("},{"line_number":6120,"context_line":"                self._update_replication_grp(context, group, volumes, []))"},{"line_number":6121,"context_line":"            if model_update.get(\u0027status\u0027) !\u003d fields.GroupStatus.ERROR:"},{"line_number":6122,"context_line":"                # Updating RCCG property to volume metadata"},{"line_number":6123,"context_line":"                for model in volumes_model:"},{"line_number":6124,"context_line":"                    model[\u0027metadata\u0027][\u0027Consistency Group Name\u0027] \u003d rccg_name"}],"source_content_type":"text/x-python","patch_set":2,"id":"bc258874_00a4d6cc","line":6121,"in_reply_to":"ff69b21b_e686d73e","updated":"2022-01-05 07:35:37.000000000","message":"We have kept this code here to avoid the wrong entries of volume replication_properties due to the call  _update_replication_properties() at L6092. We have seen a case where the property is not updated as expected.","commit_id":"bb9ba48bb0e121cc6a14a10510bc6f45c48e3413"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"843bcf5ed73fd40fd099b116a8bdbcab33f76df6","unresolved":true,"context_lines":[{"line_number":6087,"context_line":"                for model in volumes_model:"},{"line_number":6088,"context_line":"                    if vol.id \u003d\u003d model[\"id\"]:"},{"line_number":6089,"context_line":"                        volume_model \u003d model"},{"line_number":6090,"context_line":"                        break"},{"line_number":6091,"context_line":"                volume_model[\u0027replication_status\u0027] \u003d ("},{"line_number":6092,"context_line":"                    fields.ReplicationStatus.ENABLED)"},{"line_number":6093,"context_line":"                # Updating replication properties for a volume with replication"}],"source_content_type":"text/x-python","patch_set":3,"id":"0adc39fb_875db2da","line":6090,"updated":"2022-02-08 15:46:04.000000000","message":"I assume that it\u0027s impossible for the volume id not to be present, right?","commit_id":"fd610c7aa9214388a1c46b3fe8c9e1c710fcfda8"},{"author":{"_account_id":34201,"name":"Mounika Sreeram","email":"sreeram.mounika@ibm.com","username":"sreerammounika"},"change_message_id":"37aa15c362b57fc23e83a716a95bb420bcd0724f","unresolved":false,"context_lines":[{"line_number":6087,"context_line":"                for model in volumes_model:"},{"line_number":6088,"context_line":"                    if vol.id \u003d\u003d model[\"id\"]:"},{"line_number":6089,"context_line":"                        volume_model \u003d model"},{"line_number":6090,"context_line":"                        break"},{"line_number":6091,"context_line":"                volume_model[\u0027replication_status\u0027] \u003d ("},{"line_number":6092,"context_line":"                    fields.ReplicationStatus.ENABLED)"},{"line_number":6093,"context_line":"                # Updating replication properties for a volume with replication"}],"source_content_type":"text/x-python","patch_set":3,"id":"1b9ff3f5_ebb429b5","line":6090,"in_reply_to":"0adc39fb_875db2da","updated":"2022-02-09 12:16:09.000000000","message":"Yes. volume id will be present","commit_id":"fd610c7aa9214388a1c46b3fe8c9e1c710fcfda8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"843bcf5ed73fd40fd099b116a8bdbcab33f76df6","unresolved":true,"context_lines":[{"line_number":6092,"context_line":"                    fields.ReplicationStatus.ENABLED)"},{"line_number":6093,"context_line":"                # Updating replication properties for a volume with replication"},{"line_number":6094,"context_line":"                # enabled."},{"line_number":6095,"context_line":"                volume_model \u003d (self._update_replication_properties("},{"line_number":6096,"context_line":"                                context, vol, volume_model))"},{"line_number":6097,"context_line":""},{"line_number":6098,"context_line":"            opts \u003d self._get_vdisk_params(vol[\u0027volume_type_id\u0027],"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa89965_910d30b7","line":6095,"range":{"start_line":6095,"start_character":16,"end_line":6095,"end_character":31},"updated":"2022-02-08 15:46:04.000000000","message":"-1: Assigning it to volume_model is misleading, because this variable will not be used at this level.  The _update_replication_properties method already updates the volume_model dictionary.","commit_id":"fd610c7aa9214388a1c46b3fe8c9e1c710fcfda8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"843bcf5ed73fd40fd099b116a8bdbcab33f76df6","unresolved":true,"context_lines":[{"line_number":6092,"context_line":"                    fields.ReplicationStatus.ENABLED)"},{"line_number":6093,"context_line":"                # Updating replication properties for a volume with replication"},{"line_number":6094,"context_line":"                # enabled."},{"line_number":6095,"context_line":"                volume_model \u003d (self._update_replication_properties("},{"line_number":6096,"context_line":"                                context, vol, volume_model))"},{"line_number":6097,"context_line":""},{"line_number":6098,"context_line":"            opts \u003d self._get_vdisk_params(vol[\u0027volume_type_id\u0027],"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ff05e27_f8dc3607","line":6095,"range":{"start_line":6095,"start_character":31,"end_line":6095,"end_character":32},"updated":"2022-02-08 15:46:04.000000000","message":"nit: No need for the enclosing parenthesis","commit_id":"fd610c7aa9214388a1c46b3fe8c9e1c710fcfda8"},{"author":{"_account_id":34201,"name":"Mounika Sreeram","email":"sreeram.mounika@ibm.com","username":"sreerammounika"},"change_message_id":"37aa15c362b57fc23e83a716a95bb420bcd0724f","unresolved":false,"context_lines":[{"line_number":6092,"context_line":"                    fields.ReplicationStatus.ENABLED)"},{"line_number":6093,"context_line":"                # Updating replication properties for a volume with replication"},{"line_number":6094,"context_line":"                # enabled."},{"line_number":6095,"context_line":"                volume_model \u003d (self._update_replication_properties("},{"line_number":6096,"context_line":"                                context, vol, volume_model))"},{"line_number":6097,"context_line":""},{"line_number":6098,"context_line":"            opts \u003d self._get_vdisk_params(vol[\u0027volume_type_id\u0027],"}],"source_content_type":"text/x-python","patch_set":3,"id":"e7de83ee_883d9f15","line":6095,"range":{"start_line":6095,"start_character":16,"end_line":6095,"end_character":31},"in_reply_to":"3fa89965_910d30b7","updated":"2022-02-09 12:16:09.000000000","message":"Thanks for the comment. Addressed it. Changed the scope of volume_model.","commit_id":"fd610c7aa9214388a1c46b3fe8c9e1c710fcfda8"},{"author":{"_account_id":34201,"name":"Mounika Sreeram","email":"sreeram.mounika@ibm.com","username":"sreerammounika"},"change_message_id":"37aa15c362b57fc23e83a716a95bb420bcd0724f","unresolved":false,"context_lines":[{"line_number":6092,"context_line":"                    fields.ReplicationStatus.ENABLED)"},{"line_number":6093,"context_line":"                # Updating replication properties for a volume with replication"},{"line_number":6094,"context_line":"                # enabled."},{"line_number":6095,"context_line":"                volume_model \u003d (self._update_replication_properties("},{"line_number":6096,"context_line":"                                context, vol, volume_model))"},{"line_number":6097,"context_line":""},{"line_number":6098,"context_line":"            opts \u003d self._get_vdisk_params(vol[\u0027volume_type_id\u0027],"}],"source_content_type":"text/x-python","patch_set":3,"id":"e3348881_646f23c3","line":6095,"range":{"start_line":6095,"start_character":31,"end_line":6095,"end_character":32},"in_reply_to":"7ff05e27_f8dc3607","updated":"2022-02-09 12:16:09.000000000","message":"Thanks for the comment. Addressed it.","commit_id":"fd610c7aa9214388a1c46b3fe8c9e1c710fcfda8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"843bcf5ed73fd40fd099b116a8bdbcab33f76df6","unresolved":true,"context_lines":[{"line_number":6102,"context_line":"                # Updating QoS properties for a volume"},{"line_number":6103,"context_line":"                self._helpers.add_vdisk_qos(vol[\u0027name\u0027], opts[\u0027qos\u0027],"},{"line_number":6104,"context_line":"                                            vol[\u0027size\u0027])"},{"line_number":6105,"context_line":"                volume_model \u003d (self._qos_model_update(volume_model, vol))"},{"line_number":6106,"context_line":""},{"line_number":6107,"context_line":"            if is_hyper_group:"},{"line_number":6108,"context_line":"                self._helpers.ensure_vdisk_no_fc_mappings(vol[\u0027name\u0027],"}],"source_content_type":"text/x-python","patch_set":3,"id":"f122f614_d1a13dde","line":6105,"range":{"start_line":6105,"start_character":16,"end_line":6105,"end_character":28},"updated":"2022-02-08 15:46:04.000000000","message":"-1: Same as above, assigning volume_model variable is misleading.","commit_id":"fd610c7aa9214388a1c46b3fe8c9e1c710fcfda8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"843bcf5ed73fd40fd099b116a8bdbcab33f76df6","unresolved":true,"context_lines":[{"line_number":6102,"context_line":"                # Updating QoS properties for a volume"},{"line_number":6103,"context_line":"                self._helpers.add_vdisk_qos(vol[\u0027name\u0027], opts[\u0027qos\u0027],"},{"line_number":6104,"context_line":"                                            vol[\u0027size\u0027])"},{"line_number":6105,"context_line":"                volume_model \u003d (self._qos_model_update(volume_model, vol))"},{"line_number":6106,"context_line":""},{"line_number":6107,"context_line":"            if is_hyper_group:"},{"line_number":6108,"context_line":"                self._helpers.ensure_vdisk_no_fc_mappings(vol[\u0027name\u0027],"}],"source_content_type":"text/x-python","patch_set":3,"id":"4ff32842_74c9377d","line":6105,"range":{"start_line":6105,"start_character":31,"end_line":6105,"end_character":32},"updated":"2022-02-08 15:46:04.000000000","message":"nit: No need for the enclosing parenthesis","commit_id":"fd610c7aa9214388a1c46b3fe8c9e1c710fcfda8"},{"author":{"_account_id":34201,"name":"Mounika Sreeram","email":"sreeram.mounika@ibm.com","username":"sreerammounika"},"change_message_id":"37aa15c362b57fc23e83a716a95bb420bcd0724f","unresolved":false,"context_lines":[{"line_number":6102,"context_line":"                # Updating QoS properties for a volume"},{"line_number":6103,"context_line":"                self._helpers.add_vdisk_qos(vol[\u0027name\u0027], opts[\u0027qos\u0027],"},{"line_number":6104,"context_line":"                                            vol[\u0027size\u0027])"},{"line_number":6105,"context_line":"                volume_model \u003d (self._qos_model_update(volume_model, vol))"},{"line_number":6106,"context_line":""},{"line_number":6107,"context_line":"            if is_hyper_group:"},{"line_number":6108,"context_line":"                self._helpers.ensure_vdisk_no_fc_mappings(vol[\u0027name\u0027],"}],"source_content_type":"text/x-python","patch_set":3,"id":"fd366b15_d9f22d50","line":6105,"range":{"start_line":6105,"start_character":31,"end_line":6105,"end_character":32},"in_reply_to":"4ff32842_74c9377d","updated":"2022-02-09 12:16:09.000000000","message":"Thanks for the comment. Addressed it.","commit_id":"fd610c7aa9214388a1c46b3fe8c9e1c710fcfda8"},{"author":{"_account_id":34201,"name":"Mounika Sreeram","email":"sreeram.mounika@ibm.com","username":"sreerammounika"},"change_message_id":"37aa15c362b57fc23e83a716a95bb420bcd0724f","unresolved":false,"context_lines":[{"line_number":6102,"context_line":"                # Updating QoS properties for a volume"},{"line_number":6103,"context_line":"                self._helpers.add_vdisk_qos(vol[\u0027name\u0027], opts[\u0027qos\u0027],"},{"line_number":6104,"context_line":"                                            vol[\u0027size\u0027])"},{"line_number":6105,"context_line":"                volume_model \u003d (self._qos_model_update(volume_model, vol))"},{"line_number":6106,"context_line":""},{"line_number":6107,"context_line":"            if is_hyper_group:"},{"line_number":6108,"context_line":"                self._helpers.ensure_vdisk_no_fc_mappings(vol[\u0027name\u0027],"}],"source_content_type":"text/x-python","patch_set":3,"id":"3c16d9f6_cb590c9d","line":6105,"range":{"start_line":6105,"start_character":16,"end_line":6105,"end_character":28},"in_reply_to":"f122f614_d1a13dde","updated":"2022-02-09 12:16:09.000000000","message":"Thanks for the comment. Addressed it. Changed the scope of volume_model.","commit_id":"fd610c7aa9214388a1c46b3fe8c9e1c710fcfda8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"49de34578c45ebacd2f23076db75fb34b90b57a4","unresolved":true,"context_lines":[{"line_number":6081,"context_line":"        for vol in volumes:"},{"line_number":6082,"context_line":"            rep_type \u003d self._get_volume_replicated_type(context,"},{"line_number":6083,"context_line":"                                                        vol)"},{"line_number":6084,"context_line":"            volume_model \u003d dict()"},{"line_number":6085,"context_line":"            for model in volumes_model:"},{"line_number":6086,"context_line":"                if vol.id \u003d\u003d model[\"id\"]:"},{"line_number":6087,"context_line":"                    volume_model \u003d model"}],"source_content_type":"text/x-python","patch_set":4,"id":"00cae6f9_34453f10","line":6084,"updated":"2022-02-10 14:57:37.000000000","message":"Sorry, I think I explained myself incorrectly.\nThere are 2 cases:\n- We find the vol.id in volumes_model\n- We don\u0027t find it\n\nIf we find the vol.id then volume_model will point to a dictionary that is already in volumes_model, so when we modify the contents of volume_model then we will be returning that information on the method\u0027s return.\n\nIf we don\u0027t find vol.id we create a new dictionary that is NOT stored in volumes_model and is not used for anything because it\u0027s lost on the next loop iteration.\n\nFor this last case, if we know that volume_model will always be found and its dict will be used, then there is no need to reassign it on L6096 an L6106, because methods _update_replication_properties and _qos_model_update already update the volume_model dictionary, right?\n\nAnd if it isn\u0027t found, then we need to do something with that volume_model value.","commit_id":"ece9a904d0f239ba0c32867b85f7dfc9604f3007"},{"author":{"_account_id":34201,"name":"Mounika Sreeram","email":"sreeram.mounika@ibm.com","username":"sreerammounika"},"change_message_id":"2ae8dface2a4fb32d08e05e262b6b62f27cff8ae","unresolved":false,"context_lines":[{"line_number":6081,"context_line":"        for vol in volumes:"},{"line_number":6082,"context_line":"            rep_type \u003d self._get_volume_replicated_type(context,"},{"line_number":6083,"context_line":"                                                        vol)"},{"line_number":6084,"context_line":"            volume_model \u003d dict()"},{"line_number":6085,"context_line":"            for model in volumes_model:"},{"line_number":6086,"context_line":"                if vol.id \u003d\u003d model[\"id\"]:"},{"line_number":6087,"context_line":"                    volume_model \u003d model"}],"source_content_type":"text/x-python","patch_set":4,"id":"cd64e934_e9569641","line":6084,"in_reply_to":"00cae6f9_34453f10","updated":"2022-02-11 14:47:50.000000000","message":"Thanks for your insight.\n\nAgreed. Reassignment to volume_model is not required.\n\nThe methods _update_replication_properties and _qos_model_update are returning \"model_update\" dictionary. These methods are used in multiple places. Hence, as suggested by you, I have removed reassignment to volume_model at L6096 and L6106 only.","commit_id":"ece9a904d0f239ba0c32867b85f7dfc9604f3007"}]}