)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5e1840fee172fda58f68a56f66633d707919298d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"139c200d_2ebc2b3d","updated":"2024-05-20 19:42:32.000000000","message":"Thanks Gorka and Stephen for reviews","commit_id":"b52e708eb3afc71fcff4c2ab9f7dac77f103e0b7"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c6212d82db04e3d75e424ddb0ac585a306dc533d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"1079a44f_c130ac28","updated":"2024-05-21 05:19:12.000000000","message":"recheck POST_FAILURE on openstack-tox-docs","commit_id":"b52e708eb3afc71fcff4c2ab9f7dac77f103e0b7"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0925e1be92e5e81a1249763d1411c5071bafa28f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c4746143_7abe19fa","updated":"2024-06-10 15:11:28.000000000","message":"Thanks Gorka and Stephen","commit_id":"ccca3acf2c0507e683841a3fc3095e224c2cdd1b"}],"openstackclient/volume/v3/volume.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"281b576f8c268db2ddbdec0da8145af5b56ce64a","unresolved":true,"context_lines":[{"line_number":245,"context_line":"    def get_parser(self, prog_name):"},{"line_number":246,"context_line":"        parser \u003d super().get_parser(prog_name)"},{"line_number":247,"context_line":"        parser.add_argument("},{"line_number":248,"context_line":"            \u0027--remote\u0027,"},{"line_number":249,"context_line":"            action\u003d\u0027store_true\u0027,"},{"line_number":250,"context_line":"            help\u003d_(\"Specify this parameter to unmanage a volume.\"),"},{"line_number":251,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":2,"id":"ef21568e_380395d7","line":248,"updated":"2024-05-16 15:45:27.000000000","message":"I really don\u0027t like this whole `remote` concept, much less for deletion :-(","commit_id":"2469440c5833230de365d13d8c34f669279cabf8"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5e1840fee172fda58f68a56f66633d707919298d","unresolved":false,"context_lines":[{"line_number":245,"context_line":"    def get_parser(self, prog_name):"},{"line_number":246,"context_line":"        parser \u003d super().get_parser(prog_name)"},{"line_number":247,"context_line":"        parser.add_argument("},{"line_number":248,"context_line":"            \u0027--remote\u0027,"},{"line_number":249,"context_line":"            action\u003d\u0027store_true\u0027,"},{"line_number":250,"context_line":"            help\u003d_(\"Specify this parameter to unmanage a volume.\"),"},{"line_number":251,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":2,"id":"6ff81a33_0b75ea9a","line":248,"in_reply_to":"83bf0bbd_937a05bc","updated":"2024-05-20 19:42:32.000000000","message":"Me neither but that\u0027s the precedent we have followed and it would be really confusing to change it now.\nThe --remote option is mentioned in the OSC CLI docs[1] and is synonymous to the --remote-source used in manage command.\nThe first implementation was in snapshot-manage and I used the same terminology for volumes to be consistent.\nI think we can change everything at once in the future but for now I would like to keep it as it is as much as i also don\u0027t like it.\n\n[1] https://docs.openstack.org/python-openstackclient/latest/cli/decoder.html#cinder-cli","commit_id":"2469440c5833230de365d13d8c34f669279cabf8"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7df1f50a8a2d1e9417f3a5facedda0c87608f6cf","unresolved":true,"context_lines":[{"line_number":245,"context_line":"    def get_parser(self, prog_name):"},{"line_number":246,"context_line":"        parser \u003d super().get_parser(prog_name)"},{"line_number":247,"context_line":"        parser.add_argument("},{"line_number":248,"context_line":"            \u0027--remote\u0027,"},{"line_number":249,"context_line":"            action\u003d\u0027store_true\u0027,"},{"line_number":250,"context_line":"            help\u003d_(\"Specify this parameter to unmanage a volume.\"),"},{"line_number":251,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":2,"id":"83bf0bbd_937a05bc","line":248,"in_reply_to":"ef21568e_380395d7","updated":"2024-05-20 09:38:05.000000000","message":"Is there a better name we can use? I didn\u0027t come up with the terminology, but the idea that you are deleting the Cinder representation of $thing without deleting $thing itself makes some sense to me and maybe it\u0027s just the terminology that needs work?","commit_id":"2469440c5833230de365d13d8c34f669279cabf8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"281b576f8c268db2ddbdec0da8145af5b56ce64a","unresolved":true,"context_lines":[{"line_number":259,"context_line":"        for i in parsed_args.volumes:"},{"line_number":260,"context_line":"            try:"},{"line_number":261,"context_line":"                if parsed_args.remote:"},{"line_number":262,"context_line":"                    if parsed_args.force or parsed_args.purge:"},{"line_number":263,"context_line":"                        msg \u003d _("},{"line_number":264,"context_line":"                            \"The --force and --purge options are not \""},{"line_number":265,"context_line":"                            \"supported with the --remote parameter.\""},{"line_number":266,"context_line":"                        )"},{"line_number":267,"context_line":"                        raise exceptions.CommandError(msg)"},{"line_number":268,"context_line":"                    volume_client_sdk.unmanage_volume(i)"},{"line_number":269,"context_line":"                    continue"},{"line_number":270,"context_line":"                volume_obj \u003d utils.find_resource(volume_client.volumes, i)"}],"source_content_type":"text/x-python","patch_set":2,"id":"542f0cc0_ef962655","line":267,"range":{"start_line":262,"start_character":0,"end_line":267,"end_character":58},"updated":"2024-05-16 15:45:27.000000000","message":"-1: This check should happen out of the for loop","commit_id":"2469440c5833230de365d13d8c34f669279cabf8"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5e1840fee172fda58f68a56f66633d707919298d","unresolved":false,"context_lines":[{"line_number":259,"context_line":"        for i in parsed_args.volumes:"},{"line_number":260,"context_line":"            try:"},{"line_number":261,"context_line":"                if parsed_args.remote:"},{"line_number":262,"context_line":"                    if parsed_args.force or parsed_args.purge:"},{"line_number":263,"context_line":"                        msg \u003d _("},{"line_number":264,"context_line":"                            \"The --force and --purge options are not \""},{"line_number":265,"context_line":"                            \"supported with the --remote parameter.\""},{"line_number":266,"context_line":"                        )"},{"line_number":267,"context_line":"                        raise exceptions.CommandError(msg)"},{"line_number":268,"context_line":"                    volume_client_sdk.unmanage_volume(i)"},{"line_number":269,"context_line":"                    continue"},{"line_number":270,"context_line":"                volume_obj \u003d utils.find_resource(volume_client.volumes, i)"}],"source_content_type":"text/x-python","patch_set":2,"id":"81472519_b9b3eb45","line":267,"range":{"start_line":262,"start_character":0,"end_line":267,"end_character":58},"in_reply_to":"542f0cc0_ef962655","updated":"2024-05-20 19:42:32.000000000","message":"Thanks, makes sense to fail fast instead of looping it since the values won\u0027t change.","commit_id":"2469440c5833230de365d13d8c34f669279cabf8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"281b576f8c268db2ddbdec0da8145af5b56ce64a","unresolved":true,"context_lines":[{"line_number":265,"context_line":"                            \"supported with the --remote parameter.\""},{"line_number":266,"context_line":"                        )"},{"line_number":267,"context_line":"                        raise exceptions.CommandError(msg)"},{"line_number":268,"context_line":"                    volume_client_sdk.unmanage_volume(i)"},{"line_number":269,"context_line":"                    continue"},{"line_number":270,"context_line":"                volume_obj \u003d utils.find_resource(volume_client.volumes, i)"},{"line_number":271,"context_line":"                if parsed_args.force:"}],"source_content_type":"text/x-python","patch_set":2,"id":"cc291a1d_f47afac3","line":268,"updated":"2024-05-16 15:45:27.000000000","message":"?: Why are we using `find_resource` for the other deletion methods but not for this one?","commit_id":"2469440c5833230de365d13d8c34f669279cabf8"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5e1840fee172fda58f68a56f66633d707919298d","unresolved":false,"context_lines":[{"line_number":265,"context_line":"                            \"supported with the --remote parameter.\""},{"line_number":266,"context_line":"                        )"},{"line_number":267,"context_line":"                        raise exceptions.CommandError(msg)"},{"line_number":268,"context_line":"                    volume_client_sdk.unmanage_volume(i)"},{"line_number":269,"context_line":"                    continue"},{"line_number":270,"context_line":"                volume_obj \u003d utils.find_resource(volume_client.volumes, i)"},{"line_number":271,"context_line":"                if parsed_args.force:"}],"source_content_type":"text/x-python","patch_set":2,"id":"f7f87a2f_c6b42315","line":268,"in_reply_to":"cbd7e070_def52b32","updated":"2024-05-20 19:42:32.000000000","message":"So there\u0027s one issue with converting delete volume to SDK, it is missing support for cascade which i added here[1] but that patch hasn\u0027t merged yet.\nFor now I will keep the same utils.find_resource method common for unmanage and normal delete and in future, once the SDK support is there for cascade, i will convert this to only using SDK (instead of cinderclient).\n\n[1] https://review.opendev.org/c/openstack/openstacksdk/+/897929","commit_id":"2469440c5833230de365d13d8c34f669279cabf8"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7df1f50a8a2d1e9417f3a5facedda0c87608f6cf","unresolved":true,"context_lines":[{"line_number":265,"context_line":"                            \"supported with the --remote parameter.\""},{"line_number":266,"context_line":"                        )"},{"line_number":267,"context_line":"                        raise exceptions.CommandError(msg)"},{"line_number":268,"context_line":"                    volume_client_sdk.unmanage_volume(i)"},{"line_number":269,"context_line":"                    continue"},{"line_number":270,"context_line":"                volume_obj \u003d utils.find_resource(volume_client.volumes, i)"},{"line_number":271,"context_line":"                if parsed_args.force:"}],"source_content_type":"text/x-python","patch_set":2,"id":"cbd7e070_def52b32","line":268,"in_reply_to":"cc291a1d_f47afac3","updated":"2024-05-20 09:38:05.000000000","message":"IMO it\u0027d be great to convert this method to SDK prior to doing this work too. That was a bigger ask for `volume create` since it\u0027s a much larger method, but there\u0027s only 3 (which will become 2 - `delete_volume` takes a `force` argument) calls here.","commit_id":"2469440c5833230de365d13d8c34f669279cabf8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"2ece520117500804f9477217136eec6167c4ca56","unresolved":true,"context_lines":[{"line_number":268,"context_line":"                volume_obj \u003d utils.find_resource(volume_client.volumes, i)"},{"line_number":269,"context_line":"                if parsed_args.remote:"},{"line_number":270,"context_line":"                    volume_client_sdk.unmanage_volume(volume_obj.id)"},{"line_number":271,"context_line":"                    continue"},{"line_number":272,"context_line":"                if parsed_args.force:"},{"line_number":273,"context_line":"                    volume_client.volumes.force_delete(volume_obj.id)"},{"line_number":274,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"da0c58bb_2b8a9304","line":271,"updated":"2024-05-30 09:58:30.000000000","message":"nit: It\u0027s not great that we use `continue` here but use `else` onn L247.  It\u0027s usually best to use the same approach in all cases, so either do an `if ... elif ... else` or `if ... continue if ... continue`","commit_id":"b52e708eb3afc71fcff4c2ab9f7dac77f103e0b7"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0925e1be92e5e81a1249763d1411c5071bafa28f","unresolved":false,"context_lines":[{"line_number":268,"context_line":"                volume_obj \u003d utils.find_resource(volume_client.volumes, i)"},{"line_number":269,"context_line":"                if parsed_args.remote:"},{"line_number":270,"context_line":"                    volume_client_sdk.unmanage_volume(volume_obj.id)"},{"line_number":271,"context_line":"                    continue"},{"line_number":272,"context_line":"                if parsed_args.force:"},{"line_number":273,"context_line":"                    volume_client.volumes.force_delete(volume_obj.id)"},{"line_number":274,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"980232cd_0270866f","line":271,"in_reply_to":"da0c58bb_2b8a9304","updated":"2024-06-10 15:11:28.000000000","message":"Done","commit_id":"b52e708eb3afc71fcff4c2ab9f7dac77f103e0b7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b65b7b20798aee1b972cd20fbb20067e0af0eb91","unresolved":true,"context_lines":[{"line_number":282,"context_line":"                        \"Failed to delete volume with \""},{"line_number":283,"context_line":"                        \"name or ID \u0027%(volume)s\u0027: %(e)s\""},{"line_number":284,"context_line":"                    )"},{"line_number":285,"context_line":"                    % {\u0027volume\u0027: i, \u0027e\u0027: e},"},{"line_number":286,"context_line":"                )"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"        if result \u003e 0:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7085d1c4_d96dff45","line":285,"updated":"2024-06-10 14:53:25.000000000","message":"Don\u0027t we _want_ deferred logging?","commit_id":"b52e708eb3afc71fcff4c2ab9f7dac77f103e0b7"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0925e1be92e5e81a1249763d1411c5071bafa28f","unresolved":false,"context_lines":[{"line_number":282,"context_line":"                        \"Failed to delete volume with \""},{"line_number":283,"context_line":"                        \"name or ID \u0027%(volume)s\u0027: %(e)s\""},{"line_number":284,"context_line":"                    )"},{"line_number":285,"context_line":"                    % {\u0027volume\u0027: i, \u0027e\u0027: e},"},{"line_number":286,"context_line":"                )"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"        if result \u003e 0:"}],"source_content_type":"text/x-python","patch_set":3,"id":"76f4d007_26abb6a9","line":285,"in_reply_to":"7085d1c4_d96dff45","updated":"2024-06-10 15:11:28.000000000","message":"Done","commit_id":"b52e708eb3afc71fcff4c2ab9f7dac77f103e0b7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d407ab3fc2eec9acc47d26eaf0defc26d911b2c9","unresolved":true,"context_lines":[{"line_number":268,"context_line":"                volume_obj \u003d utils.find_resource(volume_client.volumes, i)"},{"line_number":269,"context_line":"                if parsed_args.remote:"},{"line_number":270,"context_line":"                    volume_client_sdk.unmanage_volume(volume_obj.id)"},{"line_number":271,"context_line":"                else:"},{"line_number":272,"context_line":"                    if parsed_args.force:"},{"line_number":273,"context_line":"                        volume_client.volumes.force_delete(volume_obj.id)"},{"line_number":274,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"0ed9a036_ea5fce4f","line":271,"updated":"2024-06-10 15:13:11.000000000","message":"nit:\n\n```\nif parsed_args.remote:\n    ...\nelif parsed_args.force:\n    ...\nelse:\n    ...\n```","commit_id":"ccca3acf2c0507e683841a3fc3095e224c2cdd1b"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"54a8d46cc4a5da2f81e5078e92a295f28226f4cf","unresolved":true,"context_lines":[{"line_number":268,"context_line":"                volume_obj \u003d utils.find_resource(volume_client.volumes, i)"},{"line_number":269,"context_line":"                if parsed_args.remote:"},{"line_number":270,"context_line":"                    volume_client_sdk.unmanage_volume(volume_obj.id)"},{"line_number":271,"context_line":"                else:"},{"line_number":272,"context_line":"                    if parsed_args.force:"},{"line_number":273,"context_line":"                        volume_client.volumes.force_delete(volume_obj.id)"},{"line_number":274,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"6b3dc2c4_3ec2c7fb","line":271,"in_reply_to":"0ed9a036_ea5fce4f","updated":"2024-06-10 15:15:19.000000000","message":"Ah, i thought both the cases were calling same methods, so we have a different SDK method for force instead of an arg ... will update this","commit_id":"ccca3acf2c0507e683841a3fc3095e224c2cdd1b"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"476aa0a96ef729c85c98a50085e454e8191bb7e1","unresolved":false,"context_lines":[{"line_number":268,"context_line":"                volume_obj \u003d utils.find_resource(volume_client.volumes, i)"},{"line_number":269,"context_line":"                if parsed_args.remote:"},{"line_number":270,"context_line":"                    volume_client_sdk.unmanage_volume(volume_obj.id)"},{"line_number":271,"context_line":"                else:"},{"line_number":272,"context_line":"                    if parsed_args.force:"},{"line_number":273,"context_line":"                        volume_client.volumes.force_delete(volume_obj.id)"},{"line_number":274,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"579a249d_71fac266","line":271,"in_reply_to":"6b3dc2c4_3ec2c7fb","updated":"2024-06-10 15:19:02.000000000","message":"Done","commit_id":"ccca3acf2c0507e683841a3fc3095e224c2cdd1b"}]}