)]}'
{"openstackclient/compute/v2/server.py":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"ff9116b1f0c3f99a3d0e938af26f57f5af2d1d7e","unresolved":true,"context_lines":[{"line_number":832,"context_line":"            action\u003dparseractions.MultiKeyValueAction,"},{"line_number":833,"context_line":"            dest\u003d\u0027block_devices\u0027,"},{"line_number":834,"context_line":"            default\u003d[],"},{"line_number":835,"context_line":"            required_keys\u003d[],"},{"line_number":836,"context_line":"            optional_keys\u003d["},{"line_number":837,"context_line":"                \u0027id\u0027, \u0027source\u0027, \u0027dest\u0027, \u0027bus\u0027, \u0027type\u0027, \u0027device\u0027, \u0027size\u0027,"},{"line_number":838,"context_line":"                \u0027format\u0027, \u0027bootindex\u0027, \u0027shutdown\u0027, \u0027tag\u0027, \u0027volume_type\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"71d32a05_6b258675","line":835,"range":{"start_line":835,"start_character":12,"end_line":835,"end_character":29},"updated":"2021-01-21 19:02:35.000000000","message":"FWIW boot_index is marked as required in the API docs.","commit_id":"0eb530ce80f8097cc1090347bf75248f49622801"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"46c755d6ba3312b145622e4e83e73d394129f38d","unresolved":false,"context_lines":[{"line_number":832,"context_line":"            action\u003dparseractions.MultiKeyValueAction,"},{"line_number":833,"context_line":"            dest\u003d\u0027block_devices\u0027,"},{"line_number":834,"context_line":"            default\u003d[],"},{"line_number":835,"context_line":"            required_keys\u003d[],"},{"line_number":836,"context_line":"            optional_keys\u003d["},{"line_number":837,"context_line":"                \u0027id\u0027, \u0027source\u0027, \u0027dest\u0027, \u0027bus\u0027, \u0027type\u0027, \u0027device\u0027, \u0027size\u0027,"},{"line_number":838,"context_line":"                \u0027format\u0027, \u0027bootindex\u0027, \u0027shutdown\u0027, \u0027tag\u0027, \u0027volume_type\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"22a7c977_e0429ca5","line":835,"range":{"start_line":835,"start_character":12,"end_line":835,"end_character":29},"in_reply_to":"71d32a05_6b258675","updated":"2021-01-22 13:01:17.000000000","message":"Done","commit_id":"0eb530ce80f8097cc1090347bf75248f49622801"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"ff9116b1f0c3f99a3d0e938af26f57f5af2d1d7e","unresolved":true,"context_lines":[{"line_number":1327,"context_line":"            # wants"},{"line_number":1328,"context_line":"            mapping \u003d {"},{"line_number":1329,"context_line":"                field: mapping[alias] for alias, field in {"},{"line_number":1330,"context_line":"                    \u0027id\u0027: \u0027uuid\u0027,"},{"line_number":1331,"context_line":"                    \u0027source\u0027: \u0027source_type\u0027,"},{"line_number":1332,"context_line":"                    \u0027dest\u0027: \u0027destination_type\u0027,"},{"line_number":1333,"context_line":"                    \u0027bus\u0027: \u0027disk_bus\u0027,"},{"line_number":1334,"context_line":"                    \u0027device\u0027: \u0027device_name\u0027,"},{"line_number":1335,"context_line":"                    \u0027size\u0027: \u0027volume_size\u0027,"},{"line_number":1336,"context_line":"                    \u0027format\u0027: \u0027guest_format\u0027,"},{"line_number":1337,"context_line":"                    \u0027bootindex\u0027: \u0027boot_index\u0027,"},{"line_number":1338,"context_line":"                    \u0027type\u0027: \u0027device_type\u0027,"},{"line_number":1339,"context_line":"                    \u0027shutdown\u0027: \u0027delete_on_termination\u0027,"},{"line_number":1340,"context_line":"                    \u0027tag\u0027: \u0027tag\u0027,"},{"line_number":1341,"context_line":"                    \u0027volume_type\u0027: \u0027volume_type\u0027,"},{"line_number":1342,"context_line":"                }.items() if alias in mapping"},{"line_number":1343,"context_line":"            }"},{"line_number":1344,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"2fa1d091_76bc43e6","line":1341,"range":{"start_line":1330,"start_character":1,"end_line":1341,"end_character":49},"updated":"2021-01-21 19:02:35.000000000","message":"I appreciate that the BDMv2 format is awful but I\u0027m not sure I agree with this, some of these changes are just confusing IMHO like device_type -\u003e type while volume_type remains the same.\n\nAs awful as it is I\u0027d stay with the original keys, hell it might even force someone to come up with a sane v3 format one day.","commit_id":"0eb530ce80f8097cc1090347bf75248f49622801"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"46c755d6ba3312b145622e4e83e73d394129f38d","unresolved":true,"context_lines":[{"line_number":1327,"context_line":"            # wants"},{"line_number":1328,"context_line":"            mapping \u003d {"},{"line_number":1329,"context_line":"                field: mapping[alias] for alias, field in {"},{"line_number":1330,"context_line":"                    \u0027id\u0027: \u0027uuid\u0027,"},{"line_number":1331,"context_line":"                    \u0027source\u0027: \u0027source_type\u0027,"},{"line_number":1332,"context_line":"                    \u0027dest\u0027: \u0027destination_type\u0027,"},{"line_number":1333,"context_line":"                    \u0027bus\u0027: \u0027disk_bus\u0027,"},{"line_number":1334,"context_line":"                    \u0027device\u0027: \u0027device_name\u0027,"},{"line_number":1335,"context_line":"                    \u0027size\u0027: \u0027volume_size\u0027,"},{"line_number":1336,"context_line":"                    \u0027format\u0027: \u0027guest_format\u0027,"},{"line_number":1337,"context_line":"                    \u0027bootindex\u0027: \u0027boot_index\u0027,"},{"line_number":1338,"context_line":"                    \u0027type\u0027: \u0027device_type\u0027,"},{"line_number":1339,"context_line":"                    \u0027shutdown\u0027: \u0027delete_on_termination\u0027,"},{"line_number":1340,"context_line":"                    \u0027tag\u0027: \u0027tag\u0027,"},{"line_number":1341,"context_line":"                    \u0027volume_type\u0027: \u0027volume_type\u0027,"},{"line_number":1342,"context_line":"                }.items() if alias in mapping"},{"line_number":1343,"context_line":"            }"},{"line_number":1344,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5507b043_6a1d0b01","line":1341,"range":{"start_line":1330,"start_character":1,"end_line":1341,"end_character":49},"in_reply_to":"2fa1d091_76bc43e6","updated":"2021-01-22 13:01:17.000000000","message":"Okay, cool. FYI it\u0027s copied from novaclient, and I was on the fence about this too. I also agree that naming of the different \u0027*_type\u0027 fields is poor. The one \"feature\" I did think useful was the \u0027id\u0027 field that allowed users to specify a name in addition to an ID. When every other field is a direct mapping though, this feels more rather than less confusing. As you\u0027ve said elsewhere, if users want this then they can rely this functionality provided by the other parameters (--volume, --snapshot, --image + --boot-from-volume) to make their lives easier.","commit_id":"0eb530ce80f8097cc1090347bf75248f49622801"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"ff9116b1f0c3f99a3d0e938af26f57f5af2d1d7e","unresolved":true,"context_lines":[{"line_number":1351,"context_line":"                )"},{"line_number":1352,"context_line":"                raise exceptions.CommandError(msg)"},{"line_number":1353,"context_line":""},{"line_number":1354,"context_line":"            if \u0027volume_type\u0027 in mapping and ("},{"line_number":1355,"context_line":"                compute_client.api_version \u003c api_versions.APIVersion(\u00272.67\u0027)"},{"line_number":1356,"context_line":"            ):"},{"line_number":1357,"context_line":"                msg \u003d _("},{"line_number":1358,"context_line":"                    \u0027--os-compute-api-version 2.67 or greater is \u0027"},{"line_number":1359,"context_line":"                    \u0027required to support the volume_type key of --block-device\u0027"},{"line_number":1360,"context_line":"                )"},{"line_number":1361,"context_line":"                raise exceptions.CommandError(msg)"},{"line_number":1362,"context_line":""},{"line_number":1363,"context_line":"            if \u0027source_type\u0027 in mapping:"},{"line_number":1364,"context_line":"                if mapping[\u0027source_type\u0027] not in ("}],"source_content_type":"text/x-python","patch_set":2,"id":"2d0e4ec6_fafd57cc","line":1361,"range":{"start_line":1354,"start_character":0,"end_line":1361,"end_character":50},"updated":"2021-01-21 19:02:35.000000000","message":"This is written after the below comment but to prove my point we would also need to add input validation for the following here in regard to a callers use of volume_type:\n\nIt can be a volume type ID or name.\n\nIt is only supported with source_type of blank, image or snapshot.\n\nIt is only supported with destination_type of volume.","commit_id":"0eb530ce80f8097cc1090347bf75248f49622801"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"ff9116b1f0c3f99a3d0e938af26f57f5af2d1d7e","unresolved":true,"context_lines":[{"line_number":1360,"context_line":"                )"},{"line_number":1361,"context_line":"                raise exceptions.CommandError(msg)"},{"line_number":1362,"context_line":""},{"line_number":1363,"context_line":"            if \u0027source_type\u0027 in mapping:"},{"line_number":1364,"context_line":"                if mapping[\u0027source_type\u0027] not in ("},{"line_number":1365,"context_line":"                    \u0027volume\u0027, \u0027image\u0027, \u0027snapshot\u0027, \u0027blank\u0027,"},{"line_number":1366,"context_line":"                ):"},{"line_number":1367,"context_line":"                    msg \u003d _("},{"line_number":1368,"context_line":"                        \u0027The source key of --block-device should be one \u0027"},{"line_number":1369,"context_line":"                        \u0027of: volume, image, snapshot, blank\u0027"},{"line_number":1370,"context_line":"                    )"},{"line_number":1371,"context_line":"                    raise exceptions.CommandError(msg)"},{"line_number":1372,"context_line":"            else:"},{"line_number":1373,"context_line":"                mapping[\u0027source_type\u0027] \u003d \u0027blank\u0027"},{"line_number":1374,"context_line":""},{"line_number":1375,"context_line":"            if \u0027destination_type\u0027 in mapping:"},{"line_number":1376,"context_line":"                if mapping[\u0027destination_type\u0027] not in (\u0027local\u0027, \u0027volume\u0027):"}],"source_content_type":"text/x-python","patch_set":2,"id":"8d4f8712_a82d0a01","line":1373,"range":{"start_line":1363,"start_character":0,"end_line":1373,"end_character":48},"updated":"2021-01-21 19:02:35.000000000","message":"So should osc do this input validation or let n-api fail the request?\n\nI\u0027m asking as I\u0027m find with the microversion checking but input validation of these fields is going to end up duplicating a whole load of logic from n-api when we could handle and translate n-api responses instead.","commit_id":"0eb530ce80f8097cc1090347bf75248f49622801"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"46c755d6ba3312b145622e4e83e73d394129f38d","unresolved":false,"context_lines":[{"line_number":1360,"context_line":"                )"},{"line_number":1361,"context_line":"                raise exceptions.CommandError(msg)"},{"line_number":1362,"context_line":""},{"line_number":1363,"context_line":"            if \u0027source_type\u0027 in mapping:"},{"line_number":1364,"context_line":"                if mapping[\u0027source_type\u0027] not in ("},{"line_number":1365,"context_line":"                    \u0027volume\u0027, \u0027image\u0027, \u0027snapshot\u0027, \u0027blank\u0027,"},{"line_number":1366,"context_line":"                ):"},{"line_number":1367,"context_line":"                    msg \u003d _("},{"line_number":1368,"context_line":"                        \u0027The source key of --block-device should be one \u0027"},{"line_number":1369,"context_line":"                        \u0027of: volume, image, snapshot, blank\u0027"},{"line_number":1370,"context_line":"                    )"},{"line_number":1371,"context_line":"                    raise exceptions.CommandError(msg)"},{"line_number":1372,"context_line":"            else:"},{"line_number":1373,"context_line":"                mapping[\u0027source_type\u0027] \u003d \u0027blank\u0027"},{"line_number":1374,"context_line":""},{"line_number":1375,"context_line":"            if \u0027destination_type\u0027 in mapping:"},{"line_number":1376,"context_line":"                if mapping[\u0027destination_type\u0027] not in (\u0027local\u0027, \u0027volume\u0027):"}],"source_content_type":"text/x-python","patch_set":2,"id":"bb268f58_8700be5f","line":1373,"range":{"start_line":1363,"start_character":0,"end_line":1373,"end_character":48},"in_reply_to":"8d4f8712_a82d0a01","updated":"2021-01-22 13:01:17.000000000","message":"That\u0027s always an issue with client vs server side validation. I\u0027m in favour of limited duplication of validation where it saves the user some heartache (see e.g. \u0027--status\u0027 flag for \u0027server list\u0027 command). Given this is a very limited enum value, I think this kind of validation makes sense. Ditto for version checks. Something beyond than these checks, such as checking for inconsistencies between different fields, is very much a nice to have and probably isn\u0027t worth the maintenance effort/test matrix. Let me know if you disagree.","commit_id":"0eb530ce80f8097cc1090347bf75248f49622801"}]}