)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2a503d588374adae0f4d706e13565086b236b620","unresolved":false,"context_lines":[{"line_number":18,"context_line":"Nova should return a BadRequest response(400) in this case, because"},{"line_number":19,"context_line":"new_volume_id is specified in a request body."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"This patch is still WIP because it is necessary to decide we need a"},{"line_number":22,"context_line":"new microversion or not for this case."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"[1]: https://github.com/openstack/api-wg/blob/master/guidelines/http.rst#failure-code-clarifications"},{"line_number":25,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a98d1f4_687873c5","line":22,"range":{"start_line":21,"start_character":0,"end_line":22,"end_character":38},"updated":"2016-09-29 12:54:07.000000000","message":"Since the method already returns 400 and 404, I think changing the new_volume_id to be a 400 is fine, a client should expect either one.","commit_id":"1fc8079d6f39e784ce6d6c1e3b31a7533d9ac85d"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"d984ba0675cf7298d15a1b0cea297f329f2f9457","unresolved":false,"context_lines":[{"line_number":18,"context_line":"Nova should return a BadRequest response(400) in this case, because"},{"line_number":19,"context_line":"new_volume_id is specified in a request body."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"This patch is still WIP because it is necessary to decide we need a"},{"line_number":22,"context_line":"new microversion or not for this case."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"[1]: https://github.com/openstack/api-wg/blob/master/guidelines/http.rst#failure-code-clarifications"},{"line_number":25,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a98d1f4_81bf43fa","line":22,"range":{"start_line":21,"start_character":0,"end_line":22,"end_character":38},"in_reply_to":"3a98d1f4_687873c5","updated":"2016-09-29 18:48:41.000000000","message":"yeah, I agree with not having a new microversion.\n\nNow Tempest also doesn\u0027t have such test as it.\nIn addition, I am trying to add some guideline about negative test on https://review.openstack.org/#/c/379013/\n\nThe basic idea is that all negative tests should be based on the api-wg guideline. So this kind of bugs should be detected on Tempest more easily, I hope.","commit_id":"1fc8079d6f39e784ce6d6c1e3b31a7533d9ac85d"}],"nova/api/openstack/compute/volumes.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"460efa5dd125eabae998e914753b798bc78f9b17","unresolved":false,"context_lines":[{"line_number":364,"context_line":"        try:"},{"line_number":365,"context_line":"            old_volume \u003d self.volume_api.get(context, old_volume_id)"},{"line_number":366,"context_line":"        except exception.VolumeNotFound as e:"},{"line_number":367,"context_line":"            raise exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"        new_volume_id \u003d body[\u0027volumeAttachment\u0027][\u0027volumeId\u0027]"},{"line_number":370,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_28a02b43","line":367,"range":{"start_line":367,"start_character":22,"end_line":367,"end_character":34},"updated":"2016-09-29 12:52:49.000000000","message":"So why is it OK for the old volume to raise a 404 but the volume to swap *to* raises a 400?","commit_id":"1fc8079d6f39e784ce6d6c1e3b31a7533d9ac85d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aec59df4d6e78a6e3aa1646c69cb23598d209c24","unresolved":false,"context_lines":[{"line_number":364,"context_line":"        try:"},{"line_number":365,"context_line":"            old_volume \u003d self.volume_api.get(context, old_volume_id)"},{"line_number":366,"context_line":"        except exception.VolumeNotFound as e:"},{"line_number":367,"context_line":"            raise exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"        new_volume_id \u003d body[\u0027volumeAttachment\u0027][\u0027volumeId\u0027]"},{"line_number":370,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_68b1538f","line":367,"range":{"start_line":367,"start_character":22,"end_line":367,"end_character":34},"in_reply_to":"3a98d1f4_28a02b43","updated":"2016-09-29 12:53:23.000000000","message":"Oh I see, the old_volume_id is in the URI, the new_volume_id is in the body...","commit_id":"1fc8079d6f39e784ce6d6c1e3b31a7533d9ac85d"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"d984ba0675cf7298d15a1b0cea297f329f2f9457","unresolved":false,"context_lines":[{"line_number":364,"context_line":"        try:"},{"line_number":365,"context_line":"            old_volume \u003d self.volume_api.get(context, old_volume_id)"},{"line_number":366,"context_line":"        except exception.VolumeNotFound as e:"},{"line_number":367,"context_line":"            raise exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"        new_volume_id \u003d body[\u0027volumeAttachment\u0027][\u0027volumeId\u0027]"},{"line_number":370,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_8104a3c0","line":367,"range":{"start_line":367,"start_character":22,"end_line":367,"end_character":34},"in_reply_to":"3a98d1f4_68b1538f","updated":"2016-09-29 18:48:41.000000000","message":"Yeah, you are right.\nIt is easy to make this kind of bugs on the code..","commit_id":"1fc8079d6f39e784ce6d6c1e3b31a7533d9ac85d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"60793a13003a08aa42e4404a5a21c4b189b91a1a","unresolved":false,"context_lines":[{"line_number":370,"context_line":"        try:"},{"line_number":371,"context_line":"            new_volume \u003d self.volume_api.get(context, new_volume_id)"},{"line_number":372,"context_line":"        except exception.VolumeNotFound as e:"},{"line_number":373,"context_line":"            raise exc.HTTPBadRequest(explanation\u003de.format_message())"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"        instance \u003d common.get_instance(self.compute_api, context, server_id)"},{"line_number":376,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3a98d1f4_79d2ae5d","line":373,"updated":"2016-09-30 17:48:39.000000000","message":"nit: it might be worth having a comment in here explaining why it\u0027s a 400 (because it\u0027s in the body of the request rather than the path), because I could see someone coming along a year from now and undoing this.","commit_id":"2fbdf1d35b15282739acd90cbae1060abf02e800"}]}
