)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a7e591ed24b3791716fc6e8d2162bc3b7e37a22c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"73ae8520_3b4cb921","updated":"2022-10-11 17:07:01.000000000","message":"Good start. Don\u0027t forget to run this with multiple different microversions when testing locally. For example:\n\n  OS_COMPUTE_API_VERSION\u003d2.1 openstack hypervisor list\n  OS_COMPUTE_API_VERSION\u003d2.88 openstack hypervisor list\n  OS_COMPUTE_API_VERSION\u003d2.1 openstack hypervisor show {hypervisor} -f yaml\n  OS_COMPUTE_API_VERSION\u003d2.88 openstack hypervisor show {hypervisor} -f yaml\n\nI tested locally and the only thing I\u0027m seeing is some additional output that probably shouldn\u0027t be there. I\u0027ve included an idea inline for avoiding that but please feel free to do whatever you see fit. We could also do with a small release note, assuming we normally include one for these things (I don\u0027t recall)","commit_id":"39400000c409d0eb482d4a25f9f53eb9cd37f63a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a827707695d6b42671f12b87b8f176e31b371f58","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"66a95422_f421221e","updated":"2022-10-26 09:27:10.000000000","message":"I still think I\u0027d rather specify fields manually, but this does what it says on the tin and is otherwise A+. Nice work again","commit_id":"c367590824876462f3352fb7123c8e6b30056d65"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"9106992e8b3be754631b9b0ec86f4a16eb318356","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"695be052_19609ddc","updated":"2022-11-16 08:30:00.000000000","message":"Let\u0027s go for it to make it easier to bring remaining parts of parallel change","commit_id":"006e35509d3cea66dc13fe2238dac92f47fe3c5c"}],"openstackclient/compute/v2/hypervisor.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a7e591ed24b3791716fc6e8d2162bc3b7e37a22c","unresolved":true,"context_lines":[{"line_number":39,"context_line":"        \u0027memory_size\u0027: \u0027memory_mb\u0027,"},{"line_number":40,"context_line":"        \u0027name\u0027: \u0027hypervisor_hostname\u0027"},{"line_number":41,"context_line":"    }"},{"line_number":42,"context_line":"    hidden_columns \u003d [\u0027location\u0027, \u0027servers\u0027]"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    return utils.get_osc_show_columns_for_sdk_resource("},{"line_number":45,"context_line":"        item, column_map, hidden_columns)"}],"source_content_type":"text/x-python","patch_set":1,"id":"68b7454a_7f7a1433","line":42,"updated":"2022-10-11 17:07:01.000000000","message":"You need to make this version dependent. As you\u0027ve no doubt noted, some of these fields have changed type or disappeared in subsequent microversions. There are comments about this in the openstacksdk code [1] but you can also see it in the nova server code [2]. Personally, rather than doing this, I would build a whole new dict based on the hypervisor object. More on this below.\n\n[1] https://github.com/openstack/openstacksdk/blob/master/openstack/compute/v2/hypervisor.py\n[2] https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/hypervisors.py","commit_id":"39400000c409d0eb482d4a25f9f53eb9cd37f63a"},{"author":{"_account_id":34327,"name":"Violet Kurtz","email":"vikurtz@osuosl.org","username":"OniLink"},"change_message_id":"4c558a3d343a8652b64f58a8923a2458af349203","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        \u0027memory_size\u0027: \u0027memory_mb\u0027,"},{"line_number":40,"context_line":"        \u0027name\u0027: \u0027hypervisor_hostname\u0027"},{"line_number":41,"context_line":"    }"},{"line_number":42,"context_line":"    hidden_columns \u003d [\u0027location\u0027, \u0027servers\u0027]"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    return utils.get_osc_show_columns_for_sdk_resource("},{"line_number":45,"context_line":"        item, column_map, hidden_columns)"}],"source_content_type":"text/x-python","patch_set":1,"id":"84d78229_67e606f5","line":42,"in_reply_to":"68b7454a_7f7a1433","updated":"2022-10-25 18:40:16.000000000","message":"Done","commit_id":"39400000c409d0eb482d4a25f9f53eb9cd37f63a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a7e591ed24b3791716fc6e8d2162bc3b7e37a22c","unresolved":true,"context_lines":[{"line_number":170,"context_line":"            parsed_args.hypervisor, ignore_missing\u003dFalse).copy()"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        aggregates \u003d compute_client.aggregates()"},{"line_number":173,"context_line":"        hypervisor[\u0027aggregates\u0027] \u003d list()"},{"line_number":174,"context_line":"        service_details \u003d hypervisor[\u0027service_details\u0027]"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"        if aggregates:"}],"source_content_type":"text/x-python","patch_set":1,"id":"29da7292_ba62a819","line":173,"updated":"2022-10-11 17:07:01.000000000","message":"Random question: do you know what this is doing? 😉 If not, it might be a good opportunity to add some comments explaining this weird logic. That is assuming you don\u0027t like my alternative idea below.","commit_id":"39400000c409d0eb482d4a25f9f53eb9cd37f63a"},{"author":{"_account_id":34327,"name":"Violet Kurtz","email":"vikurtz@osuosl.org","username":"OniLink"},"change_message_id":"4c558a3d343a8652b64f58a8923a2458af349203","unresolved":false,"context_lines":[{"line_number":170,"context_line":"            parsed_args.hypervisor, ignore_missing\u003dFalse).copy()"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        aggregates \u003d compute_client.aggregates()"},{"line_number":173,"context_line":"        hypervisor[\u0027aggregates\u0027] \u003d list()"},{"line_number":174,"context_line":"        service_details \u003d hypervisor[\u0027service_details\u0027]"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"        if aggregates:"}],"source_content_type":"text/x-python","patch_set":1,"id":"cbb732a3_2ab6b473","line":173,"in_reply_to":"29da7292_ba62a819","updated":"2022-10-25 18:40:16.000000000","message":"Ack","commit_id":"39400000c409d0eb482d4a25f9f53eb9cd37f63a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a7e591ed24b3791716fc6e8d2162bc3b7e37a22c","unresolved":true,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        aggregates \u003d compute_client.aggregates()"},{"line_number":173,"context_line":"        hypervisor[\u0027aggregates\u0027] \u003d list()"},{"line_number":174,"context_line":"        service_details \u003d hypervisor[\u0027service_details\u0027]"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"        if aggregates:"},{"line_number":177,"context_line":"            # Hypervisors in nova cells are prefixed by \"\u003ccell\u003e@\""}],"source_content_type":"text/x-python","patch_set":1,"id":"a158ff98_fea4f797","line":174,"updated":"2022-10-11 17:07:01.000000000","message":"nit: any particular reason to introduce this variable rather than continuing to access the dict directly as before?","commit_id":"39400000c409d0eb482d4a25f9f53eb9cd37f63a"},{"author":{"_account_id":34327,"name":"Violet Kurtz","email":"vikurtz@osuosl.org","username":"OniLink"},"change_message_id":"4c558a3d343a8652b64f58a8923a2458af349203","unresolved":true,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        aggregates \u003d compute_client.aggregates()"},{"line_number":173,"context_line":"        hypervisor[\u0027aggregates\u0027] \u003d list()"},{"line_number":174,"context_line":"        service_details \u003d hypervisor[\u0027service_details\u0027]"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"        if aggregates:"},{"line_number":177,"context_line":"            # Hypervisors in nova cells are prefixed by \"\u003ccell\u003e@\""}],"source_content_type":"text/x-python","patch_set":1,"id":"af3d6a19_4d700780","line":174,"in_reply_to":"a158ff98_fea4f797","updated":"2022-10-25 18:40:16.000000000","message":"I mostly did it to help shorten a few lines that grew to be too long, causing tox to complain. Couldn\u0027t really find a better, consistent way to reduce line length. Very possible I simply missed a better idea, but I can document that this is just creating a shortcut if desired.","commit_id":"39400000c409d0eb482d4a25f9f53eb9cd37f63a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a7e591ed24b3791716fc6e8d2162bc3b7e37a22c","unresolved":true,"context_lines":[{"line_number":195,"context_line":""},{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            if sdk_utils.supports_microversion(compute_client, \u00272.88\u0027) and \\"},{"line_number":198,"context_line":"                    hypervisor[\u0027uptime\u0027]:"},{"line_number":199,"context_line":"                uptime \u003d hypervisor[\u0027uptime\u0027]"},{"line_number":200,"context_line":"                del hypervisor[\u0027uptime\u0027]"},{"line_number":201,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"280b4601_4d20a8d1","line":198,"updated":"2022-10-11 17:07:01.000000000","message":"I don\u0027t think you need truth\u0027y check for hypervisor[\u0027uptime\u0027]. Even if that\u0027s not present, you don\u0027t want to make a call to the legacy API - it will fail [1]. Consider dropping this in favour of e.g.\n\n  uptime \u003d hypervisor[\u0027uptime\u0027] or \u0027\u0027\n\n[1] https://github.com/openstack/openstacksdk/blob/0.101.0/openstack/compute/v2/hypervisor.py#L90-L92","commit_id":"39400000c409d0eb482d4a25f9f53eb9cd37f63a"},{"author":{"_account_id":34327,"name":"Violet Kurtz","email":"vikurtz@osuosl.org","username":"OniLink"},"change_message_id":"4c558a3d343a8652b64f58a8923a2458af349203","unresolved":false,"context_lines":[{"line_number":195,"context_line":""},{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            if sdk_utils.supports_microversion(compute_client, \u00272.88\u0027) and \\"},{"line_number":198,"context_line":"                    hypervisor[\u0027uptime\u0027]:"},{"line_number":199,"context_line":"                uptime \u003d hypervisor[\u0027uptime\u0027]"},{"line_number":200,"context_line":"                del hypervisor[\u0027uptime\u0027]"},{"line_number":201,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"f41bf94a_d630c8ea","line":198,"in_reply_to":"280b4601_4d20a8d1","updated":"2022-10-25 18:40:16.000000000","message":"Done","commit_id":"39400000c409d0eb482d4a25f9f53eb9cd37f63a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a7e591ed24b3791716fc6e8d2162bc3b7e37a22c","unresolved":true,"context_lines":[{"line_number":228,"context_line":"                hypervisor[\u0027cpu_info\u0027] \u003d json.loads(hypervisor[\u0027cpu_info\u0027])"},{"line_number":229,"context_line":"            else:"},{"line_number":230,"context_line":"                hypervisor[\u0027cpu_info\u0027] \u003d {}"},{"line_number":231,"context_line":"        display_columns, columns \u003d _get_hypervisor_columns(hypervisor)"},{"line_number":232,"context_line":"        data \u003d utils.get_dict_properties("},{"line_number":233,"context_line":"            hypervisor, columns,"},{"line_number":234,"context_line":"            formatters\u003d{"}],"source_content_type":"text/x-python","patch_set":1,"id":"cad189c5_f3b57876","line":231,"updated":"2022-10-11 17:07:01.000000000","message":"So rather than doing this, how about we just build a new dictionary containing properties of the object? We can do this based on the API microversion present. For example:\n\n  data \u003d {\n      \u0027id\u0027: hypervisor.id,\n      \u0027host_ip\u0027: hypervisor.host_ip,\n      ...\n  }\n\n  if aggregates:\n      ...\n\n  if sdk_utils.supports_microversion(compute_client, \u00272.28\u0027):\n      data[\u0027cpu_info\u0027] \u003d hypervisor.cpu_info\n  else:\n      # microversion 2.28 transformed this to a JSON blob rather than a\n      # string; on earlier fields, do this manually\n      data[\u0027cpu_info\u0027] \u003d json.loads(hypervisor.cpu_info or \u0027{}\u0027)\n  \n  if sdk_utils.supports_microversion(compute_client, \u00272.88\u0027):\n      uptime \u003d hypervisor.uptime\n  else:\n      uptime \u003d compute_client.get_hypervisor_uptime(hypervisor.id).uptime\n\n  if not sdk_utils.supports_microversion(compute_client, \u00272.88\u0027):\n      data[\u0027current_workload\u0027] \u003d hypervisor.current_workload\n      data[\u0027disk_available_least\u0027] \u003d hypervisor.disk_available_least\n      ...\n\nYou can then continue to use \u0027utils.get_dict_properties\u0027 as before.\n\nThoughts?","commit_id":"39400000c409d0eb482d4a25f9f53eb9cd37f63a"},{"author":{"_account_id":34327,"name":"Violet Kurtz","email":"vikurtz@osuosl.org","username":"OniLink"},"change_message_id":"faca56cd21a8e5fd4eed6b4d2ba312ac30d8a182","unresolved":true,"context_lines":[{"line_number":228,"context_line":"                hypervisor[\u0027cpu_info\u0027] \u003d json.loads(hypervisor[\u0027cpu_info\u0027])"},{"line_number":229,"context_line":"            else:"},{"line_number":230,"context_line":"                hypervisor[\u0027cpu_info\u0027] \u003d {}"},{"line_number":231,"context_line":"        display_columns, columns \u003d _get_hypervisor_columns(hypervisor)"},{"line_number":232,"context_line":"        data \u003d utils.get_dict_properties("},{"line_number":233,"context_line":"            hypervisor, columns,"},{"line_number":234,"context_line":"            formatters\u003d{"}],"source_content_type":"text/x-python","patch_set":1,"id":"f9466cfc_3b00f494","line":231,"in_reply_to":"a16fb38e_f3e067c7","updated":"2022-11-02 23:27:10.000000000","message":"Fair enough. I can go ahead and make the changes before we finalize this so that the fields are all specified explicitly if you\u0027d like.","commit_id":"39400000c409d0eb482d4a25f9f53eb9cd37f63a"},{"author":{"_account_id":34327,"name":"Violet Kurtz","email":"vikurtz@osuosl.org","username":"OniLink"},"change_message_id":"4c558a3d343a8652b64f58a8923a2458af349203","unresolved":true,"context_lines":[{"line_number":228,"context_line":"                hypervisor[\u0027cpu_info\u0027] \u003d json.loads(hypervisor[\u0027cpu_info\u0027])"},{"line_number":229,"context_line":"            else:"},{"line_number":230,"context_line":"                hypervisor[\u0027cpu_info\u0027] \u003d {}"},{"line_number":231,"context_line":"        display_columns, columns \u003d _get_hypervisor_columns(hypervisor)"},{"line_number":232,"context_line":"        data \u003d utils.get_dict_properties("},{"line_number":233,"context_line":"            hypervisor, columns,"},{"line_number":234,"context_line":"            formatters\u003d{"}],"source_content_type":"text/x-python","patch_set":1,"id":"cd0c2222_8e65c871","line":231,"in_reply_to":"cad189c5_f3b57876","updated":"2022-10-25 18:40:16.000000000","message":"The idea makes sense, but an issue I see with it is if the returned structure happens to change and begins including new properties that we want to have displayed, then every time such a change occurs we\u0027d need to come back and add it in manually. Which I suppose wouldn\u0027t happen too frequently, but the current strategy of copying the returned structure and _only_ modifying the properties that need additional processing means that any newly returned columns are already included, no need for additional changes.","commit_id":"39400000c409d0eb482d4a25f9f53eb9cd37f63a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a827707695d6b42671f12b87b8f176e31b371f58","unresolved":true,"context_lines":[{"line_number":228,"context_line":"                hypervisor[\u0027cpu_info\u0027] \u003d json.loads(hypervisor[\u0027cpu_info\u0027])"},{"line_number":229,"context_line":"            else:"},{"line_number":230,"context_line":"                hypervisor[\u0027cpu_info\u0027] \u003d {}"},{"line_number":231,"context_line":"        display_columns, columns \u003d _get_hypervisor_columns(hypervisor)"},{"line_number":232,"context_line":"        data \u003d utils.get_dict_properties("},{"line_number":233,"context_line":"            hypervisor, columns,"},{"line_number":234,"context_line":"            formatters\u003d{"}],"source_content_type":"text/x-python","patch_set":1,"id":"a16fb38e_f3e067c7","line":231,"in_reply_to":"cd0c2222_8e65c871","updated":"2022-10-26 09:27:10.000000000","message":"This is true, but that won\u0027t change unless openstacksdk changes and I\u0027d hope whoever modifies SDK would modify OSC also","commit_id":"39400000c409d0eb482d4a25f9f53eb9cd37f63a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a915290e07d316aa1f2cd5747b5073af767a6393","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                hypervisor[\u0027cpu_info\u0027] \u003d json.loads(hypervisor[\u0027cpu_info\u0027])"},{"line_number":229,"context_line":"            else:"},{"line_number":230,"context_line":"                hypervisor[\u0027cpu_info\u0027] \u003d {}"},{"line_number":231,"context_line":"        display_columns, columns \u003d _get_hypervisor_columns(hypervisor)"},{"line_number":232,"context_line":"        data \u003d utils.get_dict_properties("},{"line_number":233,"context_line":"            hypervisor, columns,"},{"line_number":234,"context_line":"            formatters\u003d{"}],"source_content_type":"text/x-python","patch_set":1,"id":"e5988bc6_3cf572f5","line":231,"in_reply_to":"f9466cfc_3b00f494","updated":"2022-11-07 16:29:02.000000000","message":"It\u0027d be a nice follow-up. It doesn\u0027t need to happen in this change though. This is good enough, I just think it could be slightly better :)","commit_id":"39400000c409d0eb482d4a25f9f53eb9cd37f63a"}]}