)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"25374784bc26c510626e2ab69bf5abc63baf93a0","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2017-10-11 05:41:58 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add the delete exception message with HTTPBadRequest error"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I263c3f5201ac76773a9c962ec958f02f601c6adc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5f4e5783_8359835b","line":8,"updated":"2017-10-11 12:28:55.000000000","message":"Monica, I know this is a pretty simple change but it is best practice to include more than just the summary line here explaining the reason for the change.  Thanks.","commit_id":"f13b8ac4f115d724636c70847dbaeb6fcdcf1a24"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"f9ea316f46203018e554927cf85aa98a7a6cf902","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2017-10-11 08:52:58 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add the delete exception message with HTTPBadRequest error"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I263c3f5201ac76773a9c962ec958f02f601c6adc"},{"line_number":10,"context_line":"Closes-bug:#1722736"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5f4e5783_79b50acd","line":8,"updated":"2017-10-11 20:51:38.000000000","message":"Given there are other comments, can you please add more info here.  Thanks.","commit_id":"be6047841439070aedf78b22ac033adf941cdce5"},{"author":{"_account_id":14532,"name":"Gerald McBrearty","email":"gfm@us.ibm.com","username":"gfm"},"change_message_id":"f47eab37e7c8820707687baef1880824eb824856","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2017-10-11 08:52:58 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add the delete exception message with HTTPBadRequest error"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I263c3f5201ac76773a9c962ec958f02f601c6adc"},{"line_number":10,"context_line":"Closes-bug:#1722736"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5f4e5783_b331680e","line":8,"updated":"2017-10-11 21:58:49.000000000","message":"Yes please","commit_id":"be6047841439070aedf78b22ac033adf941cdce5"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"7f28f5e7dea6a6600ea4c14d75b31a6ddb1d759b","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Message change for Policy error message change when user is not-authorized"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In role based access control test case, a consistencyGroup snapshot delete when"},{"line_number":10,"context_line":"attempted by Non-admin role/user does return an informative policy not"},{"line_number":11,"context_line":"authorized message."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The proposed fix catches the PolicyNotAuthorized exception explicitly and hence"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5f4e5783_3354e152","line":10,"range":{"start_line":10,"start_character":32,"end_line":10,"end_character":44},"updated":"2017-10-16 17:40:01.000000000","message":"Do you \u0027does not return\u0027 ?","commit_id":"3daa9f238238ebec97f6435bb32088673c6d0457"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"48076e3cf4ec3f5ec8272a976431fc8bcde3cfaf","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     MonicaJoshi \u003cmonjoshi@in.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-10-19 08:29:55 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Message change for Policy error message change when user is not-authorized"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In role based access control test case, a consistencyGroup snapshot delete when"},{"line_number":10,"context_line":"attempted by Non-admin role/user does not return an informative policy not"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f4b6375_c334294d","line":7,"updated":"2017-10-19 12:59:46.000000000","message":"If you do another update, please reformat your commit message to follow guidelines:\n\nhttps://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure","commit_id":"893ac8f2b6ec246a8698939b0d440f2a2cff3e33"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"d9c66b7ee762caf62c117e1911689d6577307a6c","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     MonicaJoshi \u003cmonjoshi@in.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-10-20 05:32:04 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Raise PolicyNotAuthorized exception on consistencyGroup snapshot by an user"},{"line_number":8,"context_line":"not-authorized"},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"In role based access control test case, a consistencyGroup snapshot delete when"},{"line_number":11,"context_line":"attempted by Non-admin role/user does not return an informative policy not"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"3f4b6375_6561c0a9","line":8,"range":{"start_line":7,"start_character":0,"end_line":8,"end_character":14},"updated":"2017-10-22 22:28:36.000000000","message":"This needs to be reduced to one line.","commit_id":"ef7acda8906025abeee7335f5086d6c4cf44e43a"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"d9c66b7ee762caf62c117e1911689d6577307a6c","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Raise PolicyNotAuthorized exception on consistencyGroup snapshot by an user"},{"line_number":8,"context_line":"not-authorized"},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"In role based access control test case, a consistencyGroup snapshot delete when"},{"line_number":11,"context_line":"attempted by Non-admin role/user does not return an informative policy not"},{"line_number":12,"context_line":"authorized message."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"3f4b6375_255bc8f7","line":10,"updated":"2017-10-22 22:28:36.000000000","message":"Please try to wrap lines at 70 chars.  Thanks!","commit_id":"ef7acda8906025abeee7335f5086d6c4cf44e43a"}],"cinder/api/contrib/cgsnapshots.py":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"a5805112538e2c21b82d7ff2f1affe5eaf10c521","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        except exception.InvalidGroupSnapshot as e:"},{"line_number":68,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dsix.text_type(e))"},{"line_number":69,"context_line":"        except Exception as e:"},{"line_number":70,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dsix.text_type(e))"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        return webob.Response(status_int\u003dhttp_client.ACCEPTED)"},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f4e5783_f65917fe","line":70,"updated":"2017-10-11 12:37:57.000000000","message":"A couple problems with this:\n\n1) Same as previous except block, so in that respect this should just be merged together into one, but...\n2) This is too broad where it will end up returning _any_ exception details as part of the bad request message. That could potentially return more internal detail than we would want to expose.\n\nI think the best approach would be to add PolicyNotAuthorized to the InvalidGroupSnapshot exception handling, and keep Exception as a catch-all for anything else that we would want to hide with the more general \"Failed...\" message.\n\nThen there is the bigger issue that this code is basically all deprecated by the group APIs and not really used anymore.","commit_id":"f13b8ac4f115d724636c70847dbaeb6fcdcf1a24"},{"author":{"_account_id":14532,"name":"Gerald McBrearty","email":"gfm@us.ibm.com","username":"gfm"},"change_message_id":"f47eab37e7c8820707687baef1880824eb824856","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        except exception.InvalidGroupSnapshot as e:"},{"line_number":68,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dsix.text_type(e))"},{"line_number":69,"context_line":"        except Exception as e:"},{"line_number":70,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dsix.text_type(e))"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        return webob.Response(status_int\u003dhttp_client.ACCEPTED)"},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f4e5783_cfe9cc1a","line":70,"in_reply_to":"5f4e5783_f65917fe","updated":"2017-10-11 21:58:49.000000000","message":"Sean so should we be making this change given that cg api is deprecated?","commit_id":"f13b8ac4f115d724636c70847dbaeb6fcdcf1a24"},{"author":{"_account_id":23083,"name":"TommyLike","email":"tommylikehu@gmail.com","username":"TommyLike"},"change_message_id":"779143bd4a0b7957210dda0a03a05dbe507a579a","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        except exception.GroupSnapshotNotFound:"},{"line_number":65,"context_line":"            # Not found exception will be handled at the wsgi level"},{"line_number":66,"context_line":"            raise"},{"line_number":67,"context_line":"        except (exception.InvalidGroupSnapshot,"},{"line_number":68,"context_line":"                exception.PolicyNotAuthorized) as e:"},{"line_number":69,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dsix.text_type(e))"},{"line_number":70,"context_line":"        except Exception:"},{"line_number":71,"context_line":"            msg \u003d _(\"Failed to delete the cgsnapshot\")"},{"line_number":72,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_b27960fd","line":69,"range":{"start_line":67,"start_character":5,"end_line":69,"end_character":66},"updated":"2017-10-20 06:49:17.000000000","message":"if you want to get the 403 status code, please raise the PolicyNotAuthorized directly. Don\u0027t wrap it in the HTTPBadRequest object.","commit_id":"893ac8f2b6ec246a8698939b0d440f2a2cff3e33"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"f76ee8c41403eb4180c1449570ba6965239444bd","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        try:"},{"line_number":62,"context_line":"            cgsnapshot \u003d self._get_cgsnapshot(context, id)"},{"line_number":63,"context_line":"            self.group_snapshot_api.delete_group_snapshot(context, cgsnapshot)"},{"line_number":64,"context_line":"        except (exception.GroupSnapshotNotFound,"},{"line_number":65,"context_line":"                exception.PolicyNotAuthorized):"},{"line_number":66,"context_line":"            # Not found exception will be handled at the wsgi level"},{"line_number":67,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f4b6375_e76a187f","line":64,"updated":"2017-10-24 20:56:16.000000000","message":"should use oslo_utils.excutils.save_and_reraise_exception... see https://github.com/openstack/oslo.utils/blob/824adacdfc4b2fad6556a32702737284948a7059/oslo_utils/excutils.py#L146 for reasoning and usage examples (there are also plenty of examples through the cinder code base).","commit_id":"ef7acda8906025abeee7335f5086d6c4cf44e43a"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"37211c1186e0d77ae3dcc8638e89d3415945bbcf","unresolved":false,"context_lines":[{"line_number":63,"context_line":"            self.group_snapshot_api.delete_group_snapshot(context, cgsnapshot)"},{"line_number":64,"context_line":"        except (exception.GroupSnapshotNotFound,"},{"line_number":65,"context_line":"                exception.PolicyNotAuthorized):"},{"line_number":66,"context_line":"            # Not found exception will be handled at the wsgi level"},{"line_number":67,"context_line":"            raise"},{"line_number":68,"context_line":"        except exception.InvalidGroupSnapshot as e:"},{"line_number":69,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dsix.text_type(e))"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f4b6375_a77780a0","line":66,"updated":"2017-10-24 20:46:18.000000000","message":"comment should be updated since this block is no longer specific to \"not found\" exceptions.","commit_id":"ef7acda8906025abeee7335f5086d6c4cf44e43a"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"f76ee8c41403eb4180c1449570ba6965239444bd","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            raise"},{"line_number":68,"context_line":"        except exception.InvalidGroupSnapshot as e:"},{"line_number":69,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dsix.text_type(e))"},{"line_number":70,"context_line":"        except Exception:"},{"line_number":71,"context_line":"            msg \u003d _(\"Failed to delete the cgsnapshot\")"},{"line_number":72,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f4b6375_47432c14","line":70,"updated":"2017-10-24 20:56:16.000000000","message":"\"except Exception\" is almost always a bad idea, and that is certainly the case here. It is too broad, and will catch more than is intended, as we\u0027re seeing. E.g. there are many possible Exceptions that could happen here that should result in HTTP 500, rather than the HTTP 400 that this will end up returning.\n\nYou could argue that\u0027s a separate problem, but it\u0027s really what led to the bug you\u0027re trying to fix. If it wasn\u0027t so broad as to catch PolicyNotAuthorized, we wouldn\u0027t have that bug. So fixing this would be another (better) way to fix the bug.","commit_id":"ef7acda8906025abeee7335f5086d6c4cf44e43a"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"74ab042ee84675885b021ecc2261a276991f7e30","unresolved":false,"context_lines":[{"line_number":69,"context_line":"                # NotFound exception will be handled at the wsgi level"},{"line_number":70,"context_line":"                ctxt.reraise \u003d True"},{"line_number":71,"context_line":"        except exception.PolicyNotAuthorized as e:"},{"line_number":72,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":73,"context_line":"                # NotAuthorized exception will be handled at the wsgi level"},{"line_number":74,"context_line":"                ctxt.reraise \u003d True"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"df87a7cf_956b1d83","line":72,"updated":"2017-12-12 15:47:01.000000000","message":"Why do all this just to raise? Should add be:\n\nexcept exception.PolicyNotAuthorized:\n    raise\nexcept Exception:\n    msg\u003d_(\u0027Failed cgsnapshot\u0027)\n    raise exc.HTTPBadRequest(explanation\u003dmsg)","commit_id":"4bcca6513d000851f970251a3a0b358e9dd68750"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"dc7dd7e86ac9741aa24f49078618bb4dacbf153b","unresolved":false,"context_lines":[{"line_number":66,"context_line":"                raise exc.HTTPBadRequest(explanation\u003dsix.text_type(e))"},{"line_number":67,"context_line":"        except (exception.GroupSnapshotNotFound,"},{"line_number":68,"context_line":"                exception.PolicyNotAuthorized) as e:"},{"line_number":69,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":70,"context_line":"                # Exceptions will be handled at the wsgi level"},{"line_number":71,"context_line":"                raise"},{"line_number":72,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"df87a7cf_6bb2ad96","line":69,"range":{"start_line":69,"start_character":12,"end_line":69,"end_character":55},"updated":"2017-12-18 11:58:13.000000000","message":"As previously stated, this is not needed and the catchall Exception should remain.","commit_id":"0a180c97606bb6a88a4927c3827a45851c043bc7"}],"cinder/exception.py":[{"author":{"_account_id":23083,"name":"TommyLike","email":"tommylikehu@gmail.com","username":"TommyLike"},"change_message_id":"779143bd4a0b7957210dda0a03a05dbe507a579a","unresolved":false,"context_lines":[{"line_number":174,"context_line":""},{"line_number":175,"context_line":"class PolicyNotAuthorized(NotAuthorized):"},{"line_number":176,"context_line":"    message \u003d _(\"Policy doesn\u0027t allow %(action)s to be performed.\")"},{"line_number":177,"context_line":"    code \u003d 403"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"class ImageNotAuthorized(CinderException):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_d2ee1cb3","line":177,"range":{"start_line":177,"start_character":3,"end_line":177,"end_character":14},"updated":"2017-10-20 06:49:17.000000000","message":"You don\u0027t need to define \u0027code\u003d403\u0027 here, as it\u0027s already defined in it\u0027s parent.","commit_id":"893ac8f2b6ec246a8698939b0d440f2a2cff3e33"}]}
