)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"04674e231f39c8f94199273e8f3285ffd1f0c1f3","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently, if we want to manage a volume in the K2 array,"},{"line_number":10,"context_line":"we rename the volume and associated VG in K2"},{"line_number":11,"context_line":"according to cv-CinderVolumeId for volume and"},{"line_number":12,"context_line":"cvg-CinderVolumeId for VG."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"So, if K2 volume is managed by cinder, then for any future"},{"line_number":15,"context_line":"reference for the managed volume, cv-CinderVolumeId and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a71b18c_b3d87954","line":12,"range":{"start_line":11,"start_character":13,"end_line":12,"end_character":26},"updated":"2016-12-07 11:14:21.000000000","message":"Is it possible in any situation to have a Cinder volume belonging to more than 1 VG? You have 1 VG and gvg per volume?","commit_id":"8f97995c69ab5d52d21f63cfc7379dda20d51303"},{"author":{"_account_id":16258,"name":"nikesh","email":"nike.niec@gmail.com","username":"nikem"},"change_message_id":"82635f431de923af9c742060f6ab35b3f98ae84d","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently, if we want to manage a volume in the K2 array,"},{"line_number":10,"context_line":"we rename the volume and associated VG in K2"},{"line_number":11,"context_line":"according to cv-CinderVolumeId for volume and"},{"line_number":12,"context_line":"cvg-CinderVolumeId for VG."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"So, if K2 volume is managed by cinder, then for any future"},{"line_number":15,"context_line":"reference for the managed volume, cv-CinderVolumeId and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a71b18c_7aeeb1b5","line":12,"range":{"start_line":11,"start_character":13,"end_line":12,"end_character":26},"in_reply_to":"3a71b18c_b3d87954","updated":"2016-12-07 12:50:54.000000000","message":"As of now, one Kaminario cinder volume belongs to only one Kaminario Volume group.","commit_id":"8f97995c69ab5d52d21f63cfc7379dda20d51303"}],"cinder/volume/drivers/kaminario/kaminario_common.py":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"04674e231f39c8f94199273e8f3285ffd1f0c1f3","unresolved":false,"context_lines":[{"line_number":1035,"context_line":"            vol_map \u003d False"},{"line_number":1036,"context_line":"            if self.client.search(\"mappings\", volume\u003dvol).total !\u003d 0:"},{"line_number":1037,"context_line":"                vol_map \u003d True"},{"line_number":1038,"context_line":"            if is_dedup !\u003d vg.is_dedup or vg_replica or vol_map or nvol !\u003d 1:"},{"line_number":1039,"context_line":"                raise exception.ManageExistingInvalidReference("},{"line_number":1040,"context_line":"                    existing_ref\u003dexisting_ref,"},{"line_number":1041,"context_line":"                    reason\u003d_(\u0027Manage volume type invalid.\u0027))"},{"line_number":1042,"context_line":"            vol.name \u003d new_name"},{"line_number":1043,"context_line":"            vg_name \u003d vg.name"},{"line_number":1044,"context_line":"            LOG.debug(\"Manage new volume name: %s\", new_name)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_734fc133","line":1041,"range":{"start_line":1038,"start_character":11,"end_line":1041,"end_character":60},"updated":"2016-12-07 11:14:21.000000000","message":"This error message is not clear and can make a hard time to the admin. You need to explain to the user why the volume is not valid to manage.","commit_id":"8f97995c69ab5d52d21f63cfc7379dda20d51303"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"bdb292159d961c34da44fcdb75b6daec07784d38","unresolved":false,"context_lines":[{"line_number":1035,"context_line":"            vol_map \u003d False"},{"line_number":1036,"context_line":"            if self.client.search(\"mappings\", volume\u003dvol).total !\u003d 0:"},{"line_number":1037,"context_line":"                vol_map \u003d True"},{"line_number":1038,"context_line":"            if is_dedup !\u003d vg.is_dedup or vg_replica or vol_map or nvol !\u003d 1:"},{"line_number":1039,"context_line":"                raise exception.ManageExistingInvalidReference("},{"line_number":1040,"context_line":"                    existing_ref\u003dexisting_ref,"},{"line_number":1041,"context_line":"                    reason\u003d_(\u0027Manage volume type invalid.\u0027))"},{"line_number":1042,"context_line":"            vol.name \u003d new_name"},{"line_number":1043,"context_line":"            vg_name \u003d vg.name"},{"line_number":1044,"context_line":"            LOG.debug(\"Manage new volume name: %s\", new_name)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_281f930a","line":1041,"range":{"start_line":1038,"start_character":11,"end_line":1041,"end_character":60},"in_reply_to":"3a71b18c_5aac958c","updated":"2016-12-07 13:25:44.000000000","message":"I do think this could be a little confusing to an admin. Can you check each value and report the reason appropriately?\n\nSomething like\n\nif is_dedup !\u003d vg.is_dedup:\n    reason \u003d \u0027Dudupe setting mismatch.\u0027\nelse if vg_replica:\n    reason \u003d \u0027Volume is a replica.\u0027\nelse:\n    reason \u003d \u0027More than one volume in a vg is not supported.\u0027\n\nraise exception.Man...\n    reason\u003d_(\u0027Manage volume type invalid: %s\u0027) % reason","commit_id":"8f97995c69ab5d52d21f63cfc7379dda20d51303"},{"author":{"_account_id":16258,"name":"nikesh","email":"nike.niec@gmail.com","username":"nikem"},"change_message_id":"82635f431de923af9c742060f6ab35b3f98ae84d","unresolved":false,"context_lines":[{"line_number":1035,"context_line":"            vol_map \u003d False"},{"line_number":1036,"context_line":"            if self.client.search(\"mappings\", volume\u003dvol).total !\u003d 0:"},{"line_number":1037,"context_line":"                vol_map \u003d True"},{"line_number":1038,"context_line":"            if is_dedup !\u003d vg.is_dedup or vg_replica or vol_map or nvol !\u003d 1:"},{"line_number":1039,"context_line":"                raise exception.ManageExistingInvalidReference("},{"line_number":1040,"context_line":"                    existing_ref\u003dexisting_ref,"},{"line_number":1041,"context_line":"                    reason\u003d_(\u0027Manage volume type invalid.\u0027))"},{"line_number":1042,"context_line":"            vol.name \u003d new_name"},{"line_number":1043,"context_line":"            vg_name \u003d vg.name"},{"line_number":1044,"context_line":"            LOG.debug(\"Manage new volume name: %s\", new_name)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_5aac958c","line":1041,"range":{"start_line":1038,"start_character":11,"end_line":1041,"end_character":60},"in_reply_to":"3a71b18c_734fc133","updated":"2016-12-07 12:50:54.000000000","message":"Good suggestion but this might be not related to this change.\nSince we have to take care of the whole condition:\nif is_dedup !\u003d vg.is_dedup or vg_replica or vol_map or nvol !\u003d 1","commit_id":"8f97995c69ab5d52d21f63cfc7379dda20d51303"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"08cc94a404dac94af5b89f1ea201b14d5443b450","unresolved":false,"context_lines":[{"line_number":1065,"context_line":"                    LOG.debug(\"Updating vg new name to old name: %s \", vg_name)"},{"line_number":1066,"context_line":"                    vg.save()"},{"line_number":1067,"context_line":"            LOG.exception(_LE(\"manage volume: %s failed.\"), vol_name)"},{"line_number":1068,"context_line":"            if reason:"},{"line_number":1069,"context_line":"                raise exception.ManageExistingInvalidReference("},{"line_number":1070,"context_line":"                    existing_ref\u003dexisting_ref,"},{"line_number":1071,"context_line":"                    reason\u003d_(\u0027Unable to manage K2 volume due to: %s\u0027) % reason)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a71b18c_a5c9d3ae","line":1068,"range":{"start_line":1068,"start_character":12,"end_line":1068,"end_character":21},"updated":"2016-12-08 13:40:44.000000000","message":"I think it would be better to have:\n\nexcept exception.ManageExistingInvalidReference:\n    raise\nexcept Exception as ex:\n    ...\n\nThat would be cleaner than having this extra conditional logic inside the exception handling and creating a whole new exception.","commit_id":"9a45cf128b9021f487940d1cfb8d3fa5ed8f8ae1"},{"author":{"_account_id":14865,"name":"Adriano Freires Rosso","email":"adriano.rosso@fit-tecnologia.org.br","username":"adriano_rosso"},"change_message_id":"6e5ce8397f144256d55bd45ce690ee1fd5a33c68","unresolved":false,"context_lines":[{"line_number":1065,"context_line":"                    LOG.debug(\"Updating vg new name to old name: %s \", vg_name)"},{"line_number":1066,"context_line":"                    vg.save()"},{"line_number":1067,"context_line":"            LOG.exception(_LE(\"manage volume: %s failed.\"), vol_name)"},{"line_number":1068,"context_line":"            if reason:"},{"line_number":1069,"context_line":"                raise exception.ManageExistingInvalidReference("},{"line_number":1070,"context_line":"                    existing_ref\u003dexisting_ref,"},{"line_number":1071,"context_line":"                    reason\u003d_(\u0027Unable to manage K2 volume due to: %s\u0027) % reason)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a71b18c_3f0f8b89","line":1068,"range":{"start_line":1068,"start_character":12,"end_line":1068,"end_character":21},"updated":"2016-12-08 15:26:35.000000000","message":"I totally agree with Sean. Just raise the \"ManageExistingInvalidReference\" exception on L1059, and you can keep the other except code as it was before.","commit_id":"9a45cf128b9021f487940d1cfb8d3fa5ed8f8ae1"},{"author":{"_account_id":16258,"name":"nikesh","email":"nike.niec@gmail.com","username":"nikem"},"change_message_id":"6e62a46be13d6b1032bc7b4b61105027b898a1cf","unresolved":false,"context_lines":[{"line_number":1065,"context_line":"                    LOG.debug(\"Updating vg new name to old name: %s \", vg_name)"},{"line_number":1066,"context_line":"                    vg.save()"},{"line_number":1067,"context_line":"            LOG.exception(_LE(\"manage volume: %s failed.\"), vol_name)"},{"line_number":1068,"context_line":"            if reason:"},{"line_number":1069,"context_line":"                raise exception.ManageExistingInvalidReference("},{"line_number":1070,"context_line":"                    existing_ref\u003dexisting_ref,"},{"line_number":1071,"context_line":"                    reason\u003d_(\u0027Unable to manage K2 volume due to: %s\u0027) % reason)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a71b18c_2685bf58","line":1068,"range":{"start_line":1068,"start_character":12,"end_line":1068,"end_character":21},"in_reply_to":"3a71b18c_3f0f8b89","updated":"2016-12-08 17:42:20.000000000","message":"what about this\n\nhttp://paste.openstack.org/show/591838/","commit_id":"9a45cf128b9021f487940d1cfb8d3fa5ed8f8ae1"},{"author":{"_account_id":14865,"name":"Adriano Freires Rosso","email":"adriano.rosso@fit-tecnologia.org.br","username":"adriano_rosso"},"change_message_id":"0a90298c656658055c6b908b22a38252da204d2b","unresolved":false,"context_lines":[{"line_number":1065,"context_line":"                    LOG.debug(\"Updating vg new name to old name: %s \", vg_name)"},{"line_number":1066,"context_line":"                    vg.save()"},{"line_number":1067,"context_line":"            LOG.exception(_LE(\"manage volume: %s failed.\"), vol_name)"},{"line_number":1068,"context_line":"            raise"},{"line_number":1069,"context_line":""},{"line_number":1070,"context_line":"    @kaminario_logger"},{"line_number":1071,"context_line":"    def manage_existing_get_size(self, volume, existing_ref):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_993a7433","line":1068,"range":{"start_line":1068,"start_character":12,"end_line":1068,"end_character":17},"updated":"2016-12-08 18:25:27.000000000","message":"I\u0027m not sure this is going to work the way it\u0027s supposed to be. For example, what happens if you get the \"ManageExistingInvalidReference\" and then you get another exception in the other commands of your except block?\nI think this way you\u0027re going to raise only the last exception.","commit_id":"2244882189bafa4541d943aeeda1199d46a77182"},{"author":{"_account_id":16258,"name":"nikesh","email":"nike.niec@gmail.com","username":"nikem"},"change_message_id":"a37e49a4512f5806c842491cfdacc54506bd5e64","unresolved":false,"context_lines":[{"line_number":1065,"context_line":"                    LOG.debug(\"Updating vg new name to old name: %s \", vg_name)"},{"line_number":1066,"context_line":"                    vg.save()"},{"line_number":1067,"context_line":"            LOG.exception(_LE(\"manage volume: %s failed.\"), vol_name)"},{"line_number":1068,"context_line":"            raise"},{"line_number":1069,"context_line":""},{"line_number":1070,"context_line":"    @kaminario_logger"},{"line_number":1071,"context_line":"    def manage_existing_get_size(self, volume, existing_ref):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_5ac5183b","line":1068,"range":{"start_line":1068,"start_character":12,"end_line":1068,"end_character":17},"in_reply_to":"3a71b18c_993a7433","updated":"2016-12-09 07:17:04.000000000","message":"ok, I will move LOG.exception immediately after except so that it can report the exact issue first and then I will do some revert back and then finally use raise.","commit_id":"2244882189bafa4541d943aeeda1199d46a77182"},{"author":{"_account_id":14865,"name":"Adriano Freires Rosso","email":"adriano.rosso@fit-tecnologia.org.br","username":"adriano_rosso"},"change_message_id":"d88d7941915d933b10b70a241970edd781ddd3d1","unresolved":false,"context_lines":[{"line_number":1056,"context_line":"            vg.save()"},{"line_number":1057,"context_line":"            LOG.debug(\"Manage volume: %s in K2.\", vol_name)"},{"line_number":1058,"context_line":"            vol.save()"},{"line_number":1059,"context_line":"        except Exception:"},{"line_number":1060,"context_line":"            LOG.exception(_LE(\"manage volume: %s failed.\"), vol_name)"},{"line_number":1061,"context_line":"            vg_rs \u003d self.client.search(\"volume_groups\", name\u003dvg_new_name)"},{"line_number":1062,"context_line":"            if hasattr(vg_rs, \u0027hits\u0027) and vg_rs.total !\u003d 0:"},{"line_number":1063,"context_line":"                vg \u003d vg_rs.hits[0]"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a71b18c_d601f56b","line":1060,"range":{"start_line":1059,"start_character":8,"end_line":1060,"end_character":69},"updated":"2016-12-09 12:41:21.000000000","message":"I didn\u0027t get why you won\u0027t simply catch the ManageExistingInvalidReference exception here and raise it here. If you get this exception, you don\u0027t need to do any revert back, right?\nIt seems that the way you coded is going to work, but you\u0027ll be running some unnecessary code if you get the ManageExistingInvalidReference. Am I missing something?","commit_id":"324ba69f5fc26f5241b5821952a5b42e5c8bf210"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"785db8c04410e4672e8e4d4c8290bd69c390743b","unresolved":false,"context_lines":[{"line_number":1056,"context_line":"            vg.save()"},{"line_number":1057,"context_line":"            LOG.debug(\"Manage volume: %s in K2.\", vol_name)"},{"line_number":1058,"context_line":"            vol.save()"},{"line_number":1059,"context_line":"        except Exception:"},{"line_number":1060,"context_line":"            LOG.exception(_LE(\"manage volume: %s failed.\"), vol_name)"},{"line_number":1061,"context_line":"            vg_rs \u003d self.client.search(\"volume_groups\", name\u003dvg_new_name)"},{"line_number":1062,"context_line":"            if hasattr(vg_rs, \u0027hits\u0027) and vg_rs.total !\u003d 0:"},{"line_number":1063,"context_line":"                vg \u003d vg_rs.hits[0]"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a71b18c_f1afdbe0","line":1060,"range":{"start_line":1059,"start_character":8,"end_line":1060,"end_character":69},"in_reply_to":"3a71b18c_d601f56b","updated":"2016-12-09 12:52:35.000000000","message":"I agree. I still think simplifying this to just do what was suggested in patchset 6 would be better.","commit_id":"324ba69f5fc26f5241b5821952a5b42e5c8bf210"}]}