)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"26a7d22ed66a1709945e6ee76d4dc54004641f92","unresolved":true,"context_lines":[{"line_number":9,"context_line":"This commit changes the os-reset-status group-api"},{"line_number":10,"context_line":"to handle the below use case"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"1.reset group to the \"ing\" state"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Partial implement blueprint reset-state-robustification"},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"213b6e53_05363ae8","line":12,"range":{"start_line":12,"start_character":0,"end_line":12,"end_character":32},"updated":"2022-03-08 03:04:09.000000000","message":"For people who haven\u0027t read the spec, this is confusing.  You should explicitly state that what you are doing is:\n\n 1. resetting a group\u0027s status to \u0027updating\u0027, \u0027deleting\u0027 or \u0027error_deleting\u0027 is now disallowed\n\nAlso, don\u0027t forget to add the \"boilerplate\" text from https://etherpad.opendev.org/p/no-foot-shooting in this commit message.","commit_id":"8aa73ed2a5a02642b937441eef16a7ca263fbf23"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"d6a5c607fa2f9804abb5d17971d55e1f3eeb22ac","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This commit changes the os-reset-status group-api"},{"line_number":10,"context_line":"to handle the below use case"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"1.reset group to the \"ing\" state"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Partial implement blueprint reset-state-robustification"},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"0e25f185_4b773fde","line":12,"range":{"start_line":12,"start_character":0,"end_line":12,"end_character":32},"in_reply_to":"213b6e53_05363ae8","updated":"2022-03-10 02:59:39.000000000","message":"Done","commit_id":"8aa73ed2a5a02642b937441eef16a7ca263fbf23"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"ddb933174839248e33172d19637cef1d229ce71d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"1b48b66d_29c61fcd","updated":"2022-02-09 15:07:38.000000000","message":"Based on discussions I think this is ok.","commit_id":"411e5cfcbf82a65360a8a3a7d269bff938be8c4e"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"0b9d75e2a6203d09945ef81bbdd75f525699ea7f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"0a65077c_7c24aac9","updated":"2022-02-14 20:49:19.000000000","message":"Looks like a necessary check and CI passed. ","commit_id":"411e5cfcbf82a65360a8a3a7d269bff938be8c4e"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"0ac3bc2c6ce97b5538996e68db6f0f0bf8855495","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"aa35e04f_e37a42a0","updated":"2022-02-22 07:51:53.000000000","message":"recheck","commit_id":"b560687e8641831726f3b2e8b463493726c8065f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"26a7d22ed66a1709945e6ee76d4dc54004641f92","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"31c11cf1_36c1078e","updated":"2022-03-08 03:04:09.000000000","message":"See comments inline.","commit_id":"8aa73ed2a5a02642b937441eef16a7ca263fbf23"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"25910c61378f5aa2ebeab4e235dc92be6dc4fd19","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":27,"id":"7318f6b0_a0f65762","updated":"2022-03-10 03:29:40.000000000","message":"See comment inline.","commit_id":"415b712b262d5075a0e04e59fbbdcb912a93c071"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"12831a20315cafff5ad267c72d9ac3e0b389b849","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":29,"id":"cea97a50_db53c096","updated":"2022-05-12 11:00:35.000000000","message":"recheck","commit_id":"d7a1dda799263c29e62fff58833f86f5aba50e82"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"058e9d57bdd41ac4bff67e723d6ff7baa88598ae","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"e82a8b7f_02582c05","updated":"2022-09-01 10:35:25.000000000","message":"hi core reviewers could you help me to merge this patch","commit_id":"f6785eb46e9292feb285f249ef7f8d1e05e240bd"}],"cinder/api/v3/groups.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"26a7d22ed66a1709945e6ee76d4dc54004641f92","unresolved":true,"context_lines":[{"line_number":89,"context_line":"                           \u0027update\u0027: status})"},{"line_number":90,"context_line":"            group \u003d self.group_api.get(context, id)"},{"line_number":91,"context_line":"            # status \u003d group.get(\u0027status\u0027)"},{"line_number":92,"context_line":"            if group in (\u0027updating\u0027, \u0027deleting\u0027, \u0027error_deleting\u0027):"},{"line_number":93,"context_line":"                msg \u003d _(\"Cannot reset-state to %s\" % group)"},{"line_number":94,"context_line":"                raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":95,"context_line":"            self.group_api.reset_status(context, group, status)"}],"source_content_type":"text/x-python","patch_set":26,"id":"6f295632_d0158ffb","line":92,"range":{"start_line":92,"start_character":12,"end_line":92,"end_character":66},"updated":"2022-03-08 03:04:09.000000000","message":"Two things:\n\n1. I think this condition will always be false because \u0027group\u0027 is an Oslo Versioned Object that won\u0027t be in the tuple of strings.  I think what you should be checking here is the status variable you set in commented-out line 91.\n\n2. Since what you\u0027re proposing to do here is to prohibit any reset-status request based only on the target state, you don\u0027t need to know the current status of the  group.  So you can move lines 92-94 to before the \u0027try\u0027 block so you can fail before you issue the \u0027start\u0027 notification.","commit_id":"8aa73ed2a5a02642b937441eef16a7ca263fbf23"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"d6a5c607fa2f9804abb5d17971d55e1f3eeb22ac","unresolved":false,"context_lines":[{"line_number":89,"context_line":"                           \u0027update\u0027: status})"},{"line_number":90,"context_line":"            group \u003d self.group_api.get(context, id)"},{"line_number":91,"context_line":"            # status \u003d group.get(\u0027status\u0027)"},{"line_number":92,"context_line":"            if group in (\u0027updating\u0027, \u0027deleting\u0027, \u0027error_deleting\u0027):"},{"line_number":93,"context_line":"                msg \u003d _(\"Cannot reset-state to %s\" % group)"},{"line_number":94,"context_line":"                raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":95,"context_line":"            self.group_api.reset_status(context, group, status)"}],"source_content_type":"text/x-python","patch_set":26,"id":"67d7facd_fcd73e3e","line":92,"range":{"start_line":92,"start_character":12,"end_line":92,"end_character":66},"in_reply_to":"6f295632_d0158ffb","updated":"2022-03-10 02:59:39.000000000","message":"Done","commit_id":"8aa73ed2a5a02642b937441eef16a7ca263fbf23"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"25910c61378f5aa2ebeab4e235dc92be6dc4fd19","unresolved":true,"context_lines":[{"line_number":89,"context_line":"                           \u0027update\u0027: status})"},{"line_number":90,"context_line":"            group \u003d self.group_api.get(context, id)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"            if status in (\u0027updating\u0027, \u0027deleting\u0027, \u0027error_deleting\u0027):"},{"line_number":93,"context_line":"                msg \u003d _(\"Cannot reset-state to %s\" % group)"},{"line_number":94,"context_line":"                raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":95,"context_line":"            self.group_api.reset_status(context, group, status)"},{"line_number":96,"context_line":"            notifier.info(context, \u0027groups.reset_status.end\u0027,"},{"line_number":97,"context_line":"                          {\u0027id\u0027: id,"}],"source_content_type":"text/x-python","patch_set":27,"id":"81b07a43_b2496610","line":94,"range":{"start_line":92,"start_character":0,"end_line":94,"end_character":63},"updated":"2022-03-10 03:29:40.000000000","message":"This needs to be checked before the notification is sent, because if you decide to reject the request, no \u0027end\u0027 notification will be sent.  It also doesn\u0027t need to be inside the \u0027try\u0027 block, because it\u0027s simply looking at the status that the caller has passed in the request body.  So I\u0027d move it to before line 85, and possibly before you log the message that you\u0027re going to update the group (because you aren\u0027t going to do that).  We can see what other reviewers think, but I\u0027d probably just not log anything.  Either that, or log a debug message that an illegal state reset was requested before you raise the BadRequest exception (but definitely do that *before* the current log statement at line 82).","commit_id":"415b712b262d5075a0e04e59fbbdcb912a93c071"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"1cc46f954d4253d9e1b6ab6f6727c71bae7d91be","unresolved":false,"context_lines":[{"line_number":89,"context_line":"                           \u0027update\u0027: status})"},{"line_number":90,"context_line":"            group \u003d self.group_api.get(context, id)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"            if status in (\u0027updating\u0027, \u0027deleting\u0027, \u0027error_deleting\u0027):"},{"line_number":93,"context_line":"                msg \u003d _(\"Cannot reset-state to %s\" % group)"},{"line_number":94,"context_line":"                raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":95,"context_line":"            self.group_api.reset_status(context, group, status)"},{"line_number":96,"context_line":"            notifier.info(context, \u0027groups.reset_status.end\u0027,"},{"line_number":97,"context_line":"                          {\u0027id\u0027: id,"}],"source_content_type":"text/x-python","patch_set":27,"id":"99976603_e17e7888","line":94,"range":{"start_line":92,"start_character":0,"end_line":94,"end_character":63},"in_reply_to":"81b07a43_b2496610","updated":"2022-03-10 05:19:02.000000000","message":"Done","commit_id":"415b712b262d5075a0e04e59fbbdcb912a93c071"}],"cinder/tests/unit/group/test_groups_api.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"26a7d22ed66a1709945e6ee76d4dc54004641f92","unresolved":true,"context_lines":[{"line_number":309,"context_line":"                        \u0027status\u0027: fields.GroupStatus.ERROR_DELETING}"},{"line_number":310,"context_line":"        mock_group.update.assert_called_once_with(update_field)"},{"line_number":311,"context_line":"        mock_group.save.assert_called_once_with()"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"    @mock.patch.object(GROUP_QUOTAS, \"reserve\")"},{"line_number":314,"context_line":"    @mock.patch(\u0027cinder.objects.Group\u0027)"},{"line_number":315,"context_line":"    @mock.patch(\u0027cinder.db.group_type_get_by_name\u0027)"}],"source_content_type":"text/x-python","patch_set":26,"id":"ad481930_534f50f9","line":312,"updated":"2022-03-08 03:04:09.000000000","message":"A red flag about your set of tests is that the change you are making will raise an HTTPBadRequest exception, but none of your tests look for that condition.","commit_id":"8aa73ed2a5a02642b937441eef16a7ca263fbf23"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"de345f16299d0f8277e64034667660e84bbfe144","unresolved":false,"context_lines":[{"line_number":309,"context_line":"                        \u0027status\u0027: fields.GroupStatus.ERROR_DELETING}"},{"line_number":310,"context_line":"        mock_group.update.assert_called_once_with(update_field)"},{"line_number":311,"context_line":"        mock_group.save.assert_called_once_with()"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"    @mock.patch.object(GROUP_QUOTAS, \"reserve\")"},{"line_number":314,"context_line":"    @mock.patch(\u0027cinder.objects.Group\u0027)"},{"line_number":315,"context_line":"    @mock.patch(\u0027cinder.db.group_type_get_by_name\u0027)"}],"source_content_type":"text/x-python","patch_set":26,"id":"28c9ad77_bb05c3d1","line":312,"in_reply_to":"69b67701_33ec4043","updated":"2022-03-10 02:59:54.000000000","message":"Done","commit_id":"8aa73ed2a5a02642b937441eef16a7ca263fbf23"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"d6a5c607fa2f9804abb5d17971d55e1f3eeb22ac","unresolved":true,"context_lines":[{"line_number":309,"context_line":"                        \u0027status\u0027: fields.GroupStatus.ERROR_DELETING}"},{"line_number":310,"context_line":"        mock_group.update.assert_called_once_with(update_field)"},{"line_number":311,"context_line":"        mock_group.save.assert_called_once_with()"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"    @mock.patch.object(GROUP_QUOTAS, \"reserve\")"},{"line_number":314,"context_line":"    @mock.patch(\u0027cinder.objects.Group\u0027)"},{"line_number":315,"context_line":"    @mock.patch(\u0027cinder.db.group_type_get_by_name\u0027)"}],"source_content_type":"text/x-python","patch_set":26,"id":"69b67701_33ec4043","line":312,"in_reply_to":"ad481930_534f50f9","updated":"2022-03-10 02:59:39.000000000","message":"ok i\u0027ll update","commit_id":"8aa73ed2a5a02642b937441eef16a7ca263fbf23"}]}